fix NPE at compareNumber() when filtering index query (#612)

fix #611

Change-Id: I20d841a85f82fb120244834538dc89c0d8f5ce27

README.md

@@ -1,8 +1,8 @@
 # HugeGraph
 
 [![License](https://img.shields.io/badge/license-Apache%202-0E78BA.svg)](https://www.apache.org/licenses/LICENSE-2.0.html)
-[![Build Status](https://travis-ci.org/hugegraph/hugegraph.svg?branch=master)](https://travis-ci.org/hugegraph/hugegraph)
-[![codecov](https://codecov.io/gh/hugegraph/hugegraph/branch/master/graph/badge.svg)](https://codecov.io/gh/hugegraph/hugegraph)
+[![Build Status](https://travis-ci.org/hugegraph/hugegraph.svg?branch=release-0.9)](https://travis-ci.org/hugegraph/hugegraph)
+[![codecov](https://codecov.io/gh/hugegraph/hugegraph/branch/release-0.9/graph/badge.svg)](https://codecov.io/gh/hugegraph/hugegraph)
 
 HugeGraph is a fast-speed and highly-scalable [graph database](https://en.wikipedia.org/wiki/Graph_database). Billions of vertices and edges can be easily stored into and queried from HugeGraph due to its excellent OLTP ability. As compliance to [Apache TinkerPop 3](https://tinkerpop.apache.org/) framework, various complicated graph queries can be accomplished through [Gremlin](https://tinkerpop.apache.org/gremlin.html)(a powerful graph traversal language).
 

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/query/Condition.java

@@ -35,6 +35,7 @@ import com.baidu.hugegraph.structure.HugeElement;
 import com.baidu.hugegraph.structure.HugeProperty;
 import com.baidu.hugegraph.type.define.HugeKeys;
 import com.baidu.hugegraph.util.Bytes;
+import com.baidu.hugegraph.util.DateUtil;
 import com.baidu.hugegraph.util.E;
 import com.baidu.hugegraph.util.NumericUtil;
 import com.google.common.collect.ImmutableList;
@@ -58,18 +59,25 @@ public abstract class Condition {
         LTE("<=", (v1, v2) -> { return compare(v1, v2) <= 0; }),
         NEQ("!=", (v1, v2) -> { return compare(v1, v2) != 0; }),
         IN("in", (v1, v2) -> {
+            assert v2 != null;
             return ((Collection<?>) v2).contains(v1);
         }),
         NOT_IN("notin", (v1, v2) -> {
+            assert v2 != null;
             return !((Collection<?>) v2).contains(v1);
         }),
         PREFIX("prefix", (v1, v2) -> {
+            assert v2 != null;
             return Bytes.prefixWith(((Id) v2).asBytes(), ((Id) v1).asBytes());
         }),
         TEXT_CONTAINS("textcontains", (v1, v2) -> {
-            return ((String) v1).contains((String) v2);
+            return v1 != null && ((String) v1).contains((String) v2);
         }),
         TEXT_CONTAINS_ANY("textcontainsany", (v1, v2) -> {
+            assert v2 != null;
+            if (v1 == null) {
+                return false;
+            }
             @SuppressWarnings("unchecked")
             Collection<String> words = (Collection<String>) v2;
             for (String word : words) {
@@ -80,10 +88,12 @@ public abstract class Condition {
             return false;
         }),
         CONTAINS("contains", (v1, v2) -> {
-            return ((Map<?, ?>) v1).containsValue(v2);
+            assert v2 != null;
+            return v1 != null && ((Map<?, ?>) v1).containsValue(v2);
         }),
         CONTAINS_KEY("containskey", (v1, v2) -> {
-            return ((Map<?, ?>) v1).containsKey(v2);
+            assert v2 != null;
+            return v1 != null && ((Map<?, ?>) v1).containsKey(v2);
         }),
         SCAN("scan", (v1, v2) -> true);
 
@@ -108,6 +118,7 @@ public abstract class Condition {
          */
         protected static boolean equals(final Object first,
                                         final Object second) {
+            assert second != null;
             if (first == null) {
                 return second == null;
             } else if (first instanceof Id) {
@@ -134,8 +145,10 @@ public abstract class Condition {
          *         numerically greater than second.
          */
         protected static int compare(final Object first, final Object second) {
+            assert second != null;
             if (second instanceof Number) {
-                return NumericUtil.compareNumber(first, (Number) second);
+                return NumericUtil.compareNumber(first == null ? 0 : first,
+                                                 (Number) second);
             } else if (second instanceof Date) {
                 return compareDate(first, (Date) second);
             } else {
@@ -145,6 +158,9 @@ public abstract class Condition {
         }
 
         protected static int compareDate(Object first, Date second) {
+            if (first == null) {
+                first = DateUtil.DATE_ZERO;
+            }
             if (first instanceof Date) {
                 return ((Date) first).compareTo(second);
             } else {
@@ -595,6 +611,14 @@ public abstract class Condition {
         public boolean test(HugeElement element) {
             HugeProperty<?> prop = element.getProperty(this.key());
             Object value = prop != null ? prop.value() : null;
+            if (value == null) {
+                /*
+                 * Fix #611
+                 * TODO: It is possible some scenes cannot be returned false
+                 * directly, such as NEQ, null != 123 should be true.
+                 */
+                return false;
+            }
             return this.relation.test(value, this.value);
         }
 

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/tx/GraphIndexTransaction.java

@@ -27,6 +27,7 @@ import java.util.HashSet;
 import java.util.Iterator;
 import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
 import java.util.function.Function;
 import java.util.stream.Collectors;
@@ -1136,7 +1137,7 @@ public class GraphIndexTransaction extends AbstractTransaction {
                         tx.doEliminate(serializer.writeIndex(index));
                         tx.commit();
                         // If deleted by error, re-add deleted index again
-                        if (this.deletedByError(q, element)) {
+                        if (this.deletedByError(query, element)) {
                             tx.doAppend(serializer.writeIndex(index));
                             tx.commit();
                         } else {
@@ -1221,12 +1222,12 @@ public class GraphIndexTransaction extends AbstractTransaction {
                     // It's inside/between Query (processed in range index)
                     return null;
                 }
-                Object propValue = errorElem.getProperty(key).value();
-                Object conditionValue = conditionValues.iterator().next();
-                if (!propValue.equals(conditionValue)) {
+                HugeProperty<?> prop = element.getProperty(key);
+                Object errorValue = conditionValues.iterator().next();
+                if (prop == null || !Objects.equals(prop.value(), errorValue)) {
                     PropertyKey pkey = this.graph().propertyKey(key);
-                    errorElem.addProperty(pkey, conditionValue);
-                    incorrectPKs.put(pkey, conditionValue);
+                    errorElem.addProperty(pkey, errorValue);
+                    incorrectPKs.put(pkey, errorValue);
                 }
             }
             return errorElem;

hugegraph-core/src/main/java/com/baidu/hugegraph/backend/tx/GraphTransaction.java

@@ -474,6 +474,11 @@ public class GraphTransaction extends IndexableTransaction {
     }
 
     public Iterator<Vertex> queryVertices(Query query) {
+        E.checkArgument(this.removedVertexes.isEmpty() ||
+                        query.limit() == Query.NO_LIMIT,
+                        "It's not allowed to query with limit when " +
+                        "there are uncommitted delete records.");
+
         Iterator<HugeVertex> results = this.queryVerticesFromBackend(query);
 
         // Filter unused or incorrect records
@@ -604,6 +609,11 @@ public class GraphTransaction extends IndexableTransaction {
     }
 
     public Iterator<Edge> queryEdges(Query query) {
+        E.checkArgument(this.removedEdges.isEmpty() ||
+                        query.limit() == Query.NO_LIMIT,
+                        "It's not allowed to query with limit when " +
+                        "there are uncommitted delete records.");
+
         Iterator<HugeEdge> results = this.queryEdgesFromBackend(query);
 
         // TODO: any unconsidered case, maybe the query with OR condition?
@@ -1300,8 +1310,10 @@ public class GraphTransaction extends IndexableTransaction {
 
         // Filter backend record if it's updated in memory
         Iterator<V> backendResults = new FilterIterator<>(records, elem -> {
-            return (!txResults.contains(elem) &&
-                    !removedTxRecords.containsKey(elem.id()));
+            Id id = elem.id();
+            return !addedTxRecords.containsKey(id) &&
+                   !updatedTxRecords.containsKey(id) &&
+                   !removedTxRecords.containsKey(id);
         });
 
         return new ExtendableIterator<V>(txResults.iterator(), backendResults);

hugegraph-core/src/main/java/com/baidu/hugegraph/util/DateUtil.java

@@ -29,6 +29,8 @@ import com.google.common.collect.ImmutableMap;
 
 public final class DateUtil {
 
+    public static final Date DATE_ZERO = new Date(0L);
+
     private static final Map<String, String> VALID_DFS = ImmutableMap.of(
             "^\\d{4}-\\d{1,2}-\\d{1,2}",
             "yyyy-MM-dd",