notEqualTo does not accumulate when used multiple times

[sadortun]

New Issue Checklist

• I am not disclosing a vulnerability.
• I am not just asking a question.
• I have searched through existing issues.
• I can reproduce the issue with the latest version of Parse Server.

Issue Description

Hi !

Lets say you want to get all documents where the name is not bob or john. The simplest way you can do this is something like this :

Steps to reproduce

const q = new Parse.Query(User)

q.notEqualTo('name', 'bob')
q.notEqualTo('name', 'john')

await q.find()

Unfortunatly, this query will be executed as

{
  where: {
    "name": {
      "$ne": "john"
      }
   }
}

Actual Outcome

The results will contains users with name=bob.

Expected Outcome

The objects should not include any 'bob' or 'john'

Alternatively, if multiple notEqualTo is not accepted on the same key, an exception should be thrown on later calls to advise everyone to use notContainedIn or another more appropriate function.

Also, the docs do not reflect this limitation.

Failing Test Case / Pull Request

• 🤩 I submitted a PR with a fix and a test case.
• 🧐 I submitted a PR with a failing test case. 🚀 🔥 Added test case for issue #7459 #7460

Environment

Server

• Parse Server version: master / 129f7bfa9b7a3252b2d75524f441e81b7c37231c
• Operating system: FILL_THIS_OUT
• Local or remote host (AWS, Azure, Google Cloud, Heroku, Digital Ocean, etc): FILL_THIS_OUT

Database

• System (MongoDB or Postgres): mongo
• Database version: ``
• Local or remote host (MongoDB Atlas, mLab, AWS, Azure, Google Cloud, etc): local

Client

• SDK (iOS, Android, JavaScript, PHP, Unity, etc): js
• SDK version: FILL_THIS_OUT

Logs

[mtrezza]

Thanks for reporting.

This example is essentially overwriting the notEqualTo condition for the given field. Whether this is a bug may depend on how other conditions behave when being overwritten and what the expected behavior in this case would be.

I would assume that most if not every condition is overwritten when set for the same field. If there is indeed variation, then that should be reflected in the docs.

I would even say that it could be a developer mistake to set the same condition for the same field multiple times. I don't think the behavior is currently defined and it cannot be guaranteed that the first or second condition will be applied.

Another interesting though could be how to "unset" an existing notEqualTo for a field.

[sadortun]

@mtrezza I looked rapidly and the other conditions that override themselves seems OK. except the notEqualTo

For the equalTo and others it make total sense to override the condition, but not for the notEqualTo

// This make no sense (always false)
value === 'bob' && value === 'john'

// In the case of notEqualTo() it should be possible to add multiple exclusions
value !==  'bob' && value !== 'john'

Also, i did not test, but this should also probably be valid :

const q  = new Parse.Query(User)

q.notEqualTo('name' , null)  // a.k.a  q.exists('name') ?
q.notEqualTo('name' , 'bob')
// ....

[mtrezza]

For the equalTo and others it make total sense to override the condition, but not for the notEqualTo

// This make no sense (always false)
value === 'bob' && value === 'john'

// In the case of notEqualTo() it should be possible to add multiple exclusions
value !==  'bob' && value !== 'john'

Whether any of these make sense depends on which logical operator (OR or AND) you expect in between. equalTo OR equalTo makes as much sense as notEqualTo AND notEqualTo. You are correct that equalTo AND equalTo doesn't make sense, but that is not the logical complement of notEqualTo AND notEqualTo but of notEqualTo OR notEqualTo.

Do you see any condition that currently allows to be set multiple times for the same field, and do not simply overwrite each other but accumulate?

[Moumouls]

Hi @sadortun, just here for some feedback (not a solution) 🙂

Negative queries (like not) are really expensive for the database (the DB engine is forced to go through the whole DB). Also here the condition is little bit tricky.

Here i do not have you query context but from my experience in many cases the query could be refactored to a Non negative query, better performance, less unwanted results.

[sadortun]

Negative queries (like not) are really expensive for the database

Yes I agree with you they might not be the most efficient things but sometimes they are needed anyway.

My main issue here is that Parse should probably not simply silently dismiss one condition, but either run the expected (slow) query or yell at you if it's consider an error or an unsupported feature that one or many query conditions are being ignored.

I see this as a potential security issue if, for example, devloppers uses notEqualTo to exclude invalid/disabled/banned user statuses.

but that is not the logical complement of notEqualTo AND notEqualTo

Not sure what you mean 🙂, if you want to exclude bob and john you need notEqual / && operators

const t = (value)=> value !==  'bob' && value !== 'john';
['bob','john','albert'].map(f=> console.log (`${f} ${t(f)}`))

[mtrezza]

I chime in again as you suggested that this was a potential security issue. I want to make sure we give such critical concerns appropriate attention.

In the scenario you described, the security issue arises from a developer mistake, not from the Parse Server code base. notEqualTo is simply not aggregatable. In my previous comment I laid out how to determine whether it should be aggregatable or not. In short, if no other condition is aggregatable, it would be unintuitive to make this the only aggregate condition.

[sadortun]

Hi @mtrezza,

I did dig a bit more and found out that non-aggreatable behavior of noEqualTo is actually desired if you look at this test

parse-server/spec/QueryTools.spec.js

Line 296 in d8dc524

q.notEqualTo('score', 40);

I still think it would be preferable to have the not* conditions be aggregable, but I can live without this :)

I would still recommend to have either an debug/error raised or add a note of some sort in the docs.

I haven't compare to that many other frameworks, but from what I see, most of the other query API allow to aggregate negative conditions ( realtime database and Firebase for example)

I'll close the PR with the test and lmk if you think we should add something in the docs.

[mtrezza]

I still think it would be preferable to have the not* conditions be aggregable, but I can live without this :)

Let's see, pro:

• Add alternative way to add multiple $ne conditions

Con:

• Multiple $ne conditions are already possible with notContainedIn.
• Not possible to overwrite the condition anymore.
• Breaking change.
• Multiple $ne have to be translated into a AND combined DB query, which is an opaque introduction of a more complex query type. It is possible that a AND compounded $ne query is more expensive than a $nin query. It is also possible that the MongoDB query optimizer simplifies that into $nin anyway.

I would still recommend to have either an debug/error raised or add a note of some sort in the docs.

As it currently stands, it is a developer mistake to expect notEqualTo to aggregate instead of using notContainedIn. The use case of overwriting is valid and may well be used by developers, so a log entry could only be an excerpt of the docs, because we don't know whether the developer intended overwriting or aggregation. We do not use logging to guide the developer how to use the SDK, that is the purpose of the docs.

Adding a note to the docs may help developers, and maybe it makes most sense to add a general note regarding the non-aggregatability of any condition on the same field - if that is the case.

It would be great if you could investigate whether any condition currently allows to aggregate (which I don't believe) and then add a PR to the docs to add a general point about non-aggregability.

[sadortun]

Perfect! Your explanations male total sense !

For the docs this is what I had in mind

---

notEqualTo .....

notEqualTo can only be used to exclude a single field value from a query. When you need to exclude multiple values, you must use notContainedIn

For example

// name != john &&:name != bob
q.notContainrdIn('name', ['john','bob'])

[mtrezza]

If this applies to all conditions, it would be unnecessary to add this to every condition, or misleading to add it only to a few. Could you verify that there isn't a condition that is indeed aggregateable and rephrase this to make this a general note for all conditions? And were in the docs you would add this.