uros-b commented on code in PR #56595:
URL: https://github.com/apache/spark/pull/56595#discussion_r3443042703


##########
sql/core/src/main/scala/org/apache/spark/sql/execution/joins/SortMergeAsOfJoinExec.scala:
##########
@@ -389,35 +369,29 @@ private[joins] class SortMergeAsOfJoinScanner(
           result != null && result.asInstanceOf[Boolean]
         }
         if (residualSatisfied) {
-          val distance = boundOrderExpr.eval(joinedRow)
-          if (distance != null) {
-            if (bestMatch == null) {
-              bestMatch = rightRow
-              bestDistance = distance
-            } else if (distanceOrdering.lt(distance, bestDistance)) {
-              bestMatch = rightRow
-              bestDistance = distance
-            } else {
-              return bestMatch
-            }
-          }
+          // Last match wins (closest right.t to left.t)
+          bestMatch = rightRow.copy()
         }
       } else if (bestMatch != null) {
+        // as-of condition transitioned true -> false (monotone for Backward).
+        // No further rows can satisfy it.
         return bestMatch
       }
-      i -= 1
     }
     bestMatch
   }
 
-  /** Scan from start to end (optimal for Forward/Nearest joins). */
-  private def findBestLeftToRight(leftRow: InternalRow): InternalRow = {
+  /**
+   * Forward scan for Forward/Nearest joins: distance-based termination.
+   * Stop when distance starts increasing past the minimum found so far.
+   */
+  private def findBestForwardNearest(leftRow: InternalRow): InternalRow = {
     var bestMatch: InternalRow = null
     var bestDistance: Any = null
+    val iter = rightGroupBuffer.generateIterator()

Review Comment:
   Please investigate this performance concern: 
findBestBackwardForward/findBestForwardNearest call 
rightGroupBuffer.generateIterator() once per left row, re-scanning the full 
right group each time.
   
   When the group is large enough to spill (the exact skewed-key case this PR 
targets), every left row triggers a full disk re-read via UnsafeExternalSorter, 
giving O(leftGroupSize x rightGroupDiskScan) I/O.
   
   The old ArrayBuffer re-scan stayed in memory. No cursor/window reuse and no 
benchmark covering a spilled group, so the targeted scenario could regress in 
wall-clock/I/O.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to