cloud-fan commented on code in PR #56595:
URL: https://github.com/apache/spark/pull/56595#discussion_r3449092461
##########
sql/core/src/test/scala/org/apache/spark/sql/SortMergeAsOfJoinSuite.scala:
##########
@@ -637,4 +637,37 @@ class SortMergeAsOfJoinSuite extends QueryTest
)
)
}
+
+ test("backward join - spill to disk") {
Review Comment:
Nice to see a forced-spill test. It covers the Backward + Inner paths (both
`bufferAllRight` and `bufferRightGroup`), which exercises
`findBestBackwardForward`'s `.copy()` over the reused `SpillableArrayIterator`
row.
The Forward/Nearest directions and the LeftOuter null-fill path aren't
exercised under spill, though: `findBestForwardNearest` has its own
`rightRow.copy()` over the same reused iterator, and nothing forces a spill on
those paths today. Extending this test (or adding a sibling) to force a spill
for `direction = "forward"`/`"nearest"` and for a LeftOuter join would lock in
spill-path correctness for all directions, not just Backward.
This follows up @uros-b's and @peter-toth's earlier asks about spill
coverage — the backward case is now covered; this is the remaining gap.
--
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]
