You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by ur...@apache.org on 2026/09/08 14:48:44 UTC

(spark) branch branch-4.3 updated: [SPARK-58821][SQL] Return the recomputed length instead of an internal error in Sequence

This is an automated email from the ASF dual-hosted git repository.

uros-b pushed a commit to branch branch-4.3
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-4.3 by this push:
     new 4116a7e449b8 [SPARK-58821][SQL] Return the recomputed length instead of an internal error in Sequence
4116a7e449b8 is described below

commit 4116a7e449b84ac18641b5f52ea18d319706c205
Author: Sepuri Sai Krishna <sa...@gmail.com>
AuthorDate: Tue Sep 8 14:48:13 2026 +0000

    [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 <sa...@gmail.com>
    Signed-off-by: Uros Bojanic <22...@users.noreply.github.com>
    (cherry picked from commit de60883859d4427544def5c78a47124b07d5c40f)
    Signed-off-by: Uros Bojanic <22...@users.noreply.github.com>
---
 .../sql/catalyst/expressions/collectionOperations.scala    | 11 ++++++++---
 .../catalyst/expressions/CollectionExpressionsSuite.scala  | 14 ++++++++++++++
 2 files changed, 22 insertions(+), 3 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 93c6c1b17863..03bc15bfc705 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
@@ -23,7 +23,6 @@ import scala.collection.mutable
 import scala.reflect.ClassTag
 
 import org.apache.spark.{QueryContext, SparkException, SparkIllegalArgumentException}
-import org.apache.spark.SparkException.internalError
 import org.apache.spark.sql.catalyst.InternalRow
 import org.apache.spark.sql.catalyst.analysis.{TypeCheckResult, TypeCoercion, UnresolvedAttribute, UnresolvedSeed}
 import org.apache.spark.sql.catalyst.analysis.TypeCheckResult.DataTypeMismatch
@@ -3640,14 +3639,20 @@ object Sequence {
       }
       len.toInt
     } catch {
-      // We handle overflows in the previous try block by raising an appropriate exception.
+      // An overflow in the previous try block does not by itself mean the sequence is too long:
+      // `stop - start` can exceed the `Long` range while a large `step` still yields only a few
+      // elements. Recompute the length exactly and reject it only if it really cannot be
+      // allocated, otherwise return it.
       case _: ArithmeticException =>
         val safeLen =
           BigInt(1) + (BigInt(stop) - BigInt(start)) / BigInt(step)
         if (safeLen > ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH) {
           throw QueryExecutionErrors.createArrayWithElementsExceedLimitError(prettyName, safeLen)
         }
-        throw internalError("Unreachable code reached.")
+        // The check above bounds `safeLen` by `MAX_ROUNDED_ARRAY_LENGTH`, and the caller has
+        // already rejected boundaries whose step points the wrong way, so `safeLen` is positive
+        // and `toInt` is exact.
+        safeLen.toInt
       case e: Exception => throw e
     }
   }
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 2ea0f3d8cdf4..0f2d84044c33 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
@@ -1183,6 +1183,20 @@ class CollectionExpressionsSuite
         "maxRoundedArrayLength" -> ByteArrayMethods.MAX_ROUNDED_ARRAY_LENGTH.toString(),
         "parameter" -> toSQLId("count")))
 
+    // SPARK-58821: an overflow in the `Long` length computation does not by itself mean the
+    // sequence is too long, because the length also depends on the step. For a large step the
+    // result has only a few elements, so the exact length recomputed in `BigInt` has to be
+    // returned rather than treated as an unreachable case.
+    checkEvaluation(
+      new Sequence(Literal(Long.MinValue), Literal(Long.MaxValue), Literal(Long.MaxValue)),
+      Seq(Long.MinValue, -1L, Long.MaxValue - 1))
+    checkEvaluation(
+      new Sequence(Literal(Long.MinValue), Literal(0L), Literal(4611686018427387904L)),
+      Seq(Long.MinValue, -4611686018427387904L, 0L))
+    checkEvaluation(
+      new Sequence(Literal(Long.MaxValue), Literal(Long.MinValue), Literal(-Long.MaxValue)),
+      Seq(Long.MaxValue, 0L, -Long.MaxValue))
+
     // test sequence with one element (zero step or equal start and stop)
 
     checkEvaluation(new Sequence(Literal(1), Literal(1), Literal(-1)), Seq(1))


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org