Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add null value filter #1721

Draft
wants to merge 7 commits into
base: alpha
Choose a base branch
from

Conversation

sadakchap
Copy link
Member

Feature request #1658

Added new filter to filter records if field contains null value.
nullFilter

@mtrezza
Copy link
Member

mtrezza commented Jun 3, 2021

Thanks for this PR.

In MongoDB, the value null is sometimes treated the same as if the field does not exist at all. In other words, these two documents are sometimes treated as identical:

{
  a: 1,
  b: null
}

{
  a: 1
}

That means from a MongoDB perspective there may not be a difference between the Parse Dashboard filters "does not exist" and "is null". To unambiguously query for objects with a field of content null we would have to query for the BSON Type 10.

The distinction in MongoDB is:

  • a) field : { $type: 10 }: returns documents where the field has value null.
  • b) field : { $exists: false }: returns documents where the field does not exist.
  • c) field : null: returns documents where the field either has value null or does not exist.

Your video shows a filer for null values, but the filer description isNull is ambiguous - which of the filters above are applied? We would have to look behind the scenes into Parse Server MongoDB adapter to see how the query is ultimately constructed. I suspect it is c), which would in that case overlap with the "does not exist" filter.

Another aspect to consider is how that plays with the Postgres adapter, or whether that is something unique to MongoDB.

@sadakchap
Copy link
Member Author

sadakchap commented Jun 3, 2021

In MongoDB, the value null is sometimes treated the same as if the field does not exist at all. In other words, these two documents are sometimes treated as identical:

True. That's why we are filtering for both exists & equal to null value. If filtering field is age then we are sending a query like this.

where: {
  $and: [
      age: { $exists: true },
      age: null
   ]
}

Your video shows a filer for null values, but the filer description isNull is ambiguous - which of the filters above are applied?

Could you please elaborate? I don't think I understand the question properly.

@mtrezza
Copy link
Member

mtrezza commented Jun 3, 2021

That's why we are filtering for both exists & equal to null value.

Do you mean that is how you personally use the filter to query for a)?

It may return the same results, but the correct - because more efficient - query for this is:

where: {
    age: { $type: 10 }
}

@sadakchap
Copy link
Member Author

sadakchap commented Jun 3, 2021

Do you mean that is how you personally use the filter to query for a)?

yes.

It may return the same results, but the correct - because more efficient - query for this is:

where: {
    age: { $type: 10 }
}

Yes, you are correct! But, How do I send a query like this, I searched in the docs but couldn't find anything that would query on type of the field.

@mtrezza
Copy link
Member

mtrezza commented Jun 3, 2021

Not sure whether Parse Server supports this yet, you would have to look into the code, it may exist but not be in the docs.
However, both filters (a) and (c) can be useful, so we may well offer both filters in the dashboard at some point in the future.

Therefore I suggest this naming to make them distinguishable:

a) field : { $type: 10 } filter name: is null
c) field: null filter name: does not exist or is null

If later someone adds filter (a), they can name the filter is null and it won't conflict with the filter name in this PR.
I don't know what this means for Postgres or other DBs, but it would be accurate for MongoDB.

@sadakchap
Copy link
Member Author

Not sure whether Parse Server supports this yet, you would have to look into the code, it may exist but not be in the docs.

I look into this file, but I couldn't find any thing that would query on type.

Therefore I suggest this naming to make them distinguishable:

a) field : { $type: 10 } filter name: is null
c) field: null filter name: does not exist or is null

If later someone adds filter (a), they can name the filter is null and it won't conflict with the filter name in this PR.

Sounds good 👍, so should I change the current query as well for does not exist or is null option
from

query.exists(field);
let nullQuery = new Parse.Query(className);
nullQuery.equalTo(field, null); 
query = Parse.Query.and(query, nullQuery);

to

query.equalTo(field, null)

as the current query fetches records only with null value.

@mtrezza
Copy link
Member

mtrezza commented Jun 4, 2021

should I change the current query as well for does not exist or is null option

Yes I think so. Can one still query only for null values by adding the field exists filter additionally for the same field in the dashboard?

@sadakchap
Copy link
Member Author

sadakchap commented Jun 4, 2021

Can one still query only for null values by adding the field exists filter additionally for the same field in the dashboard?

I think only one filter is allowed on a field at a time. No, I don't think it is possible.

@mtrezza
Copy link
Member

mtrezza commented Jun 4, 2021

I think only one filter is allowed on a field at a time. No, I don't think it is possible.

You could additionally add the is null filter if we find out how to do where { age: { $type: 10 } }. I think it should be already possible, maybe with a REST request, if the JS SDK doesn't support this yet.

If it really isn't possible, then you could add this:

query.exists(field);
let nullQuery = new Parse.Query(className);
nullQuery.equalTo(field, null); 
query = Parse.Query.and(query, nullQuery);

with a comment that this is a workaround because where { age: { $type: 10 } } is not possible with the JS SDK.

