Merge pull request #6814 from davidegrohmann/2.2-fix-npe-nice-hasher

Fix NPEs in NiceHasher and NiceHasherValue

community/cypher/cypher-compiler-2.2/src/main/scala/org/neo4j/cypher/internal/compiler/v2_2/pipes/NiceHasher.scala

@@ -20,26 +20,42 @@
 package org.neo4j.cypher.internal.compiler.v2_2.pipes
 
 class NiceHasher(val original: Seq[Any]) {
-  override def equals(p1: Any): Boolean = {
-    if(p1 == null || !p1.isInstanceOf[NiceHasher])
-      return false
+  override def equals(p1: Any): Boolean = p1 match {
+    case null => false
+    case other: NiceHasher =>
+      hash == other.hash && NiceHasherValue.nullSafeEquals(comparableValues, other.comparableValues)
+    case _ => false
+  }
+
+  lazy val comparableValues = if (original == null) null else original.map(NiceHasherValue.comparableValuesFun)
+
+  override def toString = hashCode() + " : " + original
 
-    val other = p1.asInstanceOf[NiceHasher]
+  lazy val hash = NiceHasherValue.seqHashFun(original)
+
+  override def hashCode() = hash
+}
 
-    hash == other.hash && comparableValues.equals(other.comparableValues)
+class NiceHasherValue(val original: Any) {
+  override def equals(p1: Any): Boolean = p1 match {
+    case null => false
+    case other: NiceHasherValue =>
+      hash == other.hash && NiceHasherValue.nullSafeEquals(comparableValue, other.comparableValue)
+    case _ => false
   }
 
-  lazy val comparableValues = original.map(NiceHasherValue.comparableValuesFun)
+  lazy val comparableValue = NiceHasherValue.comparableValuesFun(original)
 
-  override def toString = hashCode() + " : " + original.toString
+  override def toString = hashCode() + " : " + original
 
-  lazy val hash = NiceHasherValue.seqHashFun(original)
+  lazy val hash = NiceHasherValue.hashFun(original)
 
   override def hashCode() = hash
 }
 
 object NiceHasherValue {
-  def seqHashFun(seq: Seq[Any]): Int = seq.foldLeft(0) ((hashValue, element) => hashFun(element) + hashValue * 31 )
+  def seqHashFun(seq: Seq[Any]): Int = if (seq == null) 0
+  else seq.foldLeft(0) ((hashValue, element) => hashFun(element) + hashValue * 31 )
 
   def hashFun(y: Any): Int = y match {
     case x: Array[Int] => java.util.Arrays.hashCode(x)
@@ -58,23 +74,11 @@ object NiceHasherValue {
     case x: Map[String, _] => x.keys.toSeq ++ x.values.map(comparableValuesFun)
     case x => x
   }
-}
 
-class NiceHasherValue(val original: Any) {
-  override def equals(p1: Any): Boolean = {
-    if(p1 == null || !p1.isInstanceOf[NiceHasherValue])
-      return false
-
-    val other = p1.asInstanceOf[NiceHasherValue]
-
-    hash == other.hash && (comparableValue equals other.comparableValue)
-  }
-
-  lazy val comparableValue = NiceHasherValue.comparableValuesFun(original)
-
-  override def toString = hashCode() + " : " + original.toString
-
-  lazy val hash = NiceHasherValue.hashFun(original)
-
-  override def hashCode() = hash
+  def nullSafeEquals(me: Any, other: Any): Boolean =
+    if ( me == null ) {
+      other == null
+    } else {
+      me equals other
+    }
 }

community/cypher/cypher-compiler-2.2/src/test/scala/org/neo4j/cypher/internal/compiler/v2_2/pipes/NiceHasherTest.scala

@@ -30,4 +30,15 @@ class NiceHasherTest extends CypherFunSuite {
     hasher1 should equal(hasher2)
   }
 
+  test("should work when nice hasher wraps null") {
+    new NiceHasher(null) should equal(new NiceHasher(null))
+    new NiceHasher(Seq.empty) should not equal new NiceHasher(null)
+    new NiceHasher(null) should not equal new NiceHasher(Seq.empty)
+  }
+
+  test("should work when nice hasher wraps seq of nulls") {
+    val hasher1 = new NiceHasher(Seq(null, null, null))
+    val hasher2 = new NiceHasher(Seq(null, null, null))
+    hasher1 should equal(hasher2)
+  }
 }

community/cypher/cypher-compiler-2.2/src/test/scala/org/neo4j/cypher/internal/compiler/v2_2/pipes/NiceHasherValueTest.scala

@@ -0,0 +1,31 @@
+/*
+ * Copyright (c) 2002-2016 "Neo Technology,"
+ * Network Engine for Objects in Lund AB [http://neotechnology.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Neo4j is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <http://www.gnu.org/licenses/>.
+ */
+package org.neo4j.cypher.internal.compiler.v2_2.pipes
+
+import org.neo4j.cypher.internal.commons.CypherFunSuite
+
+class NiceHasherValueTest extends CypherFunSuite {
+
+  test("should work when nice hasher wraps null") {
+    new NiceHasherValue(null) should equal(new NiceHasherValue(null))
+    new NiceHasherValue(new Object) should not equal new NiceHasherValue(null)
+    new NiceHasherValue(null) should not equal new NiceHasherValue(new Object)
+  }
+}

community/cypher/cypher/src/test/scala/org/neo4j/cypher/AggregationAcceptanceTest.scala

@@ -116,6 +116,18 @@ class AggregationAcceptanceTest extends ExecutionEngineFunSuite with NewPlannerT
     result.toList should equal (List(Map("count(distinct a.foo)" -> 0)))
   }
 
+  test("should be able to collect distinct nulls") {
+    val query = "unwind [NULL, NULL] AS x RETURN collect(distinct x) as c"
+    val result = executeWithAllPlanners(query)
+    result.toSeq.head shouldBe Map("c" -> List.empty)
+  }
+
+  test("should be able to collect distinct values mixed with nulls") {
+    val query = "unwind [NULL, 1, NULL] AS x RETURN collect(distinct x) as c"
+    val result = executeWithAllPlanners(query)
+    result.toSeq.head shouldBe Map("c" -> List(1))
+  }
+
   test("should aggregate on array values") {
     createNode("color" -> Array("red"))
     createNode("color" -> Array("blue"))

community/cypher/cypher/src/test/scala/org/neo4j/cypher/OptionalMatchAcceptanceTest.scala

@@ -205,4 +205,18 @@ class OptionalMatchAcceptanceTest extends ExecutionEngineFunSuite with NewPlanne
 
     assert(result.toList === List(Map("b" -> null, "r" -> null, "a" -> null)))
   }
+
+  test("optional match and collect should work") {
+    createLabeledNode(Map("property" -> 42), "DOES_EXIST")
+    createLabeledNode(Map("property" -> 43), "DOES_EXIST")
+    createLabeledNode(Map("property" -> 44), "DOES_EXIST")
+
+    val query = """OPTIONAL MATCH (f:DOES_EXIST)
+                  |OPTIONAL MATCH (n:DOES_NOT_EXIST)
+                  |RETURN collect(DISTINCT n.property), collect(DISTINCT f.property)""".stripMargin
+
+    val result = executeWithAllPlanners(query).toList
+
+    result.size should equal(1)
+  }
 }