Repository: Materials-Consortia/optimade-python-tools (python)

Issue:
Handling of KNOWN in mongo backend
Looks like we transform `property IS KNOWN` into the Mongo `"property": {"$exists": True}` where according to the spec we should also check if its null:

https://github.com/Materials-Consortia/OPTiMaDe/blob/develop/optimade.rst#36properties-with-an-unknown-value

Pull request:
Add null check to mongo filtertransformer for KNOWN/UNKNOWN filters
Closes #254, by adding comparison against `null` during KNOWN/UNKNOWN filters.

This had to be added as another post-processor, due to the mismatch between Lark/mongo. There is also some additional logic to handle the case of e.g. `NOT x is UNKNOWN`. This was achieved by adding a dummy magic keyword "#known" when building the lark query, which is then expanded out by the post-processor after the `NOT (expr)` rule has been applied.

Patch:
diff --git a/optimade/filtertransformers/mongo.py b/optimade/filtertransformers/mongo.py
index 52d82f64..3dedfcf8 100755
--- a/optimade/filtertransformers/mongo.py
+++ b/optimade/filtertransformers/mongo.py
@@ -40,6 +40,7 @@ class MongoTransformer(Transformer):
 
         query = self._apply_relationship_filtering(query)
         query = self._apply_length_operators(query)
+        query = self._apply_unknown_or_null_filter(query)
 
         return query
 
@@ -134,7 +135,10 @@ class MongoTransformer(Transformer):
 
     def known_op_rhs(self, arg):
         # known_op_rhs: IS ( KNOWN | UNKNOWN )
-        return {"$exists": arg[1] == "KNOWN"}
+        # The OPTIMADE spec also required a type comparison with null, this must be post-processed
+        # so here we use a special key "#known" which will get replaced in post-processing with the
+        # expanded dict
+        return {"#known": arg[1] == "KNOWN"}
 
     def fuzzy_string_op_rhs(self, arg):
         # fuzzy_string_op_rhs: CONTAINS value | STARTS [ WITH ] value | ENDS [ WITH ] value
@@ -389,6 +393,55 @@ class MongoTransformer(Transformer):
             filter_, check_for_entry_type, replace_with_relationship
         )
 
+    def _apply_unknown_or_null_filter(self, filter_: dict) -> dict:
+        """ This method loops through the query and replaces the check for
+        KNOWN with a check for existence and a check for not null, and the
+        inverse for UNKNOWN.
+
+        """
+
+        def check_for_known_filter(_, expr):
+            """ Find cases where the query dict looks like
+            `{"field": {"#known": T/F}}` or
+            `{"field": "$not": {"#known": T/F}}`, which is a magic word
+            for KNOWN/UNKNOWN filters in this transformer.
+
+            """
+            return isinstance(expr, dict) and (
+                "#known" in expr or "#known" in expr.get("$not", {})
+            )
+
+        def replace_known_filter_with_or(subdict, prop, expr):
+            """ Replace #known and $not->#known parsed filters with the appropriate
+            combination of $exists and/or $eq/$ne null.
+
+            """
+            not_ = set(expr.keys()) == {"$not"}
+            if not_:
+                expr = expr["$not"]
+
+            exists = expr["#known"] ^ not_
+
+            top_level_key = "$or"
+            comparison_operator = "$eq"
+            if exists:
+                top_level_key = "$and"
+                comparison_operator = "$ne"
+
+            if top_level_key not in subdict:
+                subdict[top_level_key] = []
+
+            subdict[top_level_key].append({prop: {"$exists": exists}})
+            subdict[top_level_key].append({prop: {comparison_operator: None}})
+
+            subdict.pop(prop)
+
+            return subdict
+
+        return recursive_postprocessing(
+            filter_, check_for_known_filter, replace_known_filter_with_or
+        )
+
 
 def recursive_postprocessing(filter_, condition, replacement):
     """ Recursively descend into the query, checking each dictionary


Test patch:
diff --git a/tests/filtertransformers/test_mongo.py b/tests/filtertransformers/test_mongo.py
index 63c20212..d8305dc8 100644
--- a/tests/filtertransformers/test_mongo.py
+++ b/tests/filtertransformers/test_mongo.py
@@ -598,15 +598,44 @@ class TestMongoTransformer(unittest.TestCase):
             },
         )
 
-    def test_properties(self):
+    def test_known_properties(self):
         #  Filtering on Properties with unknown value
-        # TODO: {'$not': {'$exists': False}} can be simplified to {'$exists': True}
-        # The { $not: { $gt: 1.99 } } is different from the $lte operator. { $lte: 1.99 } returns only the documents
-        # where price field exists and its value is less than or equal to 1.99.
-        # Remember that the $not operator only affects other operators and cannot check fields and documents
-        # independently. So, use the $not operator for logical disjunctions and the $ne operator to test
-        # the contents of fields directly.
-        # source: https://docs.mongodb.com/manual/reference/operator/query/not/
+        self.assertEqual(
+            self.transform("chemical_formula_anonymous IS UNKNOWN"),
+            {
+                "$or": [
+                    {"chemical_formula_anonymous": {"$exists": False}},
+                    {"chemical_formula_anonymous": {"$eq": None}},
+                ]
+            },
+        )
+        self.assertEqual(
+            self.transform("chemical_formula_anonymous IS KNOWN"),
+            {
+                "$and": [
+                    {"chemical_formula_anonymous": {"$exists": True}},
+                    {"chemical_formula_anonymous": {"$ne": None}},
+                ]
+            },
+        )
+        self.assertEqual(
+            self.transform("NOT chemical_formula_anonymous IS UNKNOWN"),
+            {
+                "$and": [
+                    {"chemical_formula_anonymous": {"$exists": True}},
+                    {"chemical_formula_anonymous": {"$ne": None}},
+                ]
+            },
+        )
+        self.assertEqual(
+            self.transform("NOT chemical_formula_anonymous IS KNOWN"),
+            {
+                "$or": [
+                    {"chemical_formula_anonymous": {"$exists": False}},
+                    {"chemical_formula_anonymous": {"$eq": None}},
+                ]
+            },
+        )
 
         self.assertEqual(
             self.transform(
@@ -614,8 +643,40 @@ class TestMongoTransformer(unittest.TestCase):
             ),
             {
                 "$and": [
-                    {"chemical_formula_hill": {"$exists": True}},
-                    {"chemical_formula_anonymous": {"$not": {"$exists": False}}},
+                    {
+                        "$and": [
+                            {"chemical_formula_hill": {"$exists": True}},
+                            {"chemical_formula_hill": {"$ne": None}},
+                        ]
+                    },
+                    {
+                        "$and": [
+                            {"chemical_formula_anonymous": {"$exists": True}},
+                            {"chemical_formula_anonymous": {"$ne": None}},
+                        ]
+                    },
+                ]
+            },
+        )
+
+        self.assertEqual(
+            self.transform(
+                "chemical_formula_hill IS KNOWN AND chemical_formula_anonymous IS UNKNOWN"
+            ),
+            {
+                "$and": [
+                    {
+                        "$and": [
+                            {"chemical_formula_hill": {"$exists": True}},
+                            {"chemical_formula_hill": {"$ne": None}},
+                        ]
+                    },
+                    {
+                        "$or": [
+                            {"chemical_formula_anonymous": {"$exists": False}},
+                            {"chemical_formula_anonymous": {"$eq": None}},
+                        ]
+                    },
                 ]
             },
         )