[SPARK-59044][SQL] Handle OFFSET 0 in physical planning when EliminateOffsets is excluded - #58520
Open
hemanthboyina wants to merge 1 commit into
Open
Conversation
…eOffsets is excluded
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
1 participant
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?
OFFSET 0 produces a logical Offset(0, child) node. The EliminateOffsets optimizer rule normally removes it (offset 0 is a no-op), so physical planning never sees it. But EliminateOffsets is an excludable
rule, so when it is disabled via spark.sql.optimizer.excludedRules, the Offset(0) node survives into planning, where it becomes CollectLimitExec(limit = -1, offset = 0) (or GlobalLimitExec(limit = -1, offset
= 0)), which fails the operator's assertion.
This PR makes the physical planner handle a leftover Offset(0) directly: since it is a no-op, it is planned as its child. This is added in both offset planning paths — SpecialLimits (terminal) and
BasicOperators (non-terminal) — so the resulting plan is identical to the plan produced when EliminateOffsets removes the node.
Why are the changes needed?
Excluding an optimization rule should only affect performance, never break correctness. Today, excluding EliminateOffsets and running a query with OFFSET 0 fails during physical planning:
$ spark-sql --conf spark.sql.optimizer.excludedRules=org.apache.spark.sql.catalyst.optimizer.EliminateOffsets
EliminateOffsets is not in nonExcludableRules, so it is legitimately excludable and the planner must tolerate the un-optimized node.
Does this PR introduce any user-facing change?
Yes. Previously, running a query containing OFFSET 0 with EliminateOffsets excluded threw an AssertionError during physical planning. Now the query succeeds and returns the correct result (OFFSET 0 is a
no-op). With default settings there is no behavior change.
How was this patch tested?
Added a unit test in SQLQuerySuite (SPARK-59044: OFFSET 0 succeeds when EliminateOffsets is in excludedRules) that excludes the rule and verifies both planning paths: SELECT 1 AS x OFFSET 0 (terminal /
CollectLimitExec) and a non-terminal OFFSET 0 subquery (GlobalLimitExec). Both return the expected rows. The test fails before the fix and passes after.
Was this patch authored or co-authored using generative AI tooling?
No