Skip to content

fix: guard array_position start_from underflow#22291

Open
Sean-Kenneth-Doherty wants to merge 1 commit into
apache:mainfrom
Sean-Kenneth-Doherty:codex/array-position-min-start
Open

fix: guard array_position start_from underflow#22291
Sean-Kenneth-Doherty wants to merge 1 commit into
apache:mainfrom
Sean-Kenneth-Doherty:codex/array-position-min-start

Conversation

@Sean-Kenneth-Doherty
Copy link
Copy Markdown

Which issue does this PR close?

Rationale for this change

array_position converts the optional 1-indexed start_from argument to a 0-indexed offset by subtracting one. When start_from is i64::MIN, that subtraction can underflow and panic before DataFusion can return a normal error.

What changes are included in this PR?

  • Adds a shared checked conversion for array_position start_from values.
  • Uses it in both scalar and array start_from paths.
  • Adds regression coverage for the i64::MIN boundary.

Are these changes tested?

  • cargo fmt --all
  • TMPDIR=/home/sean/Projects/datafusion-array-position-min/target/tmp cargo test -p datafusion-functions-nested position::tests::test_array_position_start_from_min_value -- --nocapture
  • TMPDIR=/home/sean/Projects/datafusion-array-position-min/target/tmp cargo test --profile=ci --test sqllogictests -- array/array_position.slt
  • TMPDIR=/home/sean/Projects/datafusion-array-position-min/target/tmp cargo clippy --all-targets --all-features -- -D warnings
  • git diff --check

I also manually checked the issue reproducer no longer panics: EXPLAIN SELECT array_position([1], 1, -9223372036854775808); now returns a plan, while plain execution returns Execution error: start_from out of bounds: -9223372036854775808.

Are there any user-facing changes?

Invalid array_position start_from values at the i64::MIN boundary now return a normal DataFusion error instead of panicking.

@github-actions github-actions Bot added sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation labels May 17, 2026
@Sean-Kenneth-Doherty
Copy link
Copy Markdown
Author

Fresh validation on the PR head (862c36ee8):

  • TMPDIR=/home/sean/Projects/datafusion-array-position-min/target/tmp cargo test -p datafusion-functions-nested position::tests::test_array_position_start_from_min_value -- --nocapture -> passed (1 passed)
  • TMPDIR=/home/sean/Projects/datafusion-array-position-min/target/tmp cargo test --profile=ci --test sqllogictests -- array/array_position.slt -> passed (1/1 files completed)
  • cargo fmt --all -- --check -> passed
  • git diff --check origin/main...HEAD -> clean

GitHub Actions only ran the labeler for this PR, so this keeps the regression evidence visible while the branch waits for review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

functions Changes to functions implementation sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

panic: array_position underflows start_from at i64::MIN

1 participant