Adding $nor operator support #4768

jeremypiednoel · 2018-05-18T15:09:36Z

@dplewis tell me if you want me to investigate on postgresql !

dplewis · 2018-05-18T15:14:24Z

@jeremypiednoel Go for it! Let me know if you need anything.

jeremypiednoel · 2018-05-18T15:15:35Z

@dplewis I'm very noob in postgresql. So let's see if i can figure out

flovilmart · 2018-05-18T15:17:12Z

Very nice! We'll need to add some docs as the logical explanation is not trivial for a non seasoned developer.

jeremypiednoel · 2018-05-18T15:19:34Z

Btw am i the only one to have pb with .github/ISSUE_TEMPLATE/Bug_report.md maybe it's my IDE but it always updates this file and ask me to commit it

flovilmart · 2018-05-18T15:20:39Z

@jeremypiednoel did you pull the latest master& this should be good now.

jeremypiednoel · 2018-05-18T15:24:42Z

see the latest commit it's a CRLF error i think. does it make sense to fix it ?

dplewis · 2018-05-18T15:26:25Z

@jeremypiednoel That issue was fixed minutes ago #4767

jeremypiednoel · 2018-05-18T15:27:45Z

@dplewis very true i had to commit the file first to pull the upstream then.
nvm it's fixed 👍

jeremypiednoel · 2018-05-18T15:35:03Z

@dplewis how can i specify i want the test to run on postgresql ?

dplewis · 2018-05-18T15:36:18Z

Check CONTRIBUTING.md

https://github.com/parse-community/parse-server/blob/master/CONTRIBUTING.md

jeremypiednoel · 2018-05-18T15:37:54Z

Thanks ! i'll try to make it work

dplewis · 2018-05-18T16:16:18Z

@jeremypiednoel If you want to cheat you can find a solution on my branch for PG.

https://github.com/dplewis/parse-server/tree/nor-op

You will still need test for empty $nor array (I know that mongo and pg throws an error for nonempty $or, $and, $nor) and a test for invalid nor (not array)

jeremypiednoel · 2018-05-18T16:17:49Z

thnaks @dplewis i'm gonna check that after lunch !

codecov · 2018-05-18T16:51:23Z

Codecov Report

Merging #4768 into master will decrease coverage by 0.04%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #4768      +/-   ##
==========================================
- Coverage   92.68%   92.64%   -0.05%     
==========================================
  Files         119      119              
  Lines        8659     8663       +4     
==========================================
  Hits         8026     8026              
- Misses        633      637       +4