Then we could have both filters (is null; does not exist or is null) in the dashboard with this PR.

Copy link
Member

@dplewis dplewis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sadakchap Thanks for the PR! I was the one who requested it.

@@ -104,6 +104,9 @@ function addConstraint(query, filter) {
case 'keyLte':
addQueryConstraintFromObject(query, filter, 'lessThanOrEqualTo');
break;
case 'dneOrNull':
query.equalTo(filter.get('field'), null);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this query will return null values.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

True, it will also include records which does not have filter field even set, that's why the name of filter does not exits or null

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds good

I think only one filter is allowed on a field at a time.

You can set composable to true if you to do multiple filters on a field. Similar to how less than does it. (Perhaps we should document these)

Copy link
Member

@mtrezza mtrezza Jun 4, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enabling composable is a good idea.

Regarding the commented code here, the difference is the MongoDB performance. The current query is an explicit "does not exist or is null" conditional, without $and. This distinction is also relevant for indexing.

Enabling composable and add this filter query allows for both ways, so that's the most versatile way to go.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I mean document composable and comparable it usually takes a while to figure out what they mean.

Copy link
Member

@mtrezza mtrezza Jun 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sends where={age:{$exists: true}} if you set a combined filter of (1) and (2)?

Copy link
Member Author

@sadakchap sadakchap Jun 6, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It sends where={age:{$exists: true}} if you set a combined filter of (1) and (2)?

yes.

I think that's strange as, for let's say 2 filters less than & exists, we do something like

query.lessThan(field, compareTo); // compareTo is value entered by user to which s(he) wants to compare
query.exists(field);

This sends a query like where={age: {$lt: 4, $exists: true}}

Then, why not this

query.equalTo(field, null);
query.exists(field);

sends a query like where={age: {$eq: null, $exists: true}}.

I tried searching in thee docs about equalTo query, whether it is composble or not. But, there's nothing about that 😕

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dplewis when you mentioned composable, on what level did you mean that? On a MongoDB level?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think it's a limitation with MongoDB rather it's with JS Sdk.(just an opinion)

As when I explicitly(using cURL) make a request with where={age: {$exists: true, $eq: null }}, it does return records only with null field value.

But, when I'm trying to couple $exists & $eq query, it only sends request with where={age: {$exists: true}}.
I'm trying this way to construct the query

query.equalTo(field, null);
query.exists(field);

Maybe I'm missing some point with js sdk. Would really appreciate any help with creating a query with where={age: {$exists: true, $eq: null }}.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I created a new issue for this parse-community/Parse-SDK-JS#1372.

@mtrezza
Copy link
Member

mtrezza commented Jul 26, 2021

@sadakchap Do you think we can resolve any open questions here? I think this PR is almost ready for merge, it would be great to have it in the upcoming release of Parse Dashboard.

@sadakchap
Copy link
Member Author

@sadakchap Do you think we can resolve any open questions here? I think this PR is almost ready for merge, it would be great to have it in the upcoming release of Parse Dashboard.

This pr was originally to add a feature through which users can filter records with only null value.

But, we decided to add a filter dneOrNull, on which user a add one more filter exists => then user should be getting records with only null value. But, that is not the case, $exists and $eq query seems to overriding each other. Please see #1721 (comment) .

I also raised an issue in Parse-SDK-JS. But, I think I am really not the right person to solve this issue.

For this feature, maybe we can try coupling $exists & $eq with $and. Something like this 38d2f16 , maybe ?

@mtrezza mtrezza changed the title Add isNull filter feat: add isNull filter Sep 19, 2021
@mtrezza mtrezza changed the title feat: add isNull filter feat: add null value filter Sep 19, 2021
@mtrezza
Copy link
Member

mtrezza commented Sep 24, 2021

@sadakchap So maybe we can finalize this PR like this:

  • Add a filter condition "does not exist or is null" for (c) with this PR
  • Add a filter condition "is null" for (b) in a later PR once the Parse JS SDK issue is fixed and supports this properly

What do you think?

@sadakchap
Copy link
Member Author

Thanks @mtrezza .

I was working on this issue's PR. I was able to to clear all unit test cases. Right now, I think only LiveQuery Integration test cases are failing.

I know that, this is open from long time, but If we can wait just 1 more week ( so that I can try to fix those failing integration test cases). It would be really nice. Or else we can do as you have suggested(incase it takes more than a week).

@mtrezza
Copy link
Member

mtrezza commented Sep 25, 2021

Sure. Let's wait as you suggest.

@cbaker6
Copy link
Contributor

cbaker6 commented Jan 3, 2022

Another aspect to consider is how that plays with the Postgres adapter, or whether that is something unique to MongoDB.

For Postgres, the server treats doesNotExist and null the same, so I believe the filters here will produce the same results, which should be fine. I discuss more about this here: https://community.parseplatform.org/t/query-where-field-nil/2317/16?u=cbaker6

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants