[GitHub] yucai commented on a change in pull request #21149: [SPARK-24076][SQL] Use different seed in HashAggregate to avoid hash conflict

[GitBox <gi...@apache.org>]

yucai commented on a change in pull request #21149: [SPARK-24076][SQL] Use different seed in HashAggregate to avoid hash conflict
URL: https://github.com/apache/spark/pull/21149#discussion_r257445460
 
 

 ##########
 File path: sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/HashAggregateExec.scala
 ##########
 @@ -755,7 +755,10 @@ case class HashAggregateExec(
     }
 
     // generate hash code for key
-    val hashExpr = Murmur3Hash(groupingExpressions, 42)
+    // SPARK-24076: HashAggregate uses the same hash algorithm on the same expressions
+    // as ShuffleExchange, it may lead to bad hash conflict when shuffle.partitions=8192*n,
+    // pick a different seed to avoid this conflict
+    val hashExpr = Murmur3Hash(groupingExpressions, 48)
 
 Review comment:
   @cloud-fan you mean `unsafeRowKeys.hashCode()`, right?
   I think it is a good idea, unsafe row has [null bit set] etc., the result should be different, we don't need weird `48` also. Do you want me to create a followup PR?

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

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