Commit 55ddfe3
[SPARK-56567][SQL] Fix array_insert
### What changes were proposed in this pull request?
This PR fixes `array_insert` to raise the documented `COLLECTION_SIZE_LIMIT_EXCEEDED.FUNCTION` error when called with `pos = Int.MinValue`, instead of leaking an internal JVM exception. Below is a minimalistic repro for the bug:
```
from pyspark.sql import SparkSession
spark = SparkSession.builder.appName("ArrayInsertIntMinTest").getOrCreate()
def run(query):
print(query)
try:
spark.sql(query).show()
except Exception as e:
print(f"> {type(e).__name__}: {str(e).splitlines()[0]}")
print()
print("Correctly throws COLLECTION_SIZE_LIMIT_EXCEEDED.FUNCTION")
run("SELECT array_insert(array(1), -2147483647, 3)")
print("Incorrectly throws ArrayIndexOutOfBoundsException")
run("SELECT array_insert(array(1), -2147483648, 3)")
spark.stop()
```
`array_insert` should raise `SparkRuntimeException` with condition `COLLECTION_SIZE_LIMIT_EXCEEDED.FUNCTION` here. However, it instead crashes with an internal JVM exception: `java.lang.AssertionError` from `UnsafeArrayData.setInt(-2147483646, ...)` under whole-stage codegen, or `java.lang.ArrayIndexOutOfBoundsException: Index -2147483646 out of bounds for length 2` in interpreted mode. This is because `ArrayInsert` does `int` arithmetic on `pos` (e.g. `-pos`, `java.lang.Math.abs(pos)`), which silently wraps back to `Int.MinValue`. The `newPosExtendsArrayLeft` check therefore returns the wrong answer, the code takes the wrong branch, never reaches the `MAX_ROUNDED_ARRAY_LENGTH` guard, and computes a garbage index that crashes later.
The fix is to widen `pos` arithmetic to `Long` in both `ArrayInsert.nullSafeEval` (interpreted) and `ArrayInsert.doGenCode` (codegen). The existing `MAX_ROUNDED_ARRAY_LENGTH` check then correctly catches the oversized result on the right branch.
### Why are the changes needed?
There is currently an error-reporting bug. `array_insert(arr, Int.MinValue, item)` fails with an internal JVM exception (`AssertionError` or `ArrayIndexOutOfBoundsException`) instead of the documented `COLLECTION_SIZE_LIMIT_EXCEEDED.FUNCTION` error.
### Does this PR introduce _any_ user-facing change?
Yes, for `pos = Int.MinValue` with a non-empty input array, `array_insert` now throws `SparkRuntimeException` with condition `COLLECTION_SIZE_LIMIT_EXCEEDED.FUNCTION` instead of leaking an internal `AssertionError` / `ArrayIndexOutOfBoundsException`. This matches the behavior already in place for neighboring positions like `Int.MinValue + 1`.
### How was this patch tested?
Added unit tests to `CollectionExpressionsSuite` to validate correct error reporting when `pos = Int.MinValue`.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: claude-4.7-opus-high
Closes apache#55476 from aknayar/array-insert-pos-fix.
Authored-by: Akash Nayar <akashknayar5@gmail.com>
Signed-off-by: Wenchen Fan <wenchen@databricks.com>Int.MinValue pos overflow1 parent f519b2d commit 55ddfe3
2 files changed
Lines changed: 45 additions & 18 deletions
File tree
- sql/catalyst/src
- main/scala/org/apache/spark/sql/catalyst/expressions
- test/scala/org/apache/spark/sql/catalyst/expressions
Lines changed: 27 additions & 18 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
5126 | 5126 | | |
5127 | 5127 | | |
5128 | 5128 | | |
5129 | | - | |
5130 | | - | |
| 5129 | + | |
| 5130 | + | |
| 5131 | + | |
5131 | 5132 | | |
5132 | 5133 | | |
5133 | 5134 | | |
5134 | | - | |
| 5135 | + | |
5135 | 5136 | | |
5136 | 5137 | | |
5137 | | - | |
| 5138 | + | |
5138 | 5139 | | |
5139 | 5140 | | |
5140 | 5141 | | |
5141 | 5142 | | |
5142 | | - | |
| 5143 | + | |
5143 | 5144 | | |
5144 | 5145 | | |
5145 | 5146 | | |
5146 | 5147 | | |
5147 | 5148 | | |
5148 | 5149 | | |
5149 | | - | |
| 5150 | + | |
5150 | 5151 | | |
5151 | 5152 | | |
5152 | 5153 | | |
5153 | | - | |
| 5154 | + | |
| 5155 | + | |
5154 | 5156 | | |
5155 | 5157 | | |
5156 | 5158 | | |
5157 | 5159 | | |
5158 | 5160 | | |
5159 | 5161 | | |
5160 | 5162 | | |
5161 | | - | |
5162 | | - | |
5163 | | - | |
5164 | | - | |
| 5163 | + | |
| 5164 | + | |
| 5165 | + | |
| 5166 | + | |
5165 | 5167 | | |
5166 | 5168 | | |
5167 | | - | |
| 5169 | + | |
5168 | 5170 | | |
5169 | 5171 | | |
5170 | 5172 | | |
5171 | 5173 | | |
5172 | 5174 | | |
5173 | 5175 | | |
5174 | | - | |
| 5176 | + | |
| 5177 | + | |
5175 | 5178 | | |
5176 | 5179 | | |
5177 | 5180 | | |
| |||
5238 | 5241 | | |
5239 | 5242 | | |
5240 | 5243 | | |
| 5244 | + | |
| 5245 | + | |
| 5246 | + | |
| 5247 | + | |
5241 | 5248 | | |
| 5249 | + | |
5242 | 5250 | | |
5243 | 5251 | | |
5244 | 5252 | | |
5245 | 5253 | | |
5246 | 5254 | | |
5247 | | - | |
| 5255 | + | |
5248 | 5256 | | |
5249 | 5257 | | |
5250 | 5258 | | |
5251 | | - | |
| 5259 | + | |
5252 | 5260 | | |
5253 | | - | |
5254 | | - | |
| 5261 | + | |
| 5262 | + | |
5255 | 5263 | | |
5256 | | - | |
| 5264 | + | |
5257 | 5265 | | |
| 5266 | + | |
5258 | 5267 | | |
5259 | 5268 | | |
5260 | 5269 | | |
| |||
Lines changed: 18 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
3279 | 3279 | | |
3280 | 3280 | | |
3281 | 3281 | | |
| 3282 | + | |
| 3283 | + | |
| 3284 | + | |
| 3285 | + | |
| 3286 | + | |
| 3287 | + | |
| 3288 | + | |
| 3289 | + | |
| 3290 | + | |
| 3291 | + | |
| 3292 | + | |
| 3293 | + | |
| 3294 | + | |
| 3295 | + | |
| 3296 | + | |
| 3297 | + | |
| 3298 | + | |
| 3299 | + | |
3282 | 3300 | | |
0 commit comments