This is an automated email from the ASF dual-hosted git repository.
cloud-fan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git
The following commit(s) were added to refs/heads/master by this push:
new dec7ce169369 [SPARK-57665][SQL] Fix slice() returning empty for large
length in the interpreted path
dec7ce169369 is described below
commit dec7ce1693692b8f151a2d43cc4b5e36db117b5c
Author: Sepuri Sai Krishna <[email protected]>
AuthorDate: Wed Aug 12 16:45:29 2026 +0800
[SPARK-57665][SQL] Fix slice() returning empty for large length in the
interpreted path
### What changes were proposed in this pull request?
This PR fixes `slice(array, start, length)` returning an empty array
(dropping all
elements) when `length` is large enough that `start_0based + length`
overflows a
32-bit int in the interpreted evaluation path.
`Slice.nullSafeEval` computed the result as:
data.slice(startIndex, startIndex + lengthInt)
For a large `length`, `startIndex + lengthInt` overflows to a negative
`until`, and
under Scala 2.13 `Seq.slice` with a negative `until` returns an empty
result, so the
whole tail is silently dropped. The codegen path already routes through
`ArrayExpressionUtils.sliceLength`, which clamps the length to the number
of elements
remaining after `startIndex`, so the interpreted and codegen paths
disagreed.
The fix routes the interpreted path through the same
`ArrayExpressionUtils.sliceLength`
helper that codegen uses (introduced in SPARK-57171). `sliceLength`
rejects a negative
length and clamps to `numElements - startIndex`, so `startIndex +
resLength` can no
longer overflow and both execution paths produce identical results.
### Why are the changes needed?
`slice` silently returns wrong results (data loss) instead of the correct
sub-array.
For constant arguments the wrong value is produced even by default,
because
`ConstantFolding` evaluates the expression through the interpreted
`eval()` at plan
time. Example on released Spark 4.0.3 (Scala 2.13.16):
spark.sql("SELECT slice(array(1,2,3,4,5,6), 2,
2147483647)").show(false)
-- returns [] (expected [2, 3, 4, 5, 6])
The interpreted path is also used when whole-stage codegen falls back at
runtime
(e.g. when the generated method exceeds
`spark.sql.codegen.hugeMethodLimit`), so the
wrong result can surface without any internal configuration being set.
(Spark 3.5 / Scala 2.12 is unaffected: 2.12's `slice` double-overflows
`until - lo`
back to a positive count and accidentally returns the correct elements.)
### Does this PR introduce _any_ user-facing change?
Yes. `slice` with a large `length` now returns the correct tail of the
array in the
interpreted path, matching the codegen path, instead of an empty array.
- Before (Spark 4.0+, interpreted/constant-folded):
`slice(array(1,2,3,4,5,6), 2, 2147483647)` => `[]`
- After:
`slice(array(1,2,3,4,5,6), 2, 2147483647)` => `[2, 3, 4, 5, 6]`
### How was this patch tested?
Added regression cases to `CollectionExpressionsSuite` ("Slice" test).
`checkEvaluation`
exercises both the interpreted and codegen paths, so the new cases fail
on the old code
(interpreted returned `[]`) and pass with the fix:
checkEvaluation(Slice(a0, Literal(2), Literal(Int.MaxValue)), Seq(2,
3, 4, 5, 6))
checkEvaluation(Slice(a0, Literal(1), Literal(Int.MaxValue)), Seq(1,
2, 3, 4, 5, 6))
Ran the full suite locally:
build/sbt 'catalyst/testOnly
org.apache.spark.sql.catalyst.expressions.CollectionExpressionsSuite'
-- Tests: succeeded 61, failed 0
Also reproduced the original bug on a released Spark 4.0.3 (Scala
2.13.16) `spark-shell`
and confirmed the corrected behavior with the fix.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Claude Opus 4.8)
Closes #56746 from
SEPURI-SAI-KRISHNA/SPARK-57665-slice-interpreted-overflow.
Authored-by: Sepuri Sai Krishna <[email protected]>
Signed-off-by: Wenchen Fan <[email protected]>
---
.../catalyst/expressions/collectionOperations.scala | 14 ++++++++++----
.../expressions/CollectionExpressionsSuite.scala | 19 +++++++++++++++++++
2 files changed, 29 insertions(+), 4 deletions(-)
diff --git
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
index 4a842f2c27f9..051c1b43664c 100644
---
a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
+++
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala
@@ -2160,16 +2160,22 @@ case class Slice(x: Expression, start: Expression,
length: Expression)
val lengthInt = lengthVal.asInstanceOf[Int]
val arr = xVal.asInstanceOf[ArrayData]
val startIndex = ArrayExpressionUtils.sliceStartIndex(startInt,
arr.numElements(), prettyName)
- if (lengthInt < 0) {
- throw
QueryExecutionErrors.unexpectedValueForLengthInFunctionError(prettyName,
lengthInt)
- }
+ // Resolve (and validate) the result length via the shared helper,
mirroring the codegen path.
+ // Besides rejecting a negative length, this clamps the length to the
elements remaining after
+ // `startIndex`. For an in-range `startIndex`, this clamp keeps
`startIndex + resLength` from
+ // overflowing `Int` -- the unclamped `startIndex + lengthInt` could wrap
negative and make
+ // `slice` drop all elements. An out-of-range `startIndex` (a large
negative `start`) can
+ // still wrap the helper's own `numElements - startIndex`, but the guard
below returns before
+ // `resLength` is used.
+ val resLength =
+ ArrayExpressionUtils.sliceLength(lengthInt, arr.numElements(),
startIndex, prettyName)
// startIndex can be negative if start is negative and its absolute value
is greater than the
// number of elements in the array
if (startIndex < 0 || startIndex >= arr.numElements()) {
return new GenericArrayData(Array.empty[AnyRef])
}
val data = arr.toSeq[AnyRef](elementType)
- new GenericArrayData(data.slice(startIndex, startIndex + lengthInt))
+ new GenericArrayData(data.slice(startIndex, startIndex + resLength))
}
override def doGenCode(ctx: CodegenContext, ev: ExprCode): ExprCode = {
diff --git
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
index ec4ee75d8acd..2ea0f3d8cdf4 100644
---
a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
+++
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala
@@ -923,6 +923,25 @@ class CollectionExpressionsSuite
checkEvaluation(Slice(a1, Literal(1), Literal(2)), Seq("a", "b"))
checkEvaluation(Slice(a2, Literal(1), Literal(2)), Seq("", null))
checkEvaluation(Slice(a0, Literal(10), Literal(1)), Seq.empty[Int])
+ // SPARK-57665: a large length must not overflow startIndex + length; both
the interpreted and
+ // codegen paths must return the tail of the array, not an empty array.
+ checkEvaluation(Slice(a0, Literal(2), Literal(Int.MaxValue)), Seq(2, 3, 4,
5, 6))
+ checkEvaluation(Slice(a0, Literal(1), Literal(Int.MaxValue)), Seq(1, 2, 3,
4, 5, 6))
+ // Negative start with a large length exercises the start < 0 branch
together with clamping.
+ checkEvaluation(Slice(a0, Literal(-2), Literal(Int.MaxValue)), Seq(5, 6))
+ // Both extremes: an Int.MinValue start resolves far below 0, so the guard
returns empty and
+ // the (harmlessly overflowing) resolved length is never used.
+ checkEvaluation(Slice(a0, Literal(Int.MinValue), Literal(Int.MaxValue)),
Seq.empty[Int])
+ // A negative length is still rejected when the start is out of range,
because the length is
+ // resolved before the start guard.
+ checkErrorInExpression[SparkRuntimeException](
+ expression = Slice(a0, Literal(-20), Literal(-1)),
+ condition = "INVALID_PARAMETER_VALUE.LENGTH",
+ parameters = Map(
+ "parameter" -> toSQLId("length"),
+ "length" -> (-1).toString,
+ "functionName" -> toSQLId("slice")
+ ))
checkEvaluation(Slice(a1, Literal(10), Literal(1)), Seq.empty[String])
checkEvaluation(Slice(a3, Literal(2), Literal(3)), Seq(2, null, 4))
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
