feat: combine equalTo clauses with clauses #1373

sadakchap · 2021-06-08T13:44:03Z

New Pull Request Checklist

I am not disclosing a vulnerability.
I am creating this PR in reference to an issue.

Issue Description

equalTo query overrides any existing clauses before it. For e.g.

let q = new Parse.Query('MyClass');
q.greaterThan('age', 10);
q.equalTo('age', 9);
// this will override $gt query and will result something like this where: { age : 9 }

Related issue: #1372

Approach

After following through #1372 and parse-community/parse-php-sdk#476 and #1372 (comment).

Using this.addCondition(key, $eq, value) instead this.where[key], so that equalTo clause can be combined with other clauses and vice-versa.

TODOs before merging

Add tests
Add entry to changelog
Add changes to documentation (guides, repository pages, in-code descriptions)

davimacedo

@sadakchap do you mind to look at the failing tests?

Overall I like the idea. @dplewis what do you think?

mtrezza · 2021-09-10T16:14:06Z

have added only a failing test case.

@sadakchap it seems you have actually also added the fix in this PR, correct? If so, is this ready for review? I would like to add the related dashboard feature with the null filter.

sadakchap · 2021-09-15T02:25:43Z

If so, is this ready for review? I would like to add the related dashboard feature with the null filter.

Thanks @mtrezza , I think it still has some failing tests. I will try to fix those during this weekend and will let you know 🙂

parse-github-assistant · 2021-09-22T17:23:55Z

Thanks for opening this pull request!

🎉 We are excited about your hands-on contribution!

sadakchap · 2021-10-05T14:11:59Z

@mtrezza Thanks for following up & waiting.

About failing test cases. Only Live Query test are failing as they depend on parse-server to fire events. We will need to update parse-server to send events for $eq queries in order to fix all failing test case.

sadakchap · 2021-10-05T15:51:55Z

Here's the pr with added support for $eq query for LiveQuery.

@mtrezza @davimacedo @dplewis, would really like to hear your thoughts about it?

mtrezza · 2021-10-06T09:22:27Z

src/ParseQuery.js

@@ -1222,7 +1222,8 @@ class ParseQuery {
      return this.doesNotExist(key);
    }

-    this._where[key] = encode(value, false, true);
+    // this._where[key] = encode(value, false, true);


This comment seems to be a leftover?

Sure, will remove.

Will update the pr once all checks are passing.

mtrezza · 2021-10-06T09:24:33Z

src/__tests__/Cloud-test.js

@@ -301,7 +301,9 @@ describe('CloudController', () => {
    expect(data).toEqual({
      limit: 1,
      where: {
-        objectId: 'jobId1234',


Should all these tests be modified? That means the original syntax would not be tested anymore?

Yes, all test are modified. And yes, original syntax would not be tested anymore as it will always make any equalTo query with this updated syntax { $eq: '' }

Could that be a breaking change for existing deployments?

I don't see it as a breaking change, as it will just change the syntax of query for equalTo, everything else should be same.

For adding equalTo query, we still would be doing same thing.

q.equalTo('age', 10); // same implementation, just query that will be sent is of new syntax.

Does this mean the query sent to the DB has a different syntax, or only the query sent to Parse Server?

mtrezza · 2021-10-08T21:44:13Z

src/__tests__/ParseQuery-test.js

@@ -136,6 +140,7 @@ describe('ParseQuery', () => {
    expect(q.toJSON()).toEqual({
      where: {
        size: {
+          $eq: 'small',


It seems strange that this results in $eq:small and $exists:false, isn't that a contradiction?

The comment assumed: equalTo('key', undefined) resolves to 'does not exist'
But in this case it just adds dne to the query, which probably results in a different query sent to the DB?

Yes, It is contradiction. 😕

Suggestion: Maybe we can make $exists override $eq clause.

Problem: this pr is in response to this #1372, which was created due to this pr, where we were trying to make a query something like this(for filtering values which are null & exists)

where: { age: { $eq: null, $exists: true } }

which again won't be possible if $exists can't be combined with $eq

mtrezza · 2021-10-08T21:48:04Z

src/__tests__/ParseQuery-test.js

-        stock: true,
+        size: {
+          $eq: 'medium',
+          $exists: false,


This also looks strange

#1373 (comment)

added failing test case

62b8cd3

sadakchap mentioned this pull request Jun 8, 2021

Can not combine equalTo clause with any other clause #1372

Open

4 tasks

dplewis mentioned this pull request Jun 8, 2021

ParseQuery::equalTo with strange behavior parse-community/parse-php-sdk#476

Closed

sadakchap added 3 commits June 9, 2021 10:23

adding other clauses on equalTo

0fc3c45

add testcase for combine equalTo to existing clauses

0baadba

added condition of string type in add condition

2a04e8b

davimacedo requested changes Jun 10, 2021

View reviewed changes

sadakchap added 8 commits September 22, 2021 08:14

using addCondition in equalTo method

64bd728

updated testcase to check with arrays

2c294b4

added case for '$eq' at matchesKeyConstraints func

03ee504

fixed failing testcase in ParseQuery-test.js file

445fa8e

fixing failing testcases in Cloud-test.js

b20de35

fixed some testcase in LiveQueryClient-test.js

aaff959

safe cond, compareTo = null for matchesKeyConstraints

eb1ead9

updated $eq condition for Pointer and Array contains

7c9f111

sadakchap changed the title ~~Combine equalTo clause with other clauses~~ feat: combine equalTo clauses with clauses Sep 22, 2021

sadakchap mentioned this pull request Sep 25, 2021

feat: add null value filter parse-community/parse-dashboard#1721

Draft

This was referenced Oct 5, 2021

feat: Support $eq query for liveQuery parse-community/parse-server#7606

Closed

feat: add support for $eq in LiveQuery parse-community/parse-server#7607

Open

mtrezza reviewed Oct 6, 2021

View reviewed changes

mtrezza reviewed Oct 8, 2021

View reviewed changes

Merge branch 'alpha' into combineEqualToClause

d6ca68d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: combine equalTo clauses with clauses #1373

feat: combine equalTo clauses with clauses #1373

sadakchap commented Jun 8, 2021 •

edited

davimacedo left a comment

mtrezza commented Sep 10, 2021

sadakchap commented Sep 15, 2021

parse-github-assistant bot commented Sep 22, 2021 •

edited

sadakchap commented Oct 5, 2021

sadakchap commented Oct 5, 2021

mtrezza Oct 6, 2021

sadakchap Oct 6, 2021

sadakchap Oct 6, 2021

mtrezza Oct 6, 2021

sadakchap Oct 6, 2021

mtrezza Oct 6, 2021

sadakchap Oct 8, 2021

mtrezza Oct 8, 2021

mtrezza Oct 8, 2021

sadakchap Oct 13, 2021

mtrezza Oct 8, 2021

sadakchap Oct 13, 2021

feat: combine equalTo clauses with clauses #1373

Are you sure you want to change the base?

feat: combine equalTo clauses with clauses #1373

Conversation

sadakchap commented Jun 8, 2021 • edited

New Pull Request Checklist

Issue Description

Approach

TODOs before merging

davimacedo left a comment

Choose a reason for hiding this comment

mtrezza commented Sep 10, 2021

sadakchap commented Sep 15, 2021

parse-github-assistant bot commented Sep 22, 2021 • edited

Thanks for opening this pull request!

sadakchap commented Oct 5, 2021

sadakchap commented Oct 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sadakchap commented Jun 8, 2021 •

edited

parse-github-assistant bot commented Sep 22, 2021 •

edited