parthchandra commented on code in PR #1424:
URL: https://github.com/apache/datafusion-comet/pull/1424#discussion_r1964726051


##########
spark/src/main/scala/org/apache/comet/rules/RewriteJoin.scala:
##########
@@ -67,4 +78,21 @@ object RewriteJoin extends JoinSelectionHelper {
       }
     case _ => plan
   }
+
+  def getOptimalBuildSide(join: Join): BuildSide = {
+    val leftSize = join.left.stats.sizeInBytes
+    val rightSize = join.right.stats.sizeInBytes
+    val leftRowCount = join.left.stats.rowCount
+    val rightRowCount = join.right.stats.rowCount
+    if (leftSize == rightSize && rightRowCount.isDefined && 
leftRowCount.isDefined) {
+      if (rightRowCount.get <= leftRowCount.get) {

Review Comment:
   > Do we have any sort of cardinality stats (possibly with some catalog 
implementations)?
   With cbo enabled, we might have cardinality information. Column stats 
require a catalog to keep the metadata and stats so with just plain Parquet 
files, we may not have the column stats information. With iceberg, there is 
some work that has been done to incorporate column stats into the iceberg table 
but I'm not sure where the effort to use them in Spark is.
   Ref: 
https://github.com/apache/spark/blob/f37be893d01884461ac515c8b197fb30d9ba68ff/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/plans/logical/Statistics.scala#L101
 
   
   >  I'm envisioning a scenario where the right table has fewer rows but much 
higher cardinality so the resulting hash table is bigger.
   
   The hash table might have more keys but total size is still a very good 
metric simply because a larger size hash table might not fit into memory. Also, 
depending on the  implementation, the larger the size of the data in the hash 
table, the less cache friendly it might be and we may end up with _slower_ 
performance.
   For reference, Spark uses 
https://github.com/apache/spark/blob/master/core/src/main/java/org/apache/spark/unsafe/map/BytesToBytesMap.java
 as the fallback hash table implementation (afaik).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org

Reply via email to