Repository navigation
fix: preserve unchanged VALUES schemas during binding [skip ci] - #175
Merged
osipovartem merged 1 commit intoNov 1, 2026
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Which issue does this PR close?
Follow-up to #174. Issues are disabled in this fork; the reproducible bug and
regression evidence are described below. Base:
df55-physical-udf-instanceatab14407b730762126717039a77eed0a91ef75420.Rationale for this change
VALUES can initially retain a canonical Arrow logical field, then lose it during
an unrelated
with_param_valuescall. Parameter replacement unconditionallyre-infers VALUES from its expressions, even if nothing was substituted. With
Parquet child field IDs, a type-only row coercion is not a substitute for the
supplied logical field: the extension name and declared timestamp precision
disappear. Rustice subsequently rejects that untagged struct at its existing
schema-aware gate.
What changes are included in this PR?
parameter traversal. Keep the full original VALUES schema when unchanged.
native traversal order, recursion protection, parent schema recomputation,
PREPARE verification, and DML/UNION validation.
precision 9 versus row casts at 6; NULL and plain-struct controls; empty and
unrelated maps; PREPARE; projections; direct and subquery substitutions.
There is no additional whole-plan traversal, blind metadata overlay, per-row
value conversion, type-gate relaxation, or new dependency.
Are these changes tested?
Local Rust 1.97.0, offline/locked qualification:
production implementation. The corrected source is restored byte-for-byte.
datafusion-exprlibrary: 268 passed.10,752 passed, 0 failed, 10 ignored. The custom SQLLogicTest runner also exits 0.
dev/rust_lint.sh: exit 0, including strict workspace Clippy, formatting,TOML, licenses, typos, pinned Prettier and rustdoc.
cargo clippy --all-targets --all-features --profile ci --offline --locked -- -D warnings: exit 0.Focused performance screen
Four alternating fresh-process pairs, one guest CPU, identical frozen harness,
aligned dependency locks, recursive protection enabled, five warmups and twenty
samples per case/process. Source cloning and result validation/drop are outside
timing. Every timed output is checked for typed values, dimensions, names,
nullability and the declared metadata expectation.
These are medians of per-process medians in a local unoptimized CI profile, not
SQL, release, Iceberg, HTTP or SPCS latency. All four equivalent-output medians
meet the predeclared 5% regression budget. The one-row binding pairs include
an adverse +48.79% sample; it is retained, not discarded, so this does not prove
zero overhead or statistical non-regression. Annotated baseline output is wrong
and receives no equivalent-output speedup claim.
The valid cohort contains 960 timed samples and 240 warmups across eight
zero-exit processes. An earlier cohort stopped at a validator error: it did not
recognize DataFusion's required
$1name-preserving alias. Its failed receiptand binaries are retained separately; it contributes no performance result.
The harness runner's nine negative/control tests pass normally and under
python -O, including timeout receipts with partial output.Are there any user-facing changes?
Unchanged parameter binding no longer strips supplied VALUES field metadata.
Actual substitutions still determine the resulting schema. Public API signatures
are unchanged. This fork fix does not itself update Rustice's dependency pin or
claim downstream connector qualification.