Repository navigation
fix: preserve native CASE field metadata on the runtime branch - #172
Merged
osipovartem merged 2 commits intoOct 30, 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?
Backport the applicable CASE field-metadata corrections from Apache DataFusion
#26145 and
#26158.
The base is
df55-physical-udf-instanceat78e3e064e, which Rustice actuallypins, not this repository's default branch. This is a fork-specific adaptation,
not a cherry-pick of Apache's different metadata merge policy.
Rationale for this change
Logical and physical CASE inference previously dropped function result metadata
past depth 8. The logical fallback also discarded metadata for native expressions
outside its allowlist. This loses known type annotations without reporting an
unsupported expression or an error.
Strict workspace Clippy additionally exposed missing delegation of this fork's
CASE literal-matcher hook through
ScalarUDF::with_aliases. Aliases must preservethe function's matcher, fallback and errors, rather than silently disabling its
indexed dispatch. That correction is kept in a separate commit.
What changes are included in this PR?
expressions use native
to_field, with no depth classifier or structural rescans.branch's nullable-OR behavior.
type-only cast rules and physical NOT non-inheritance.
a typed unmarked binary/NOT first result. This fast path is valid for the fork's
strict merge policy; it is not applicable to Apache's shared-key merge rule.
Keep existing deep-binary and marked-NULL expectations intact.
and exact type/nullability/metadata preflight assertions.
Are these changes tested?
cargo fmt --all --checkandgit diff --check: passed.cargo clippy --all-targets --all-features --locked --offline -- -D warnings:passed for the full workspace.
ignored tests plus the new ignored benchmark. All 509 SQLLogicTest files passed.
field/type lookups and the aliased matcher loss before their respective fixes.
API compatibility and test coverage. Final source/artifact review explicitly
approved PR creation and merge, with no blocking findings or outstanding gates.
Runtime-branch performance qualification
The benchmark measures
Expr::to_fieldon prebuilt expressions, not queryexecution. Baseline and candidate use identical fixtures, compiler, lockfile,
default features, Arrow 59 and a temporary
ci-derived opt-level-1 profile.The dedicated benchmark profile is cleaned between source roots to prevent
cross-workspace Cargo artifact reuse. Executables are frozen before alternating
fresh-process runs; warmup, validation, formatting and field destruction are
outside the timed region.
Five alternating fresh-process pairs, with 20 warmups and 101 samples per fixture
per process, passed all preflight checks. The table reports the median of five
process medians in microseconds, not SQL latency or a p95:
The intermediate approximately 2x nested-binary regression was removed before
final qualification. The remaining measured increases are small planning costs,
not proof of noise or zero regression: depth-128 aliases add 1.335 us, plain
columns add 0.306 us, and the shallow depth-4 UDF control adds 0.261 us.
They are accepted for removing silent annotation loss while preserving the
fork's established semantics and common fast paths. Deep-UDF cases with wrong
baseline metadata are correctness tests, not paired performance claims.
CASE/UDF native re-inference remains a separate limitation.
Environment: Rust 1.97.0, Arrow 59.2.0, WSL2 x86-64, Ryzen 9 9950X3D. Default
datafusion-exprfeatures are identical. Cargo's artifact records confirmopt-level 1, debug info 0, debug assertions and overflow checks enabled, and a
fresh build from the intended source directory. This is a local planning screen,
not the reserved-host benchmark campaign or release-query qualification.
Identities (SHA-256):
84c956ecf80e17150f2759c06b3533776bfe877ae8bc7ecd3ca2c2dd6d5e6693.b21ee5ea28f8b057ad9dcc4f1ab14458ba3c87da7659f0088cafef41973a371a.bc95c18eca32b794f21925414d0a45c1fea23fe69c90d4ebc2d71356fe47a4e6.4482fbd44dc9e313fc2ca31b58a44ea8495c06a80a8a84a89af05149095477ca.To reproduce, copy the same benchmark into the base revision and build each
revision separately with the temporary profile below. Clean that profile between
source roots, freeze each printed executable, then alternate five pairs of
<executable> --exact case_field_metadata_perf --ignored --nocapture:Are there any user-facing changes?
Known CASE annotations are preserved instead of silently disappearing after an
arbitrary nesting depth; aliased comparison UDFs retain their literal matcher.
There is no public API, manifest, lockfile or runtime-config change, and no new
per-row conversion or wrapper is added.
This PR does not update Rustice's dependency revision. Arrow 60 upstream timings
and these Arrow 59 planning microbenchmarks are not SQL, Iceberg or SPCS speedups.