Fix neq and null condition query behavior

Fixes JanusGraph#2205 Signed-off-by: Boxuan Li <[email protected]>
li-boxuan · Dec 6, 2020 · 9d5b2c0 · 9d5b2c0
1 parent 6a483bf
commit 9d5b2c0
Show file tree

Hide file tree

Showing 4 changed files with 53 additions and 9 deletions.
diff --git a/janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphIndexTest.java b/janusgraph-backend-testutils/src/main/java/org/janusgraph/graphdb/JanusGraphIndexTest.java
@@ -188,6 +188,49 @@ public static void assertGraphOfTheGods(JanusGraph graphOfTheGods) {
         graphOfTheGods.tx().commit();
     }
 
+    @Test
+    public void testNeqQuery() {
+        makeKey("p2", String.class);
+        PropertyKey p3 = makeKey("p3", String.class);
+        PropertyKey p4 = makeKey("p4", String.class);
+        mgmt.buildIndex("composite", Vertex.class).addKey(p3).buildCompositeIndex();
+        mgmt.buildIndex("mixed", Vertex.class).addKey(p4, Mapping.STRING.asParameter()).buildMixedIndex(INDEX);
+        finishSchema();
+
+        tx.addVertex();
+
+        // property not registered in schema
+        assertFalse(tx.traversal().V().has("p1", P.neq("v")).hasNext());
+        // property registered in schema
+        assertFalse(tx.traversal().V().has("p2", P.neq("v")).hasNext());
+        // property registered in schema and has composite index
+        assertFalse(tx.traversal().V().has("p3", P.neq("v")).hasNext());
+        // property registered in schema and has mixed index
+        assertFalse(tx.traversal().V().has("p4", P.neq("v")).hasNext());
+    }
+
+    @Test
+    public void testNullHasConditionQuery() {
+        makeKey("name", String.class);
+        makeKey("p2", String.class);
+        PropertyKey p3 = makeKey("p3", String.class);
+        PropertyKey p4 = makeKey("p4", String.class);
+        mgmt.buildIndex("composite", Vertex.class).addKey(p3).buildCompositeIndex();
+        mgmt.buildIndex("mixed", Vertex.class).addKey(p4, Mapping.STRING.asParameter()).buildMixedIndex(INDEX);
+        finishSchema();
+
+        tx.addVertex();
+
+        // property not registered in schema
+        assertFalse(tx.traversal().V().has("p1", (Object) null).hasNext());
+        // property registered in schema
+        assertFalse(tx.traversal().V().has("p2", (Object) null).hasNext());
+        // property registered in schema and has composite index
+        assertFalse(tx.traversal().V().has("p3", (Object) null).hasNext());
+        // property registered in schema and has mixed index
+        assertFalse(tx.traversal().V().has("p4", (Object) null).hasNext());
+    }
+
     /**
      * Ensure clearing storage actually removes underlying graph and index databases.
      * @throws Exception

diff --git a/janusgraph-core/src/main/java/org/janusgraph/graphdb/query/QueryUtil.java b/janusgraph-core/src/main/java/org/janusgraph/graphdb/query/QueryUtil.java
@@ -160,10 +160,6 @@ public static <E extends JanusGraphElement> And<E> constraints2QNF(StandardJanus
             final RelationType type = getType(tx, atom.getKey());
 
             if (type == null) {
-                if (atom.getPredicate() == Cmp.EQUAL && atom.getValue() == null ||
-                        (atom.getPredicate() == Cmp.NOT_EQUAL && atom.getValue() != null))
-                    continue; //Ignore condition, its trivially satisfied
-
                 return null;
             }
 

diff --git a/janusgraph-driver/src/main/java/org/janusgraph/core/attribute/Cmp.java b/janusgraph-driver/src/main/java/org/janusgraph/core/attribute/Cmp.java
@@ -41,8 +41,9 @@ public boolean isValidCondition(Object condition) {
 
         @Override
         public boolean test(Object value, Object condition) {
-            if (condition==null) {
-                return value==null;
+            if (condition == null) {
+                // since JanusGraph does not support null value, has(p, null) should always evaluate to false
+                return false;
             } else {
                 return condition.equals(value) || (condition.getClass().isArray() && ArrayUtils.isEquals(condition, value));
             }
@@ -73,10 +74,13 @@ public boolean isValidCondition(Object condition) {
 
         @Override
         public boolean test(Object value, Object condition) {
-            if (condition==null) {
-                return value!=null;
+            if (value == null) {
+                // To align with TinkerPop behaviour, if an element does not have property p, then has(p, neq(anything))
+                // should always evaluate to false. Note that JanusGraph does not support null value, so the "value == null"
+                // here implies the element does not have such property.
+                return false;
             } else {
-                return !condition.equals(value);
+                return !value.equals(condition);
             }
         }
 

diff --git a/janusgraph-test/src/test/java/org/janusgraph/graphdb/query/QueryTest.java b/janusgraph-test/src/test/java/org/janusgraph/graphdb/query/QueryTest.java
@@ -15,6 +15,7 @@
 package org.janusgraph.graphdb.query;
 
 import com.google.common.collect.Iterators;
+import org.apache.tinkerpop.gremlin.process.traversal.P;
 import org.apache.tinkerpop.gremlin.structure.Vertex;
 import org.janusgraph.core.*;
 import org.janusgraph.core.attribute.Contain;