[SPARK-58411][SQL] Preserve ORDER BY in LIMIT decorrelation when correlation references only the outer table - #57614
Open
AveryQi115 wants to merge 2 commits into
Open
[SPARK-58411][SQL] Preserve ORDER BY in LIMIT decorrelation when correlation references only the outer table#57614AveryQi115 wants to merge 2 commits into
AveryQi115 wants to merge 2 commits into
Conversation
…elation references only the outer table
### What changes were proposed in this pull request?
In `DecorrelateInnerQuery`, the `Limit` case peels the `Sort` off the subquery
and saves its ordering. When the correlated predicate references only outer
columns, decorrelation produces no partitioning key (`partitionFields.isEmpty`)
and that branch re-emits the limit as `Limit(k, newChild)` /
`Limit(k, Offset(m, newChild))` without re-applying the saved ordering, turning
`ORDER BY ... LIMIT k` into an arbitrary `LIMIT k`.
This PR re-applies the saved ordering as a global `Sort` below the limit in the
`partitionFields.isEmpty` branch (covering both `LIMIT` and `LIMIT ... OFFSET`),
so ordered limits stay order-preserving and deterministic. A new internal flag
`spark.sql.optimizer.decorrelateLimitOffsetLegacyIncorrectOrderHandling.enabled`
(default false) can restore the legacy behavior.
The standalone `Offset` case (no `LIMIT`) is not affected: it decorrelates the
original input with the `Sort` retained, so its ordering is already preserved.
### Why are the changes needed?
The dropped `ORDER BY` makes an ordered-limit subquery return a non-deterministic
row. For example:
```
SELECT t1a, t1b FROM t1
WHERE t1c = (SELECT t2c FROM t2 WHERE t1b < t1d ORDER BY t2c LIMIT 1);
```
With distinct `t2c` values `{12, 16, NULL}` and Spark's default `ASC NULLS FIRST`,
the correct ordered `LIMIT 1` returns `NULL`, so the row is correctly excluded.
With the bug, an arbitrary row is picked, which can incorrectly emit a row and
varies by physical execution order.
### Does this PR introduce any user-facing change?
Yes. Correlated scalar/predicate subqueries of the form `ORDER BY ... LIMIT`
whose correlation references only the outer table now return correct,
deterministic results. Results that previously depended on the dropped ordering
may change. The legacy behavior can be restored via the config above.
### How was this patch tested?
New unit tests in `DecorrelateInnerQuerySuite` covering default ordering,
explicit `ASC NULLS LAST`, `LIMIT ... OFFSET`, and the legacy-flag path.
Golden-file results for `scalar-subquery-predicate.sql` are updated accordingly.
### Was this patch authored or co-authored using generative AI tooling?
Yes, drafted with assistance from Claude.
Contributor
Author
|
@agubichev @srielau @cloud-fan for review. Plz let me know for such correctness issues, do we add flags to guard it? Do we need to backport the fix? Would like some suggestions for such fix since I'm not familiar with the process. TY |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changes were proposed in this pull request?
In
DecorrelateInnerQuery, theLimitcase peels theSortoff the subquery and saves its ordering. When the correlated predicate references only outer columns, decorrelation produces no partitioning key (partitionFields.isEmpty) and that branch re-emits the limit asLimit(k, newChild)/Limit(k, Offset(m, newChild))without re-applying the saved ordering, turningORDER BY ... LIMIT kinto an arbitraryLIMIT k.This PR re-applies the saved ordering as a global
Sortbelow the limit in thepartitionFields.isEmptybranch (covering bothLIMITandLIMIT ... OFFSET), so ordered limits stay order-preserving and deterministic. A new internal flagspark.sql.optimizer.decorrelateLimitOffsetLegacyIncorrectOrderHandling.enabled(default false) can restore the legacy behavior.The standalone
Offsetcase (noLIMIT) is not affected: it decorrelates the original input with theSortretained, so its ordering is already preserved.Why are the changes needed?
The dropped
ORDER BYmakes an ordered-limit subquery return a non-deterministic row. For example:With distinct
t2cvalues{12, 16, NULL}and Spark's defaultASC NULLS FIRST, the correct orderedLIMIT 1returnsNULL, so the row is correctly excluded. With the bug, an arbitrary row is picked, which can incorrectly emit a row and varies by physical execution order.ground truth verification
Does this PR introduce any user-facing change?
Yes. Correlated scalar/predicate subqueries of the form
ORDER BY ... LIMITwhose correlation references only the outer table now return correct, deterministic results. Results that previously depended on the dropped ordering may change. The legacy behavior can be restored via the config above.How was this patch tested?
New unit tests in
DecorrelateInnerQuerySuitecovering default ordering, explicitASC NULLS LAST,LIMIT ... OFFSET, and the legacy-flag path. Golden-file results forscalar-subquery-predicate.sqlare updated accordingly.Was this patch authored or co-authored using generative AI tooling?
Yes, drafted with assistance from Claude.