This is an automated email from the ASF dual-hosted git repository.
uros-b pushed a commit to branch branch-4.x
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/branch-4.x by this push:
new b2d8fb7d97a2 [SPARK-59044][SQL] Handle OFFSET 0 in physical planning
when EliminateOffsets is excluded
b2d8fb7d97a2 is described below
commit b2d8fb7d97a249955779f2fa890d4f7d0d468f0e
Author: hemanthboyina <[email protected]>
AuthorDate: Mon Oct 5 13:16:11 2026 +0000
[SPARK-59044][SQL] Handle OFFSET 0 in physical planning when
EliminateOffsets is excluded
**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
> SELECT 1 AS x OFFSET 0;
java.lang.AssertionError: assertion failed
at
org.apache.spark.sql.execution.CollectLimitExec.<init>(limit.scala:49)
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
Closes #58520 from hemanthboyina/SPARK-59044-offset-zero-planning.
Authored-by: hemanthboyina <[email protected]>
Signed-off-by: Uros Bojanic <[email protected]>
(cherry picked from commit 596bc42c461776077abb5492fd2c067858a40d0b)
Signed-off-by: Uros Bojanic <[email protected]>
---
.../org/apache/spark/sql/execution/SparkStrategies.scala | 10 ++++++++++
.../test/scala/org/apache/spark/sql/SQLQuerySuite.scala | 16 +++++++++++++++-
2 files changed, 25 insertions(+), 1 deletion(-)
diff --git
a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
index 4d48bed992fd..f36c902f8259 100644
---
a/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
+++
b/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkStrategies.scala
@@ -115,6 +115,11 @@ abstract class SparkStrategies extends
QueryPlanner[SparkPlan] {
CollectLimitExec(limit = offset + limit, child = planLater(child),
offset = offset)
case Limit(IntegerLiteral(limit), child) =>
CollectLimitExec(limit = limit, child = planLater(child))
+ case logical.Offset(IntegerLiteral(0), child) =>
+ // OFFSET 0 is a no-op. It is normally removed by the
EliminateOffsets optimizer rule,
+ // but that rule is excludable, so handle it defensively here to
avoid constructing a
+ // CollectLimitExec with no limit and a zero offset (which fails its
assertion).
+ planLater(child)
case logical.Offset(IntegerLiteral(offset), child) =>
CollectLimitExec(child = planLater(child), offset = offset)
case Tail(IntegerLiteral(limit), child) =>
@@ -1243,6 +1248,11 @@ abstract class SparkStrategies extends
QueryPlanner[SparkPlan] {
execution.LocalLimitExec(limit, planLater(child)) :: Nil
case logical.GlobalLimit(IntegerLiteral(limit), child) =>
execution.GlobalLimitExec(limit, planLater(child)) :: Nil
+ case logical.Offset(IntegerLiteral(0), child) =>
+ // OFFSET 0 is a no-op; see the note in SpecialLimits. Excluding
EliminateOffsets leaves
+ // the Offset node in place, so avoid building a GlobalLimitExec with
no limit and a zero
+ // offset (which fails its assertion).
+ planLater(child) :: Nil
case logical.Offset(IntegerLiteral(offset), child) =>
GlobalLimitExec(child = planLater(child), offset = offset) :: Nil
case union: logical.Union =>
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
b/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
index 25184f52b219..2aa37ba2fa6e 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/SQLQuerySuite.scala
@@ -35,7 +35,7 @@ import org.apache.spark.sql.catalyst.ExtendedAnalysisException
import org.apache.spark.sql.catalyst.expressions.{CodegenObjectFactoryMode,
GenericRow, Hex}
import org.apache.spark.sql.catalyst.expressions.Cast._
import org.apache.spark.sql.catalyst.expressions.aggregate.{Complete, Partial}
-import org.apache.spark.sql.catalyst.optimizer.{ConvertToLocalRelation,
NestedColumnAliasingSuite, RewriteWithExpression}
+import org.apache.spark.sql.catalyst.optimizer.{ConvertToLocalRelation,
EliminateOffsets, NestedColumnAliasingSuite, RewriteWithExpression}
import org.apache.spark.sql.catalyst.parser.ParseException
import org.apache.spark.sql.catalyst.plans.logical.{LocalLimit, Project,
RepartitionByExpression, Sort}
import org.apache.spark.sql.connector.catalog.CatalogManager
@@ -5355,6 +5355,20 @@ class SQLQuerySuite extends SharedSparkSession with
AdaptiveSparkPlanHelper
Row(false))
}
}
+
+ test("SPARK-59044: OFFSET 0 succeeds when EliminateOffsets is in
excludedRules") {
+ // EliminateOffsets normally removes an OFFSET 0 (a no-op) before physical
planning. When the
+ // rule is excluded the Offset node survives; before the fix this failed
the assertion in
+ // CollectLimitExec/GlobalLimitExec during physical planning.
+ withSQLConf(SQLConf.OPTIMIZER_EXCLUDED_RULES.key ->
EliminateOffsets.ruleName) {
+ // Collected to the driver -> SpecialLimits -> CollectLimitExec path.
+ checkAnswer(sql("SELECT 1 AS x OFFSET 0"), Row(1))
+ // Non-terminal OFFSET 0 -> BasicOperators -> GlobalLimitExec path.
+ checkAnswer(
+ sql("SELECT * FROM (SELECT id FROM range(3) OFFSET 0) ORDER BY id"),
+ Seq(Row(0), Row(1), Row(2)))
+ }
+ }
}
case class Foo(bar: Option[String])
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
