Skip to content

feat: add null value filter #1721

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

Draft
wants to merge 7 commits into
base: alpha
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 12 additions & 5 deletions src/lib/Filters.js
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,19 @@ export const Constraints = {
field: null,
comparable: false
},
dneOrNull: {
name: 'does not exists or null',
field: null,
comparable: false
}
};

export const FieldConstraints = {
'Pointer': [ 'exists', 'dne', 'eq', 'neq', 'unique' ],
'Boolean': [ 'exists', 'dne', 'eq', 'unique' ],
'Number': [ 'exists', 'dne', 'eq', 'neq', 'lt', 'lte', 'gt', 'gte', 'unique' ],
'String': [ 'exists', 'dne', 'eq', 'neq', 'starts', 'ends', 'stringContainsString', 'unique' ],
'Date': [ 'exists', 'dne', 'before', 'after', 'unique' ],
'Pointer': [ 'exists', 'dne', 'eq', 'neq', 'unique', 'dneOrNull' ],
'Boolean': [ 'exists', 'dne', 'eq', 'unique', 'dneOrNull' ],
'Number': [ 'exists', 'dne', 'eq', 'neq', 'lt', 'lte', 'gt', 'gte', 'unique', 'dneOrNull' ],
'String': [ 'exists', 'dne', 'eq', 'neq', 'starts', 'ends', 'stringContainsString', 'unique', 'dneOrNull' ],
'Date': [ 'exists', 'dne', 'before', 'after', 'unique', 'dneOrNull' ],
'Object': [
'exists',
'dne',
Expand All @@ -183,6 +188,7 @@ export const FieldConstraints = {
'keyLt',
'keyLte',
'unique',
'dneOrNull'
],
'Array': [
'exists',
Expand All @@ -193,6 +199,7 @@ export const FieldConstraints = {
'doesNotContainNumber',
'containsAny',
'doesNotContainAny',
'dneOrNull'
]
};

Expand Down
3 changes: 3 additions & 0 deletions src/lib/queryFromFilters.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.

break;
}
return query;
}