HyukjinKwon commented on code in PR #59445:
URL: https://github.com/apache/spark/pull/59445#discussion_r4280655926
##########
sql/core/src/test/scala/org/apache/spark/sql/SubquerySuite.scala:
##########
@@ -78,6 +78,42 @@ class SubquerySuite extends SharedSparkSession
assert(joins.size == numJoins)
}
+ test("preserve sorted LIMIT and OFFSET with correlated subqueries") {
Review Comment:
**Nit (P3):** This test is the only coverage for the new `canPush` guard,
and it catches the bug only indirectly. Without the fix, the rows come out in
hash-partition order (ids 3,5,4,1,2,6 with the suite's 5 shuffle partitions),
so each case misses the expected rows by one row. A change to partitioning or
join selection could hide a reverted guard. SQL `LIMIT`/`OFFSET` also never
produce a `Project` directly over `LocalLimit`, so removing the guard from that
arm would leave every test green.
Could you add `comparePlans` cases to
`PushProjectionThroughLimitAndOffsetSuite`? They should show that a `Project`
with a correlated scalar or EXISTS subquery stays above `LocalLimit`,
`GlobalLimit(LocalLimit)`, and `Offset`, while a subquery-free deterministic
`Project` is still pushed. This end-to-end test can stay as it is.
**Verification:**
- **Regression:** Removing the subquery check from any one of the three
match arms makes a plan-level assertion fail, regardless of shuffle
partitioning, AQE, or join selection.
- **Behavior:** A deterministic subquery-free Project is still pushed below
LocalLimit, GlobalLimit(LocalLimit), and Offset.
--
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]
