Skip to content

Commit 1094e68

Browse files
SEPURI-SAI-KRISHNAuros-b
authored andcommitted
[SPARK-58821][SQL] Return the recomputed length instead of an internal error in Sequence
### What changes were proposed in this pull request? `Sequence.sequenceLength` computes the sequence length in `Long` and falls back to `BigInt` when that arithmetic overflows. The fallback recomputes the length exactly, raises `COLLECTION_SIZE_LIMIT_EXCEEDED` if it is too large to allocate, and then throws `internalError("Unreachable code reached.")` on the assumption that no other outcome is possible. That assumption does not hold. `Math.subtractExact(stop, start)` overflows whenever `stop - start` exceeds the `Long` range, which says nothing about the length, because the length also depends on `step`. For a large step the exact length is small, no limit error is raised, and control reaches the `internalError` with the correct length sitting unused in `safeLen`. This PR returns `safeLen.toInt` instead of throwing. The check immediately above already bounds `safeLen` by `ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH`, and the caller has already rejected boundaries whose step points the wrong way, so the value is positive and the narrowing conversion is exact. The now-unused `SparkException.internalError` import is removed. No other behaviour changes: an overflow with a length that really is too large still raises `COLLECTION_SIZE_LIMIT_EXCEEDED.PARAMETER` from the same fallback. ### Why are the changes needed? `sequence()` over BIGINT raises an `INTERNAL_ERROR` (SQLSTATE XX000) for queries that have a small, well-defined result. `INTERNAL_ERROR` is reserved for conditions that indicate a bug in Spark, so surfacing it for ordinary user input is wrong on both counts: the query should have succeeded, and the error tells the user to report a bug rather than fix their query. ```sql SELECT sequence(-9223372036854775808L, 9223372036854775807L, 9223372036854775807L); -- [INTERNAL_ERROR] Unreachable code reached. SQLSTATE: XX000 ``` The element-filling code already handles steps of this magnitude correctly, so only the length computation was at fault: `sequence(-4611686018427387904L, 4611686018427387903L, 4611686018427387903L)`, whose endpoints are close enough together to avoid the overflow, returns `[-4611686018427387904, -1, 4611686018427387902]` on master today. `sequenceLength` was introduced in this shape by SPARK-43393 (`afc4c49927cb`), which fixed a real correctness bug where the `Long` length computation overflowed silently and `sequence` returned an empty array. The overflow cases that fix was written against all had huge lengths, which is why the tail looked unreachable. The method has not changed since, so every release containing SPARK-43393 is affected. ### Does this PR introduce _any_ user-facing change? Yes. Queries that failed with `INTERNAL_ERROR` now return their result. Both the interpreted and the codegen path are affected, since both call `Sequence.sequenceLength`. Before: ```sql SELECT sequence(-9223372036854775808L, 9223372036854775807L, 9223372036854775807L); -- [INTERNAL_ERROR] Unreachable code reached. SQLSTATE: XX000 SELECT sequence(-9223372036854775808L, 0L, 4611686018427387904L); -- [INTERNAL_ERROR] Unreachable code reached. SQLSTATE: XX000 ``` After: ```sql SELECT sequence(-9223372036854775808L, 9223372036854775807L, 9223372036854775807L); -- [-9223372036854775808, -1, 9223372036854775806] SELECT sequence(-9223372036854775808L, 0L, 4611686018427387904L); -- [-9223372036854775808, -4611686018427387904, 0] ``` No query that previously succeeded changes its result, and no query that previously raised `COLLECTION_SIZE_LIMIT_EXCEEDED` stops raising it. Since the previous behaviour was an internal error, no migration guide entry is needed. ### How was this patch tested? Three cases added to the `Sequence of numbers` test in `CollectionExpressionsSuite`, next to the existing SPARK-43393 overflow cases. `checkEvaluation` exercises both the interpreted and the codegen path: * a positive step at both `Long` extremes, * a positive step where only `stop - start` overflows, * the negative-step mirror. `build/sbt 'catalyst/testOnly *CollectionExpressionsSuite'` passes (62 tests). The existing SPARK-43393 cases, which cover the fallback still raising the limit error, pass unchanged. Also verified end to end against a local `SparkSession`, including a non-foldable form reading the arguments from a temp view, to confirm the fix applies at runtime and not only through constant folding. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Code (Claude Opus 5) Closes #58047 from SEPURI-SAI-KRISHNA/SPARK-58821-sequence-length-internal-error. Authored-by: Sepuri Sai Krishna <saik20533@gmail.com> Signed-off-by: Uros Bojanic <221401595+uros-b@users.noreply.github.com> (cherry picked from commit de60883) Signed-off-by: Uros Bojanic <221401595+uros-b@users.noreply.github.com>
1 parent 00d3243 commit 1094e68

