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

Fix query negation in MongoDB #814

Merged
merged 35 commits into from
Jun 29, 2021
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
35 commits
Select commit Hold shift + click to select a range
9a2a6d6
Test now expects NOT statements to be simplified and propagated and a…
JPBergsma May 18, 2021
495b74b
fixed typo in test_operators in test_mongo.py
JPBergsma May 18, 2021
14d43be
Fix issue_#79, added not propagation and simplifies queries with not …
JPBergsma May 18, 2021
795a228
Documents with missing properties or a value of null for the queried …
JPBergsma May 19, 2021
4078922
Double negations now remove "prop" : {"$ne" : None} field. No longer …
JPBergsma May 21, 2021
d3de4b1
fixed issue with extra : None incase of NOT KNOWN
JPBergsma May 26, 2021
1ad3cea
added extra test for NOT HAS ALL querry
JPBergsma May 27, 2021
7ce7aff
Merge branch 'master' into JPBergsma/closes_#79
JPBergsma May 27, 2021
bea4eeb
Merge branch 'master' into JPBergsma/closes_#79
ml-evs Jun 1, 2021
3dce37f
Merge branch 'master' into JPBergsma/closes_#79
JPBergsma Jun 15, 2021
9632c07
added extra test to tets cases with AND and OR statements with three …
JPBergsma Jun 16, 2021
fbe469c
Apply suggestions from code review
JPBergsma Jun 17, 2021
8996dc7
Merge branch 'JPBergsma/closes_#79' of https://github.com/Materials-C…
JPBergsma Jun 17, 2021
a54e8eb
changed formatting description handle_not_or
JPBergsma Jun 17, 2021
91f336a
tried to remove whitespace in empty line 220
JPBergsma Jun 17, 2021
33353f2
added test to test_filter.py to test not propegation.
JPBergsma Jun 17, 2021
ef5a273
Renamed opposite_operator_map to inverse_operator_map and turned it i…
JPBergsma Jun 17, 2021
846a7dc
added test for double negation : {'': {''
JPBergsma Jun 18, 2021
8ebe7ad
added test for double negation : {'': {''
JPBergsma Jun 18, 2021
2d2f28d
Merge branch 'master' into JPBergsma/closes_#79
JPBergsma Jun 20, 2021
80edca9
Update optimade/server/data/providers.json
JPBergsma Jun 21, 2021
2340c99
Another attempt to create a working link to providers.json
JPBergsma Jun 21, 2021
1e5e79a
Updated INSTALL.md with with instructions to retrieve the list of pro…
JPBergsma Jun 21, 2021
f13a337
reworded instructions for retrieving optimade providers
JPBergsma Jun 22, 2021
be8182b
added extra tests to test behaviour NOT operator.
JPBergsma Jun 23, 2021
605ad5c
Merge branch 'JPBergsma/closes_#79' of https://github.com/Materials-C…
JPBergsma Jun 23, 2021
e1623c6
Updated comments in code with suggestions Matthew
JPBergsma Jun 25, 2021
f8b312a
Processed comments code review Matthew
JPBergsma Jun 25, 2021
8685e40
Applied coode suggestions for comments from Matthew
JPBergsma Jun 25, 2021
6d842aa
storing current state to add codesuggestions from github
JPBergsma Jun 25, 2021
9228d5d
Merge branch 'JPBergsma/closes_#79' of https://github.com/Materials-C…
JPBergsma Jun 25, 2021
d1e97a6
Merge branch 'master' into JPBergsma/closes_#79
JPBergsma Jun 25, 2021
9d06214
Double negations, e.g. NOT(NOT(... , is now simplified correctly.
JPBergsma Jun 25, 2021
9387420
Merge branch 'JPBergsma/closes_#79' of https://github.com/Materials-C…
JPBergsma Jun 27, 2021
b92fc9f
test wether precommit black behaves different from local black.
JPBergsma Jun 29, 2021
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
3 changes: 3 additions & 0 deletions INSTALL.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,9 @@ conda activate optimade
# Install package and dependencies in editable mode (including "dev" requirements).
pip install -e ".[dev]"

# Retrieve a list of providers. (Without this list some of the test will fail because providers.json cannot be found.)
ml-evs marked this conversation as resolved.
Show resolved Hide resolved
git submodule update --init

# Run the tests with pytest
py.test

Expand Down
101 changes: 79 additions & 22 deletions optimade/filtertransformers/mongo.py
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,17 @@ class MongoTransformer(BaseTransformer):
"=": "$eq",
}

inverse_operator_map = {
ml-evs marked this conversation as resolved.
Show resolved Hide resolved
"$lt": "$gte",
"$lte": "$gt",
"$gt": "$lte",
"$gte": "$lt",
"$ne": "$eq",
"$eq": "$ne",
"$in": "$nin",
"$nin": "$in",
}

def postprocess(self, query: Dict[str, Any]):
"""Used to post-process the nested dictionary of the parsed query."""
query = self._apply_relationship_filtering(query)
Expand Down Expand Up @@ -208,30 +219,82 @@ def property_zip_addon(self, arg):

def _recursive_expression_phrase(self, arg):
JPBergsma marked this conversation as resolved.
Show resolved Hide resolved
"""Helper function for parsing `expression_phrase`. Recursively sorts out
the correct precedence for `$and`, `$or` and `$nor`.
the correct precedence for `$not', '$and` and `$or`.
JPBergsma marked this conversation as resolved.
Show resolved Hide resolved

"""
if len(arg) == 1:
# without NOT
return arg[0]

# handle the case of {"$not": {"$or": [expr1, expr2]}} using {"$nor": [expr1, expr2]}.
if "$or" in arg[1]:
return {"$nor": self._recursive_expression_phrase([arg[1]["$or"]])}
def handle_not_and(arg):
JPBergsma marked this conversation as resolved.
Show resolved Hide resolved
"""Handle the case of `{"$not": {"$and": [expr1, expr2]}}`
which is equal to `{"or": [{"$not": expr1}, {"$not": expr2}]}`}
We have to check for the special case in which the "and" was created by a previous NOT
e.g `NOT (NOT ({"a": {"$eq" : 6}})) -> NOT({$and:[{"a": {"$ne" : 6}},{"a": {"$ne": None}}]})`

"""
JPBergsma marked this conversation as resolved.
Show resolved Hide resolved

expr1 = arg["$and"][0]
expr2 = arg["$and"][1]
key = list(expr1.keys())[0]
if expr1.keys() == expr2.keys():
if expr1.get(key) == {"$ne": None}:
return self._recursive_expression_phrase(["NOT", expr2])
elif expr2.get(key) == {"$ne": None}:
return self._recursive_expression_phrase(["NOT", expr1])
ml-evs marked this conversation as resolved.
Show resolved Hide resolved
return {
"$or": [
self._recursive_expression_phrase(["NOT", subdict])
for subdict in arg["$and"]
]
}

def handle_not_or(arg):
JPBergsma marked this conversation as resolved.
Show resolved Hide resolved
"""Handle the case of {"$not": {"$or": [expr1, expr2]}} using:
{"$not": {"$or": [expr1, expr2]}} == {"$and": [(NOT, expr1), (NOT, expr2)]}}
{"$nor": [expr1, expr2]} is not convenient because nor will also return documents where the field in expr1 or exp2 is missing.

"""
JPBergsma marked this conversation as resolved.
Show resolved Hide resolved

# handle the case of {"$not": {"$and": [expr1, expr2]}} using per-expression negation,
# e.g. {"$and": [{prop1: {"$not": expr1}}, {prop2: {"$not": ~expr2}}]}.
# Note that this is not the same as NOT (expr1 AND expr2)!
if "$and" in arg[1]:
return {
"$and": [
self._recursive_expression_phrase(["NOT", subdict])
for subdict in arg[1]["$and"]
for subdict in arg["$or"]
]
}

# simple case of negating one expression, from NOT (expr) to ~expr.
return {prop: {"$not": expr} for prop, expr in arg[1].items()}
if len(arg) == 1:
# without NOT
return arg[0]

if "$or" in arg[1]:
return handle_not_or(arg[1])

if "$and" in arg[1]:
return handle_not_and(arg[1])

# Incase the NOT operator occurs at the lowest nesting level,
# the expression can be simplified by using the opposite operator and removing the not.

JPBergsma marked this conversation as resolved.
Show resolved Hide resolved
for prop, expr in arg[1].items():
for operator, value in expr.items():
if operator in self.inverse_operator_map:
if operator in ("$in", "$eq"):
return {
"$and": [
{
prop: {
self.inverse_operator_map.get(operator): value
}
},
{prop: {"$ne": None}},
]
}

else:
return {prop: {self.inverse_operator_map.get(operator): value}}
else:
if "#known" in expr:
return {prop: {"$not": expr}}
else:
return {"$and": [{prop: {"$not": expr}}, {prop: {"$ne": None}}]}
JPBergsma marked this conversation as resolved.
Show resolved Hide resolved

def _apply_length_operators(self, filter_: dict) -> dict:
"""Check for any invalid pymongo queries that involve applying a
Expand Down Expand Up @@ -275,9 +338,7 @@ def apply_length_op(subdict, prop, expr):
return subdict

return recursive_postprocessing(
filter_,
check_for_length_op_filter,
apply_length_op,
filter_, check_for_length_op_filter, apply_length_op
JPBergsma marked this conversation as resolved.
Show resolved Hide resolved
)

def _apply_relationship_filtering(self, filter_: dict) -> dict:
Expand Down Expand Up @@ -307,11 +368,7 @@ def replace_with_relationship(subdict, prop, expr):
subdict["$and"] = []
subdict["$and"].extend(
[
{
f"relationships.{_prop}.data": {
"$size": expr.pop("$size"),
}
},
{f"relationships.{_prop}.data": {"$size": expr.pop("$size")}},
ml-evs marked this conversation as resolved.
Show resolved Hide resolved
{f"relationships.{_prop}.data.{_field}": expr},
]
)
Expand Down