Skip to content

Guard insert placeholder zero#22299

Open
Sean-Kenneth-Doherty wants to merge 1 commit into
apache:mainfrom
Sean-Kenneth-Doherty:codex/insert-placeholder-zero
Open

Guard insert placeholder zero#22299
Sean-Kenneth-Doherty wants to merge 1 commit into
apache:mainfrom
Sean-Kenneth-Doherty:codex/insert-placeholder-zero

Conversation

@Sean-Kenneth-Doherty
Copy link
Copy Markdown

@Sean-Kenneth-Doherty Sean-Kenneth-Doherty commented May 17, 2026

Which issue does this PR close?

Fixes #22224.

Rationale for this change

EXPLAIN INSERT INTO t VALUES ($0) currently panics while inferring parameter types for INSERT VALUES. Other SQL paths already reject $0 as an invalid placeholder index, and the INSERT inference path should do the same instead of subtracting one from zero.

What changes are included in this PR?

  • Adds a zero-index guard before INSERT VALUES placeholder numbers are converted to parameter vector positions.
  • Aligns the INSERT VALUES error with the existing invalid-placeholder planning error.
  • Adds a standalone SQL logic regression for EXPLAIN INSERT INTO ... VALUES ($0).

Are these changes tested?

Yes.

  • cargo fmt --check --all
  • cargo test -p datafusion-sqllogictest --test sqllogictests -- insert_values_placeholders
  • cargo test -p datafusion-sql --test sql_integration test_insert_schema_errors -> 6 passed
  • git diff --check origin/main...HEAD
  • git diff --cached --check

Are there any user-facing changes?

Yes. INSERT ... VALUES ($0) now returns a planning error instead of panicking.

Scope note

The change is limited to INSERT VALUES placeholder type inference and the new regression coverage. I did not find any broader placeholder semantics that needed to change.

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

Local validation on current head (f15210789):

  • cargo test -p datafusion-sqllogictest --test sqllogictests -- insert_values_placeholders -> completed 1/1 files
  • cargo test -p datafusion-sql --test sql_integration test_insert_schema_errors -> 6 passed
  • cargo test -p datafusion-sql --test sql_integration params -> 35 passed
  • cargo fmt --all --check -> passed
  • cargo clippy -p datafusion-sql --all-targets -- -D warnings -> passed
  • git diff --check origin/main...HEAD && git diff --check -> clean

The local checkout is clean after validation.

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

Labels

sql SQL Planner sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

panic: INSERT VALUES placeholder $0 underflows parameter index inference

1 participant