2 files changed

Lines changed: 22 additions & 3 deletions

File tree

sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/collectionOperations.scala

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,6 @@ import scala.collection.mutable
2323
import scala.reflect.ClassTag
2424

2525
import org.apache.spark.{QueryContext, SparkException, SparkIllegalArgumentException}
26-
import org.apache.spark.SparkException.internalError
2726
import org.apache.spark.sql.catalyst.InternalRow
2827
import org.apache.spark.sql.catalyst.analysis.{TypeCheckResult, TypeCoercion, UnresolvedAttribute, UnresolvedSeed}
2928
import org.apache.spark.sql.catalyst.analysis.TypeCheckResult.DataTypeMismatch
@@ -3420,14 +3419,20 @@ object Sequence {
34203419
}
34213420
len.toInt
34223421
} catch {
3423-
// We handle overflows in the previous try block by raising an appropriate exception.
3422+
// An overflow in the previous try block does not by itself mean the sequence is too long:
3423+
// `stop - start` can exceed the `Long` range while a large `step` still yields only a few
3424+
// elements. Recompute the length exactly and reject it only if it really cannot be
3425+
// allocated, otherwise return it.
34243426
case _: ArithmeticException =>
34253427
val safeLen =
34263428
BigInt(1) + (BigInt(stop) - BigInt(start)) / BigInt(step)
34273429
if (safeLen > ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH) {
34283430
throw QueryExecutionErrors.createArrayWithElementsExceedLimitError(prettyName, safeLen)
34293431
}
3430-
throw internalError("Unreachable code reached.")
3432+
// The check above bounds `safeLen` by `MAX_ROUNDED_ARRAY_LENGTH`, and the caller has
3433+
// already rejected boundaries whose step points the wrong way, so `safeLen` is positive
3434+
// and `toInt` is exact.
3435+
safeLen.toInt
34313436
case e: Exception => throw e
34323437
}
34333438
}

sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/expressions/CollectionExpressionsSuite.scala

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1137,6 +1137,20 @@ class CollectionExpressionsSuite
11371137
"maxRoundedArrayLength" -> ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH.toString(),
11381138
"parameter" -> toSQLId("count")))
11391139

1140+
// SPARK-58821: an overflow in the `Long` length computation does not by itself mean the
1141+
// sequence is too long, because the length also depends on the step. For a large step the
1142+
// result has only a few elements, so the exact length recomputed in `BigInt` has to be
1143+
// returned rather than treated as an unreachable case.
1144+
checkEvaluation(
1145+
new Sequence(Literal(Long.MinValue), Literal(Long.MaxValue), Literal(Long.MaxValue)),
1146+
Seq(Long.MinValue, -1L, Long.MaxValue - 1))
1147+
checkEvaluation(
1148+
new Sequence(Literal(Long.MinValue), Literal(0L), Literal(4611686018427387904L)),
1149+
Seq(Long.MinValue, -4611686018427387904L, 0L))
1150+
checkEvaluation(
1151+
new Sequence(Literal(Long.MaxValue), Literal(Long.MinValue), Literal(-Long.MaxValue)),
1152+
Seq(Long.MaxValue, 0L, -Long.MaxValue))
1153+
11401154
// test sequence with one element (zero step or equal start and stop)
11411155

11421156
checkEvaluation(new Sequence(Literal(1), Literal(1), Literal(-1)), Seq(1))

0 commit comments

Comments
 (0)