Fix query negation in MongoDB #814
Conversation
…lso requires checks for the existence of properties in case of and .
…by using the opposite operator. The issue with not selecting entries where the property is missing or has value null however remains.
…property are no longer returned.
…uses $nor as this would also return documents with null in the queried fields.
Codecov Report
@@ Coverage Diff @@
## master #814 +/- ##
==========================================
+ Coverage 92.64% 92.69% +0.04%
==========================================
Files 67 67
Lines 3724 3749 +25
==========================================
+ Hits 3450 3475 +25
Misses 274 274
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report at Codecov.
|
|
Thanks for this @JPBergsma, I'd really like to get #797 in first, if you wouldn't mind re-reviewing that. I will take a look at this in the meantime. |
There was a problem hiding this comment.
Looks good so far @JPBergsma!
Could you please also add some tests to some of the end results, e.g. concoct some queries on the test data that should return a certain set of structures? That way we can also test that this is already being handled correctly in ES (and that the underlying MongoDB can use the query we generated). These tests can be found in tests.server.query_params.test_filter.
Changed formatting description handle_not_and function Co-authored-by: Matthew Evans <7916000+ml-evs@users.noreply.github.com>
…onsortia/optimade-python-tools into JPBergsma/closes_#79
…nto a class attribute.
reverted providers.json back to its original state Co-authored-by: Matthew Evans <7916000+ml-evs@users.noreply.github.com>
There was a problem hiding this comment.
Thanks for this @JPBergsma!
I've just had a first look and this is really shaping up. I've made a couple of preliminary comments below before I do a full review soon.
Co-authored-by: Matthew Evans <7916000+ml-evs@users.noreply.github.com>
…onsortia/optimade-python-tools into JPBergsma/closes_#79
Co-authored-by: Matthew Evans <7916000+ml-evs@users.noreply.github.com>
Co-authored-by: Matthew Evans <7916000+ml-evs@users.noreply.github.com>
…onsortia/optimade-python-tools into JPBergsma/closes_#79
…onsortia/optimade-python-tools into JPBergsma/closes_#79
JPBergsma
force-pushed
the
JPBergsma/closes_#79
branch
from
6846522
to
9387420
Compare
There was a problem hiding this comment.
Great work @JPBergsma, happy to approve this.
As there are two orthogonal changes in this PR (query negation & providers installation instructions), we would typically rebase/squash this PR into a smaller number of commits before merging, but as the provider installation instructions are very minor and the other changes are well localised, I think we can just squash this PR as one big commit.
The other option would be to manually rebase the changes in a sensible way and split into 3-4 commits, but I think this will require quite a lot of unpicking given the whitespace changes (even though they are no longer in the final changes).
So it's up to you - feel free to hit the squash+merge button when you are ready, or reorganise the PR and re-request my review!
|
I will go for the squash and merge. |
I have tried to fix the issues mentioned in issue #79 including simplifying the queries, for example $not :{$gt : 1} -> {$lte:1}