Impacted Files	Coverage Δ
src/Adapters/Storage/Mongo/MongoTransform.js	`86.66% <100%> (-0.03%)`	⬇️
...dapters/Storage/Postgres/PostgresStorageAdapter.js	`97.13% <100%> (ø)`	⬆️
src/Controllers/DatabaseController.js	`94.64% <100%> (+0.04%)`	⬆️
src/Adapters/Auth/meetup.js	`84.21% <0%> (-5.27%)`	⬇️
src/RestWrite.js	`93.06% <0%> (-0.55%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a9ebc2b...318501c. Read the comment docs.

jeremypiednoel · 2018-05-18T16:51:36Z

@dplewis can you take a look ?

dplewis · 2018-05-18T17:13:30Z

spec/ParseQuery.spec.js

@@ -2567,6 +2567,48 @@ describe('Parse.Query testing', () => {
    });
  });

+  it('$select inside $nor', (done) => {


Change the name as $select isn't being used here

dplewis · 2018-05-18T17:14:04Z

spec/ParseQuery.spec.js

+    obj.save().then(() => {
+      return rp.get(Parse.serverURL + "/classes/TestObject", options);
+    }).then(done.fail).catch((error) => {
+      equal(error.error.code, Parse.Error.INVALID_JSON);


Should be INVALID_QUERY.

dplewis · 2018-05-18T17:15:56Z

src/Controllers/DatabaseController.js

@@ -62,7 +62,7 @@ const validateQuery = (query: any): void => {
  }

  if (query.$or) {
-    if (query.$or instanceof Array) {


Revert these changes to $or and $and as are they are causing tests to fail.

They also have nothing to do with the PR.

dplewis · 2018-05-18T17:17:12Z

spec/ParseQuery.spec.js

+    });
+  });
+
+  it('$nor invalid query', (done) => {


Maybe another test when $nor: 1234 (non array)

jeremypiednoel · 2018-05-18T17:24:50Z

Thanks for the feedback, it's fixed

i'll fix the minimum size of the $and and $or in another PR according to the mongo doc

$and performs a logical AND operation on an array of two or more expressions
The $or operator performs a logical OR operation on an array of two or more

flovilmart · 2018-05-18T18:18:05Z

src/Adapters/Storage/Postgres/PostgresStorageAdapter.js

      const clauses = [];
      const clauseValues = [];
      fieldValue.forEach((subQuery) =>  {
        const clause = buildWhereClause({ schema, query: subQuery, index });
        if (clause.pattern.length > 0) {
+          if (fieldName === '$nor') {
+            clause.pattern = `(NOT ${clause.pattern})`;
+          }


you're doing (NOT query1) AND (NOT query2) instead of NOT (query1 OR query2) whhich tecnically work but can be a bit misleading for maintenance. Can« you document it or revert the logic with the help of line 323? Event if it's the same, rtaht will help for comprehension in the future ;)

True I was pretty lazy 😅

Yeah I figured it worked but letMs write it cleanly please

I'll fix that

flovilmart

A small nit for maintainability :)

jeremypiednoel · 2018-05-18T19:02:14Z

@flovilmart @dplewis i reverted the logic.
Thanks for the feedbacks

flovilmart · 2018-05-18T19:09:17Z

@jeremypiednoel thanks for the changes, looking good, waiting for the CI.

jeremypiednoel · 2018-05-18T19:12:02Z

@flovilmart do you think it makes sense to add the array.length test on $end and $or conditions ?

flovilmart · 2018-05-18T19:24:05Z

Let’s not change it now, as it would be a breaking change for the clients (change in behavior) that yield an error instead of returning results.

* adding nor to specialQuerykeys * adding nor suport * adding test * CRLF * adding postgres NOR * adding nor validation * adding NOR test * adding test amd fixing NOR cases * revert the nor logic

jeremypiednoel added 4 commits May 18, 2018 10:57

adding nor to specialQuerykeys

8898481

adding nor suport

b668356

adding test

9431576

Merge branch 'master' into master

b648ccc

jeremypiednoel added 2 commits May 18, 2018 11:23

CRLF

1c597b7

Merge branch 'master' of github.com:jeremypiednoel/parse-server

5301fc3

jeremypiednoel added 3 commits May 18, 2018 12:50

adding postgres NOR

64a9ea5

adding nor validation

d493f39

adding NOR test

6944639

jeremypiednoel added 3 commits May 18, 2018 13:12

Merge branch 'master' into master

51d799a

Merge branch 'master' of github.com:parse-community/parse-server

730ad21

Merge branch 'master' of github.com:jeremypiednoel/parse-server

388ea12

dplewis suggested changes May 18, 2018

View reviewed changes

adding test amd fixing NOR cases

4415748

dplewis requested a review from flovilmart May 18, 2018 18:10

dplewis previously approved these changes May 18, 2018

View reviewed changes

flovilmart reviewed May 18, 2018

View reviewed changes

flovilmart suggested changes May 18, 2018

View reviewed changes

revert the nor logic

318501c

jeremypiednoel dismissed dplewis’s stale review via 318501c May 18, 2018 19:01

flovilmart approved these changes May 18, 2018

View reviewed changes

dplewis merged commit 77ed10f into parse-community:master May 18, 2018

dplewis mentioned this pull request May 24, 2018

Roadmap parse-community/parse-php-sdk#401

Closed

8 tasks

dplewis mentioned this pull request Aug 10, 2018

Support nor Query parse-community/Parse-SDK-JS#634

Merged

This was referenced Aug 24, 2018

Roadmap parse-community/Parse-SDK-iOS-OSX#1332

Closed

Roadmap parse-community/Parse-SDK-Android#879

Open

[Feature Request] nor Query parse-community/Parse-SDK-iOS-OSX#1334

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding $nor operator support #4768

Adding $nor operator support #4768

jeremypiednoel commented May 18, 2018 •

edited

Loading

dplewis commented May 18, 2018

jeremypiednoel commented May 18, 2018

flovilmart commented May 18, 2018 •

edited

Loading

jeremypiednoel commented May 18, 2018 •

edited

Loading

flovilmart commented May 18, 2018

jeremypiednoel commented May 18, 2018

dplewis commented May 18, 2018

jeremypiednoel commented May 18, 2018

jeremypiednoel commented May 18, 2018

dplewis commented May 18, 2018

jeremypiednoel commented May 18, 2018

dplewis commented May 18, 2018

jeremypiednoel commented May 18, 2018

codecov bot commented May 18, 2018 •

edited

Loading

jeremypiednoel commented May 18, 2018

dplewis May 18, 2018

dplewis May 18, 2018

dplewis May 18, 2018

dplewis May 18, 2018

jeremypiednoel commented May 18, 2018

flovilmart May 18, 2018 •

edited

Loading

dplewis May 18, 2018

flovilmart May 18, 2018

jeremypiednoel May 18, 2018

flovilmart left a comment

jeremypiednoel commented May 18, 2018

flovilmart commented May 18, 2018

jeremypiednoel commented May 18, 2018

flovilmart commented May 18, 2018

Adding $nor operator support #4768

Adding $nor operator support #4768

Conversation

jeremypiednoel commented May 18, 2018 • edited Loading

dplewis commented May 18, 2018

jeremypiednoel commented May 18, 2018

flovilmart commented May 18, 2018 • edited Loading

jeremypiednoel commented May 18, 2018 • edited Loading

flovilmart commented May 18, 2018

jeremypiednoel commented May 18, 2018

dplewis commented May 18, 2018

jeremypiednoel commented May 18, 2018

jeremypiednoel commented May 18, 2018

dplewis commented May 18, 2018

jeremypiednoel commented May 18, 2018

dplewis commented May 18, 2018

jeremypiednoel commented May 18, 2018

codecov bot commented May 18, 2018 • edited Loading

Codecov Report

jeremypiednoel commented May 18, 2018

dplewis May 18, 2018

Choose a reason for hiding this comment

dplewis May 18, 2018

Choose a reason for hiding this comment

dplewis May 18, 2018

Choose a reason for hiding this comment

dplewis May 18, 2018

Choose a reason for hiding this comment

jeremypiednoel commented May 18, 2018

flovilmart May 18, 2018 • edited Loading

Choose a reason for hiding this comment

dplewis May 18, 2018

Choose a reason for hiding this comment

flovilmart May 18, 2018

Choose a reason for hiding this comment

jeremypiednoel May 18, 2018

Choose a reason for hiding this comment

flovilmart left a comment

Choose a reason for hiding this comment

jeremypiednoel commented May 18, 2018

flovilmart commented May 18, 2018

jeremypiednoel commented May 18, 2018

flovilmart commented May 18, 2018

jeremypiednoel commented May 18, 2018 •

edited

Loading

flovilmart commented May 18, 2018 •

edited

Loading

jeremypiednoel commented May 18, 2018 •

edited

Loading

codecov bot commented May 18, 2018 •

edited

Loading

flovilmart May 18, 2018 •

edited

Loading