You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by we...@apache.org on 2022/09/06 13:15:48 UTC

[spark] branch branch-3.1 updated: [SPARK-40315][SQL] Add hashCode() for Literal of ArrayBasedMapData

This is an automated email from the ASF dual-hosted git repository.

wenchen pushed a commit to branch branch-3.1
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-3.1 by this push:
     new 63025e952e8 [SPARK-40315][SQL] Add hashCode() for Literal of ArrayBasedMapData
63025e952e8 is described below

commit 63025e952e8452d23df4eb01dd5099745f595bee
Author: Carmen Kwan <ca...@databricks.com>
AuthorDate: Tue Sep 6 21:11:24 2022 +0800

    [SPARK-40315][SQL] Add hashCode() for Literal of ArrayBasedMapData
    
    ### What changes were proposed in this pull request?
    There is no explicit `hashCode()` function override for `ArrayBasedMapData`. As a result, there is a non-deterministic error where the `hashCode()` computed for `Literal`s of `ArrayBasedMapData` can be different for two equal objects (`Literal`s of `ArrayBasedMapData` with equal keys and values).
    
    In this PR, we add a `hashCode` function so that it works exactly as we expect.
    
    ### Why are the changes needed?
    This is a bug fix for a non-deterministic error. It is also more consistent with the rest of Spark if we implement the `hashCode` method instead of relying on defaults. We can't add the `hashCode` directly to `ArrayBasedMapData` because of SPARK-9415.
    
    ### Does this PR introduce _any_ user-facing change?
    No
    
    ### How was this patch tested?
    A simple unit test was added.
    
    Closes #37807 from c27kwan/SPARK-40315-lit.
    
    Authored-by: Carmen Kwan <ca...@databricks.com>
    Signed-off-by: Wenchen Fan <we...@databricks.com>
    (cherry picked from commit e85a4ffbdfa063c8da91b23dfbde77e2f9ed62e9)
    Signed-off-by: Wenchen Fan <we...@databricks.com>
---
 .../spark/sql/catalyst/expressions/literals.scala  |  3 +++
 .../catalyst/expressions/ComplexTypeSuite.scala    | 26 ++++++++++++++++++++++
 2 files changed, 29 insertions(+)

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/literals.scala b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/literals.scala
index 810cecff379..e3296829430 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/literals.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/literals.scala
@@ -309,6 +309,9 @@ case class Literal (value: Any, dataType: DataType) extends LeafExpression {
     val valueHashCode = value match {
       case null => 0
       case binary: Array[Byte] => util.Arrays.hashCode(binary)
+      // SPARK-40315: Literals of ArrayBasedMapData should have deterministic hashCode.
+      case arrayBasedMapData: ArrayBasedMapData =>
+        arrayBasedMapData.keyArray.hashCode() * 37 + arrayBasedMapData.valueArray.hashCode()
       case other => other.hashCode()
     }
     31 * Objects.hashCode(dataType) + valueHashCode
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ComplexTypeSuite.scala b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ComplexTypeSuite.scala
index 3d6f6937e78..96147b4b3e8 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ComplexTypeSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/ComplexTypeSuite.scala
@@ -526,4 +526,30 @@ class ComplexTypeSuite extends SparkFunSuite with ExpressionEvalHelper {
 
     assert(m1.semanticEquals(m2))
   }
+
+  test("SPARK-40315: Literals of ArrayBasedMapData should have deterministic hashCode.") {
+    val keys = new Array[UTF8String](1)
+    val values1 = new Array[UTF8String](1)
+    val values2 = new Array[UTF8String](1)
+
+    keys(0) = UTF8String.fromString("key")
+    values1(0) = UTF8String.fromString("value1")
+    values2(0) = UTF8String.fromString("value2")
+
+    val d1 = new ArrayBasedMapData(new GenericArrayData(keys), new GenericArrayData(values1))
+    val d2 = new ArrayBasedMapData(new GenericArrayData(keys), new GenericArrayData(values1))
+    val d3 = new ArrayBasedMapData(new GenericArrayData(keys), new GenericArrayData(values2))
+    val m1 = Literal.create(d1, MapType(StringType, StringType))
+    val m2 = Literal.create(d2, MapType(StringType, StringType))
+    val m3 = Literal.create(d3, MapType(StringType, StringType))
+
+    // If two Literals of ArrayBasedMapData have the same elements, we expect them to be equal and
+    // to have the same hashCode().
+    assert(m1 == m2)
+    assert(m1.hashCode() == m2.hashCode())
+    // If two Literals of ArrayBasedMapData have different elements, we expect them not to be equal
+    // and to have different hashCode().
+    assert(m1 != m3)
+    assert(m1.hashCode() != m3.hashCode())
+  }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org