Branch 55 by gene-bordegaray · Pull Request #167 · DataDog/datafusion

gene-bordegaray · 2026-08-12T18:30:01Z

Which issue does this PR close?

Closes #.

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

## Which issue does this PR close?  - Closes apache#23513. ## Rationale for this change  Window plans still relied on centralized protobuf dispatch, keeping serialization separate from the execution plans that own their state. Both window executors share one protobuf variant. Decoding must inspect `input_order_mode` before selecting the concrete plan. Add plan-local encoders for both executors and a decoder for the shared node. Keep the legacy helpers as deprecated delegates while preserving window frames and UDF payloads on the existing wire format. ## What changes are included in this PR?  ## Are these changes tested?  Yes ## Are there any user-facing changes?   --------- Signed-off-by: Jiawei Zhao <[email protected]>

## Which issue does this PR close?  - Closes apache#23511. ## Rationale for this change  Physical plan protobuf serialization is being moved from the central dispatch module into each `ExecutionPlan` implementation. Co-locating this logic with the corresponding execution plan makes the serialization code easier to maintain and incrementally reduces the central downcast chain. ## What changes are included in this PR?  - Implement plan-local protobuf serialization and deserialization for `ExplainExec`. - Implement plan-local protobuf serialization and deserialization for `AnalyzeExec`. - Remove both plans from the live central serialization dispatch. - Retain the deprecated compatibility methods as delegates to the new implementations. - Preserve the existing protobuf wire format. - Strengthen roundtrip tests to cover all stored fields, including every `StringifiedPlan` variant, metric categories, and explain formats. Each execution plan migration is kept in a separate commit. ## Are these changes tested?  Yes. The following checks pass: - `cargo fmt --all` - `cargo clippy --all-targets --all-features -- -D warnings` - Full extended workspace test suite - `cargo test -p datafusion-proto --test proto_integration` The proto integration suite passes all 209 tests after rebasing onto the latest `main`. ## Are there any user-facing changes?   No. This is an internal refactor and does not change the protobuf wire format or user-facing behavior. --------- Signed-off-by: Jiawei Zhao <[email protected]>

## Which issue does this PR close?  - Closes apache#23512. ## Rationale for this change  Aggregate protobuf conversion lived in the central proto crate, which prevented AggregateExec from owning its function-codec serialization. Move encoding and decoding into AggregateExec and keep the deprecated helpers as delegates so existing callers retain wire-compatible behavior. ## What changes are included in this PR?  ## Are these changes tested?  Yes ## Are there any user-facing changes?   --------- Signed-off-by: Jiawei Zhao <[email protected]>

…nparser (apache#23789) ## Which issue does this PR close? - Closes apache#23668. ## Rationale for this change - When printing a GROUP BY query, the unparser sometimes wraps the aggregate's input in an inner subquery (`... FROM (SELECT ...)`). - Table aliases like `cs` only exist inside that subquery. Any clause outside it (SELECT, GROUP BY, HAVING, QUALIFY, ORDER BY) must use the subquery's output columns, not the aliases, so the unparser drops the alias. - Each clause does that dropping on its own, so it is easy to miss one. - ORDER BY on an aggregate that is not in the SELECT list was missed: it printed `ORDER BY round(sum("cs"."total_revenue"), 2)`, while the SELECT list in the same query correctly printed `sum("total_revenue")`. - DataFusion reads that SQL back fine, but stricter databases reject it because `cs` is out of scope there. ## What changes are included in this PR? I moved the rule into one helper, `UnparserAggScope`, that checks once per aggregate whether the input is an inner subquery and then prepares expressions for each clause. SELECT, GROUP BY, HAVING, QUALIFY, and both ORDER BY paths now go through it, which fixes the ORDER BY case. The window-over-aggregate path is left as-is with a comment: it is only reachable from hand-built plans, since a window in SQL always sits inside a SELECT that already drops the alias. Only ORDER BY output changes; every already-correct case stays the same. ## Are these changes tested? Yes. New tests in `datafusion/core/tests/sql/unparser.rs` cover the inner-subquery shape for a window sorting by an aggregate, ORDER BY on an unselected aggregate (the fixed case), and a top-level ORDER BY (the second sort path). Existing unparser tests and the TPC-H/Clickbench roundtrips still pass. ## Are there any user-facing changes? ORDER BY over an aggregate is now printed without an out-of-scope table qualifier, so the generated SQL is valid for stricter databases. No API changes.

…che#22980) ## Which issue does this PR close? * Part of apache#20835 ## Rationale for this change `FixedSizeList` containing `Struct` values was not handled by the existing recursive nested adaptation logic used for schema evolution. As a result, planner-time compatibility checks, nested cast detection, and runtime casting did not support additive struct evolution within `FixedSizeList` containers. This change adds `FixedSizeList` support and verifies planner/runtime parity so that planning allows exactly the cases runtime can adapt while continuing to reject incompatible schema changes. ## What changes are included in this PR? * Extend `cast_column` to support recursive casting of `FixedSizeList` values when source and target list sizes match. * Add `FixedSizeList` handling to: * `requires_nested_struct_cast` * `validate_data_type_compatibility` * Implement recursive casting of nested `Struct` values contained in `FixedSizeList`. * Preserve planner/runtime parity by validating child type compatibility before runtime fallback logic is applied. * Add handling for null-parent `FixedSizeList` entries by masking hidden child values before retrying casts, avoiding failures caused by semantically inaccessible child data. * Refactor list and list-view casting helpers to use Arrow `AsArray` accessors. ## Are these changes tested? Yes. The following tests were added: * `test_cast_fixed_size_list_struct` * `test_validate_fixed_size_list_struct_compatibility` * `test_validate_fixed_size_list_struct_missing_non_nullable_field_rejected` * `test_validate_fixed_size_list_struct_size_mismatch_rejected` * `test_cast_fixed_size_list_struct_all_null` * `test_fixed_size_list_struct_planner_runtime_parity_on_incompatible_type` * `test_cast_fixed_size_list_struct_missing_non_nullable_field_runtime_rejected` * `test_cast_fixed_size_list_struct_ignores_hidden_child_values_for_null_parent` Existing coverage in `test_requires_nested_struct_cast` was also extended to include `FixedSizeList` cases. These tests cover: * Additive nullable nested-field evolution * All-null and partially null list cases * Incompatible nested type changes * Non-nullable field addition rejection * Planner/runtime parity validation ## Are there any user-facing changes? No user-facing changes. This is an internal enhancement to nested schema adaptation and casting behavior for `FixedSizeList<Struct>` types. ## LLM-generated code disclosure This PR includes LLM-generated code and comments. All LLM-generated content has been manually reviewed. --------- Co-authored-by: Andrew Lamb <[email protected]>

… spark (apache#23766) ## Which issue does this PR close? N/A ## Rationale for this change Hex encoding was implemented six times across the workspace, at two different levels of optimization. Three copies used a fast byte-pair lookup table: - `datafusion/spark/src/function/math/hex.rs` - `datafusion/functions/src/string/to_hex.rs` - `datafusion/functions/src/encoding/inner.rs` (via the `hex` crate) The other three used a slower nibble-at-a-time loop pushing one character at a time: - `datafusion/functions/src/crypto/md5.rs` - `datafusion/spark/src/function/hash/sha1.rs` - `datafusion/spark/src/function/hash/sha2.rs` Beyond the duplication, this split meant the digest functions were paying for a slower encoder than the one already sitting elsewhere in the tree. Consolidating on a single implementation removes the duplication and moves `md5`, `sha1`, and `sha2` onto the fast path. ## What changes are included in this PR? A new `datafusion_common::utils::hex` module holds the only hex encoder in the workspace: ```rust pub enum HexCase { Lower, Upper } pub fn encode_bytes_into(bytes: &[u8], case: HexCase, out: &mut Vec<u8>); pub fn encode_bytes(bytes: &[u8], case: HexCase) -> String; pub fn encode_bytes_to_slice(bytes: &[u8], case: HexCase, out: &mut [u8]); pub fn encode_u64(v: u64, case: HexCase, buf: &mut [u8; 16]) -> &[u8]; ``` Four entry points rather than one because the call sites have genuinely different output needs, and forcing them through a single shape would cost an allocation somewhere: `to_hex` writes straight into a `StringArray` values buffer, Spark's `hex` appends to a reused scratch `Vec`, `encode` writes into a pre-sized slice, and the digest functions want an owned `String`. Migrated call sites, all producing bit-identical output: | File | Change | | --- | --- | | `functions/src/string/to_hex.rs` | local table and two write helpers deleted; eight trait impls collapsed to two macros | | `spark/src/function/math/hex.rs` | two nibble tables, two lookup tables, `build_hex_lookup` and `hex_int64` deleted | | `functions/src/crypto/md5.rs` | local table and `hex_encode` deleted | | `spark/src/function/hash/sha1.rs` | local table and inline nibble loop deleted | | `spark/src/function/hash/sha2.rs` | local table and `hex_encode` deleted; eight call sites migrated | | `functions/src/encoding/inner.rs` | both encode sites moved off the `hex` crate | Deliberately left alone: - `hex::decode` / `hex::decode_to_slice` in `encoding/inner.rs`, and Spark's `unhex`. The decode direction has different semantics — Spark's `unhex` left-pads odd-length input, the `hex` crate does not — so unifying it is a separate question. The `hex` dependency stays for those. - `ScalarValue`'s binary `Display` impl in `common/src/scalar/mod.rs`, which writes to a `fmt::Formatter` rather than a byte buffer, and is a cold path. Three details worth a reviewer's attention: - The two ancestors of `encode_u64` disagreed on zero: `to_hex` wrote `'0'` into the caller's buffer, Spark's `hex_int64` returned a `'static` `b"0"` that never touched it. The shared version always writes into the buffer and returns a subslice of it, so the lifetime is uniform. Both callers still produce `"0"`. - Spark's `hex_encode_bytes` guards large binary input with `checked_mul(2)` + `try_reserve`, returning a `DataFusionError` rather than aborting on allocation failure. That guard stays at the call site; `encode_bytes_into` performs no reservation of its own, so behaviour is unchanged. - The encoders are `#[inline(always)]`, not `#[inline]`. They are called once per row from two other crates, and plain `#[inline]` left them out-of-line across the crate boundary. Measured cost of that: +3% on Spark's byte paths and up to +18% on `to_hex`'s i32 path, i.e. the refactor was a net regression on those benchmarks until the attribute changed. ## Are these changes tested? Every migrated function keeps its existing unit and sqllogictest coverage, which is what pins bit-identical output — in particular `spark/hash/sha1.slt` and `sha2.slt` assert concrete digest strings for all four SHA-2 bit lengths across both the scalar and array paths, and `expr.slt` covers `to_hex` and `md5`. New unit tests in `common/src/utils/hex.rs` cover zero, `u64::MAX`, single-nibble values, the odd/even digit-count boundary, two's complement of negative input, empty input, all 256 byte values in both cases, appending into a non-empty buffer, and that a reused scratch buffer never leaks stale digits between calls. Tests cross-check against `format!("{:x}")` rather than restating the implementation. Two Spark tests changed. `test_hex_int64` now drives `hex_encode_int64` instead of the deleted private `hex_int64`, keeping all ten cases including `i64::MIN`, `-1`, and the uppercase expectations. `test_hex_lookup_table_covers_all_bytes` was deleted — it only cross-checked the raw lookup tables, and `encode_bytes_covers_every_byte_value` now does that exhaustively through the public API. A test was added for Spark's lowercase byte path, which previously had no coverage. ### Benchmarks Criterion, `apache/main` @ `eef101769` as baseline. Median of the reported change interval. `datafusion/functions/benches/to_hex.rs`: | Benchmark | 1024 | 4096 | 8192 | | --- | --- | --- | --- | | `i32_random` | −13.8% | −12.8% | −10.2% | | `i64_random` | −8.8% | −9.5% | −8.1% | | `i64_large_values` | −9.9% | −9.8% | −7.9% | `scalar_i32` −2.3%, `scalar_i64` −1.8%. `datafusion/spark/benches/sha2.rs`: | Benchmark | 1024 | 4096 | 8192 | | --- | --- | --- | --- | | `array_binary_256` | −20.7% | −18.2% | −19.1% | | `array_scalar_binary_256` | −14.6% | −13.2% | −13.1% | `scalar/size=1` −3.3%. `datafusion/spark/benches/hex.rs`: | Benchmark | 1024 | 4096 | 8192 | | --- | --- | --- | --- | | `hex_int64` | −2.9% | −3.6% | −3.5% | | `hex_int64_dict` | −3.2% | −1.5% | −0.2% | | `hex_utf8` | −0.2% | −0.3% | +0.7% | | `hex_binary` | −0.4% | −0.6% | −0.3% | The `hex_utf8` and `hex_binary` paths already used the byte-pair table before this PR, so they are expected to be flat; they are. `datafusion/functions/benches/crypto.rs`: `md5_array` −4.0%, `md5_scalar` −3.7%. The `sha224` / `sha256` / `sha384` / `sha512` cases in that file range from −0.1% to +2.2%, but they exercise `crypto/basic.rs`, which this PR does not modify and which contains no hex encoding — those numbers are run-to-run variance, not an effect of this change. No number is quoted for Spark `sha1`: it has no benchmark, and its change is the same substitution applied to `md5` and `sha2`. ## Are there any user-facing changes? No behaviour change — all migrated functions produce byte-identical output. `datafusion_common::utils::hex` is new public API on `datafusion-common`. --------- Co-authored-by: Andrew Lamb <[email protected]> Co-authored-by: Jeffrey Vo <[email protected]>

…t_batch` memory (apache#23873) ## Which issue does this PR close? - related to apache#23716 ## Rationale for this change It adds test coverage for two gaps found while reviewing apache#23716. ## What changes are included in this PR? Tests only, no functional change. - Dictionary inputs - memory usage on retract (make sure memory is released) Note I moved `array_agg` cases out `aggregate.slt` as it is already more than 9k lines long ## Are these changes tested? They are only tests ## Are there any user-facing changes? No. Tests only, no public API changes. --------- Co-authored-by: Claude Opus 5 (1M context) <[email protected]>

…3881) ## Which issue does this PR close? N/A ## Rationale for this change Two Spark functions allocated a `String` per row purely to render a small, bounded amount of text: - `bin` called `format!("{value:b}")` for every row. - `char` called `ch.to_string()` for every row — a heap allocation for a single character. In both cases the output has a known upper bound (64 binary digits for an `i64`, 4 bytes for a UTF-8 character), so the rendering fits in a stack buffer and the result can be appended straight to a pre-sized builder. ## What changes are included in this PR? `math/bin.rs`: - `spark_bin` now writes digits right-aligned into a caller-supplied `[u8; 64]` and returns a `&str` borrowed from it, instead of returning an owned `String`. - The `collect::<StringArray>()` becomes an explicit loop over a `StringBuilder::with_capacity`, sized at 8 digits per row. - Negative values still render as their two's-complement bit pattern, matching `{:b}`. The digit loop is a `loop`, not a `while`, so zero renders as `"0"` rather than the empty string. `string/char.rs`: - `ch.to_string()` becomes `ch.encode_utf8(&mut encoded)` against a `[u8; 4]` hoisted out of the loop. Output is unchanged in both cases. ## Are these changes tested? Existing coverage pins the behaviour. `spark/math/bin.slt` asserts concrete output for the cases the rewrite had to get right: zero, negative values, `i64::MIN` (`-9223372036854775808`), `i64::MAX`, and `-2147483648` / `-32768` widened from narrower integer types. `spark/string/char.slt` covers the negative-input empty string, the null path, and characters on both sides of the ASCII boundary (`char(256)` and above wrap via `% 256`). All 119 `spark/math` and `spark/string` sqllogictest files pass, along with the 258 `datafusion-spark` unit tests. The `bin` benchmark used below, `datafusion/spark/benches/bin.rs`, is added separately in apache#23882 so the baseline can be measured on `main` before this change lands. It covers 1024 and 8192 rows with 20% nulls over two value distributions: small values that render to a handful of digits, and full-range values that render to the maximum 64. `char` already had `datafusion/spark/benches/char.rs` on `main`, so no benchmark change is needed for it. ### Benchmarks Criterion, `apache/main` @ `f1ab86dad` as baseline. Median of the reported change interval. | Benchmark | 1024 | 8192 | | --- | --- | --- | | `bin/small` | −75.7% | −73.7% | | `bin/wide` | −47.8% | −49.8% | `char` (1024 rows): −76.1%. ## Are there any user-facing changes? No. Both functions produce byte-identical output; this is purely an allocation change.

…ts enabled (apache#23848) ## Which issue does this PR close?  - Closes apache#23847 . ## Rationale for this change Fixes a correctness bug, not sure when that config used/desirable. This is another thing I ran into during apache#21585 ## What changes are included in this PR? 1. New SLT test 2. Fix for bug in `datafusion-optimizer` ## Are these changes tested? Existing tests, and a new SLT test verifying the config change doesn't affect the query result and basic unit test. ## Are there any user-facing changes? None --------- Signed-off-by: Adam Gutglick <[email protected]>

…lter (apache#23901) ## Which issue does this PR close? - Closes apache#23900. ## Rationale for this change `push_down_filter` infers equi-key predicates across a join's ON keys and pushes them to the opposite side. For a null-aware join (the `LeftAnti` join produced by `NOT IN` with a nullable subquery), an outer predicate on the left key like `outer.id > 5` is rewritten to `sub.id > 5` and pushed onto the subquery input. Since the inferred predicate must be null-rejecting to be pushed, this drops the subquery's NULL rows and breaks the three-valued `NOT IN` semantics — a NULL in the subquery key must reach the join so the result is empty. Same class of bug as apache#23848, in a different rule. ## What changes are included in this PR? - Skip predicate inference in `infer_join_predicates` when `join.null_aware` is set (mirrors the apache#23848 guard on `FilterNullJoinKeys`). - A `push_down_filter` unit test asserting no predicate is inferred onto the subquery side of a null-aware `LeftAnti` join. - SLT coverage for the failing query, plus a `prefer_hash_join = false` / multi-partition variant. ## Are these changes tested? Yes — new unit test (verified it fails without the guard) and SLT cases. The full optimizer lib suite passes. ## Are there any user-facing changes? No, aside from the correctness fix.

## Which issue does this PR close? - Part of apache#19241. - Stacked on [apache#23311](apache#23311). - Next in stack: apache#23015. - Extracted from apache#19390. ## Rationale for this change For very small `IN` lists, building or probing a hash table can be more work than just comparing the input value with each constant. For example, for `x IN (10, 20, 30)`, the fast path can behave like: ```text x == 10 OR x == 20 OR x == 30 ``` Because the list is tiny, those comparisons are cheap. The implementation stores the constants in a fixed-size array and checks them with a compact comparison chain. “Branchless” here means the comparisons are combined without stopping at the first match. That can be faster for these small fixed-width lists because the CPU gets a predictable sequence of simple operations instead of hash-table setup and probe logic. For primitive values that are not already plain unsigned integers, this PR keeps the logical Arrow type explicit and uses a matching same-width comparison representation only inside the branchless filter. For example, `Float16` uses `UInt16` storage, `Float32` uses `UInt32` storage, and `TimestampNanosecond` uses `UInt64` storage. `Decimal128` and `IntervalMonthDayNano` use their own 16-byte native representation. This preserves bit-pattern equality while relying on Arrow's native primitive compatibility rules: timestamp timezone metadata and Decimal128 precision/scale metadata may differ, while incompatible primitive representations remain rejected. ## What changes are included in this PR? - Adds a const-generic `BranchlessFilter` for small primitive `IN` lists. - Adds thresholds for when this path is used: - up to 16 values for 1-byte types - up to 8 values for 2-byte types - up to 32 values for 4-byte types - up to 16 values for 8-byte types - up to 4 values for 16-byte types - Keeps dispatch concrete and explicit in `strategy.rs`. - Maps each optimized logical type to the comparison representation used by the branchless filter: - `Int8` -> `UInt8` - `Int16`, `Float16` -> `UInt16` - `Int32`, `Float32`, `Date32`, `Time32` -> `UInt32` - `Int64`, `Float64`, `Date64`, `Time64`, `Timestamp`, `Duration` -> `UInt64` - `Decimal128`, `IntervalMonthDayNano` -> their native 16-byte representation - Leaves larger 1-byte and 2-byte lists on the existing bitmap filters. - Leaves larger 4-byte and 8-byte lists on the existing hash/generic paths. - Leaves wider primitive types such as `Decimal256` and unsupported complex types on the generic path. - Keeps the same `IN` / `NOT IN` null behavior as the rest of the stack. - Adds focused coverage for branchless null handling, signed boundary values, slices, Float16/Float32/Float64 bit patterns, compatible timestamp/Decimal128 metadata, incompatible timestamp units, IntervalMonthDayNano values, and same-width wrong-type probe rejection. ## Are these changes tested? Yes. - `cargo fmt --all -- --check` - `cargo test -p datafusion-physical-expr expressions::in_list --lib` - `cargo test -p datafusion-physical-expr --bench in_list_strategy --no-run` - `cargo clippy --all-targets --all-features -- -D warnings` ## Are there any user-facing changes? No. This is an internal performance optimization only. ## Local benchmark snapshot Built and run with `release-nonlto`, filtered to the relevant small primitive-list rows: ```bash cargo bench -p datafusion-physical-expr --profile release-nonlto --bench in_list_strategy -- <filter> --save-baseline <baseline> ``` Filters used: `narrow_integer`, `primitive/i32/small_list`, `primitive/i64/small_list`, `f32/small_list`, `timestamp_ns/small_list`, and `interval_month_day_nano/small_list`. Method: directly compared Criterion's raw sample minima (`min(time / iterations)`) from `sample.json`. Lower is better; changes within +/-5% are treated as noise. Compared baselines: [apache#23311](apache#23311) -> [apache#23014](apache#23014) Relevant scope: small primitive-list rows. Summary: 39 relevant rows, 28 faster, 0 slower, 11 within +/-5%. Largest relevant deltas: | Benchmark | Before | After | Change | |---|---:|---:|---:| | `timestamp_ns/small_list/list=4/match=50%` | 46.55 us | 3.17 us | -93.2% (14.69x faster) | | `f32/small_list/list=4/match=50%` | 33.93 us | 3.04 us | -91.0% (11.15x faster) | | `primitive/i32/small_list/list=4/match=50%` | 32.63 us | 3.08 us | -90.5% (10.58x faster) | | `primitive/i64/small_list/list=4/match=50%` | 33.55 us | 3.18 us | -90.5% (10.54x faster) | | `timestamp_ns/small_list/list=4/match=0%` | 19.57 us | 3.18 us | -83.8% (6.16x faster) | | `f32/small_list/list=4/match=0%` | 18.14 us | 3.05 us | -83.2% (5.95x faster) | | `primitive/i32/small_list/list=4/match=0%` | 17.00 us | 3.04 us | -82.1% (5.59x faster) | | `primitive/i64/small_list/list=4/match=0%` | 17.12 us | 3.22 us | -81.2% (5.31x faster) | | `primitive/i32/small_list/list=16/match=50%/NOT_IN` | 31.98 us | 7.26 us | -77.3% (4.41x faster) | | `nulls/primitive/i32/small_list/list=16/match=50%/nulls=20%` | 29.35 us | 7.32 us | -75.1% (4.01x faster) | | `timestamp_ns/small_list/list=16/match=50%` | 45.32 us | 11.79 us | -74.0% (3.84x faster) | | `nulls/primitive/i32/small_list/list=16/match=50%/nulls=50%` | 25.89 us | 7.31 us | -71.8% (3.54x faster) | | `nulls/primitive/i32/small_list/list=16/match=50%/nulls=20%/NOT_IN` | 26.05 us | 7.42 us | -71.5% (3.51x faster) | | `interval_month_day_nano/small_list/list=4/match=50%` | 52.94 us | 15.52 us | -70.7% (3.41x faster) | | `f32/small_list/list=32/match=50%` | 38.78 us | 13.27 us | -65.8% (2.92x faster) | | `primitive/i64/small_list/list=16/match=50%` | 29.46 us | 11.76 us | -60.1% (2.50x faster) | <details> <summary>Full relevant table (39 rows)</summary> | Benchmark | Before | After | Change | |---|---:|---:|---:| | `narrow_integer/u8/list=4/match=0%` | 3.86 us | 2.79 us | -27.8% (1.38x faster) | | `narrow_integer/u8/list=4/match=50%` | 3.84 us | 2.78 us | -27.7% (1.38x faster) | | `narrow_integer/u8/list=16/match=0%` | 3.88 us | 3.85 us | -0.8% (within noise) | | `narrow_integer/u8/list=16/match=50%` | 3.84 us | 3.86 us | +0.5% (within noise) | | `narrow_integer/i16/list=4/match=0%` | 3.93 us | 3.18 us | -19.1% (1.24x faster) | | `narrow_integer/i16/list=4/match=50%` | 3.92 us | 3.16 us | -19.5% (1.24x faster) | | `narrow_integer/i16/list=64/match=0%` | 3.96 us | 3.82 us | -3.5% (within noise) | | `narrow_integer/i16/list=64/match=50%` | 3.91 us | 3.80 us | -2.9% (within noise) | | `narrow_integer/i16/list=256/match=0%` | 3.90 us | 3.81 us | -2.5% (within noise) | | `narrow_integer/i16/list=256/match=50%` | 3.97 us | 3.81 us | -4.1% (within noise) | | `narrow_integer/f16/list=4/match=0%` | 3.87 us | 3.16 us | -18.5% (1.23x faster) | | `narrow_integer/f16/list=4/match=50%` | 3.94 us | 3.15 us | -20.2% (1.25x faster) | | `narrow_integer/f16/list=64/match=0%` | 3.87 us | 3.84 us | -0.6% (within noise) | | `narrow_integer/f16/list=64/match=50%` | 3.93 us | 3.85 us | -1.9% (within noise) | | `narrow_integer/f16/list=256/match=0%` | 3.90 us | 3.84 us | -1.5% (within noise) | | `narrow_integer/f16/list=256/match=50%` | 3.87 us | 3.91 us | +1.2% (within noise) | | `nulls/narrow_integer/u8/list=16/match=50%/nulls=20%` | 3.92 us | 4.02 us | +2.5% (within noise) | | `primitive/i32/small_list/list=4/match=0%` | 17.00 us | 3.04 us | -82.1% (5.59x faster) | | `primitive/i32/small_list/list=4/match=50%` | 32.63 us | 3.08 us | -90.5% (10.58x faster) | | `primitive/i32/small_list/list=32/match=0%` | 16.34 us | 13.33 us | -18.5% (1.23x faster) | | `primitive/i32/small_list/list=32/match=50%` | 31.17 us | 13.31 us | -57.3% (2.34x faster) | | `primitive/i32/small_list/list=16/match=50%/NOT_IN` | 31.98 us | 7.26 us | -77.3% (4.41x faster) | | `nulls/primitive/i32/small_list/list=16/match=50%/nulls=20%` | 29.35 us | 7.32 us | -75.1% (4.01x faster) | | `nulls/primitive/i32/small_list/list=16/match=50%/nulls=20%/NOT_IN` | 26.05 us | 7.42 us | -71.5% (3.51x faster) | | `nulls/primitive/i32/small_list/list=16/match=50%/nulls=50%` | 25.89 us | 7.31 us | -71.8% (3.54x faster) | | `primitive/i64/small_list/list=4/match=0%` | 17.12 us | 3.22 us | -81.2% (5.31x faster) | | `primitive/i64/small_list/list=4/match=50%` | 33.55 us | 3.18 us | -90.5% (10.54x faster) | | `primitive/i64/small_list/list=16/match=0%` | 16.34 us | 11.93 us | -27.0% (1.37x faster) | | `primitive/i64/small_list/list=16/match=50%` | 29.46 us | 11.76 us | -60.1% (2.50x faster) | | `f32/small_list/list=4/match=0%` | 18.14 us | 3.05 us | -83.2% (5.95x faster) | | `f32/small_list/list=4/match=50%` | 33.93 us | 3.04 us | -91.0% (11.15x faster) | | `f32/small_list/list=32/match=0%` | 22.05 us | 13.43 us | -39.1% (1.64x faster) | | `f32/small_list/list=32/match=50%` | 38.78 us | 13.27 us | -65.8% (2.92x faster) | | `timestamp_ns/small_list/list=4/match=0%` | 19.57 us | 3.18 us | -83.8% (6.16x faster) | | `timestamp_ns/small_list/list=4/match=50%` | 46.55 us | 3.17 us | -93.2% (14.69x faster) | | `timestamp_ns/small_list/list=16/match=0%` | 19.73 us | 12.07 us | -38.8% (1.63x faster) | | `timestamp_ns/small_list/list=16/match=50%` | 45.32 us | 11.79 us | -74.0% (3.84x faster) | | `interval_month_day_nano/small_list/list=4/match=0%` | 20.12 us | 13.20 us | -34.4% (1.52x faster) | | `interval_month_day_nano/small_list/list=4/match=50%` | 52.94 us | 15.52 us | -70.7% (3.41x faster) | </details> --------- Co-authored-by: Andrew Lamb <[email protected]>

## Which issue does this PR close? - Closes apache#23770 - Part of #15914 ## Rationale for this change Spark provides [`hypot(expr1, expr2)`](https://spark.apache.org/docs/latest/api/sql/#hypot), which returns `sqrt(expr1^2 + expr2^2)` computed without intermediate overflow or underflow. It was not yet implemented in `datafusion-spark` — only an auto-generated test stub existed at `spark/math/hypot.slt` with its query commented out. ## What changes are included in this PR? - Add `SparkHypot` (implementing `ScalarUDFImpl`) in `datafusion/spark/src/function/math/hypot.rs`, backed by Rust's `f64::hypot` — the same overflow-safe algorithm as Java/Spark's `Math.hypot`. - Register it in `datafusion/spark/src/function/math/mod.rs`. - Enable the `hypot.slt` sqllogictest. The signature is `exact(Float64, Float64) -> Float64`, following the `datafusion-spark` convention of only accepting types Spark supports. Computation uses the Arrow `binary` kernel so NULL in either argument propagates to a NULL result, matching Spark. ## Are these changes tested? Yes — `datafusion/sqllogictest/test_files/spark/math/hypot.slt` covers: - scalar Pythagorean triples (`hypot(3, 4)` → 5, `hypot(5, 12)` → 13), - double inputs, - NULL propagation when either argument is NULL, - the array path (including a NULL row), - overflow-safety: `hypot(3e200, 4e200)` stays finite, whereas a naive `sqrt(a^2 + b^2)` would overflow to `Infinity`. ## Are there any user-facing changes? Yes — adds the Spark-compatible `hypot` scalar function to `datafusion-spark`. No breaking changes to public APIs.

## Which issue does this PR close?  - Part of apache#23393 . ## Rationale for this change `SlidingMinAccumulator::size` and `SlidingMaxAccumulator::size` only reported the stack size of their `ScalarValue` field plus its heap payload, ignoring the memory held by the underlying `MovingMin` / `MovingMax` sliding-window buffers. For windowed `MIN`/`MAX` over string or list data, the two per-element stacks can hold megabytes of `ScalarValue` payload that the memory pool was never told about, understating accumulator memory usage. ## What changes are included in this PR? - Add a private `heap_size(elem_heap)` method to `MovingMin<T>` and `MovingMax<T>` that reports the two stack buffers' capacity in bytes plus each stored element's heap payload. - Factor the shared implementation into a `moving_stacks_heap_size` free helper so the two types stay in sync. - Include the buffer bytes in `SlidingMinAccumulator::size` and `SlidingMaxAccumulator::size` via the new method. ## Are these changes tested? Yes. Two new unit tests in `datafusion/functions-aggregate/src/min_max.rs`: - `moving_min_max_heap_size_i32` — fixed-width `T`, verifies buffer-only accounting with and without pushed elements. - `moving_min_max_heap_size_counts_elems` — `T = String`, verifies each of the two slots in a `(T, T)` pair contributes independently to the heap payload (mirroring the two independent `Clone`s made by `push`).

## Which issue does this PR close? - Related to apache#21882 (comment). ## Rationale for this change PR apache#21882 adds custom spill file support. Having an example showing how a downstream application can use this new API will help make sure the API is good enough for our needs ## What changes are included in this PR? This PR adds a small example showing how users can back spill files with an `ObjectStore`, using a local object store for a runnable example while keeping the implementation applicable to remote stores. ## Are these changes tested? Yes by CI ## Are there any user-facing changes? Yes. This adds a new example for configuring ObjectStore-backed spill files.

## Which issue does this PR close?  - Closes #N/A. ## Rationale for this change  Since DataFusion doesn't typically guartantee wire format compatibility, I remove the backward compatiblity shim that introduced in PR apache#23189 FYI: apache#23189 (comment) ## What changes are included in this PR?  DataFusion does not guarantee serialized plans across versions. Keeping `partitioned_by_file_group` therefore leaves a dead schema field and decoder path after `output_partitioning` became the source of truth. Reserve the old field number and name to prevent future reuse. ## Are these changes tested?  Yes ## Are there any user-facing changes?   Yes Signed-off-by: Jiawei Zhao <[email protected]> Co-authored-by: Andrew Lamb <[email protected]>

…llable build key (apache#23173) ## Which issue does this close? - Closes apache#23126. ## Rationale for this change `x NOT IN (subquery)` plans to a null-aware `LeftAnti` hash join (build = outer `x`, probe = subquery). Join dynamic filter pushdown pushes a bounds + membership filter, built from the build keys, onto the probe scan. That filter can prune every probe row. A null-aware `LeftAnti` reads an empty probe as a genuinely-empty subquery, so it emits build-side NULL rows that should drop: `NULL NOT IN (non-empty)` is UNKNOWN, not TRUE. The result is scan-dependent, so it's a silent correctness bug. A `VALUES` scan ignores the pushed filter and stays correct; a parquet scan applies it and is wrong. apache#23103 (the probe-side NULL drop) is orthogonal; this is the build-side NULL. ## What changes are included in this PR? Skip join dynamic filter pushdown for a null-aware anti join when the build key can be NULL. The build-side NULL emission depends on whether the probe is truly empty, which the pushed filter can change by emptying it. A NOT NULL build key has no such NULL, so it keeps the pushdown. The check is static: a schema-nullable build key disables the pushdown even when the data contains no NULLs. A runtime alternative (keep the pushdown and neutralize the filter only when the build actually holds a NULL key) would restore the optimization for those cases. I'd leave that as a follow-up. ## Are these changes tested? Yes. A `push_down_filter_parquet.slt` case reproduces it (build-side NULL, a non-matching parquet probe) and asserts the single correct row. Without the change it returns the extra NULL. In addition, unit tests pin both directions of the guard: a nullable build key rejects the pushdown and a NOT NULL build key keeps it. ## Are there any user-facing changes? `NOT IN` over a parquet (or otherwise prunable) scan with a nullable outer key now returns correct results. Such joins lose the dynamic filter pushdown.

…cimal) (apache#23631) ## Which issue does this PR close?  N/A ## Rationale for this change  I noticed there were some subtle errors with how decimals were handled in scalar values, and also opportunity to remove power calls in favour of precomputed constant tables. Also filling out some other missing support. ## What changes are included in this PR?  I recommend looking at the commits as they are self contained with detailed messages for each. ## Are these changes tested?  Yes ## Are there any user-facing changes?  No

…lter pushdown enabled (apache#23638) ## Which issue does this PR close? - Closes apache#23531. ## Rationale for this change `input_file_name()` is `FileSource` dependent like `file_row_index()`, and therefore shouldn't be pushed down into a filter. ## What changes are included in this PR? `PushdownChecker` now handles both UDFs consistently. If we keep adding this sort of metadata functions, we might want a better API to detect them, but for now I think this is a reasonable approach that isn't very invasive. ## Are these changes tested? Additional SLT test that verifies that both function behave correctly with pushdown either enabled or disabled. ## Are there any user-facing changes? None --------- Signed-off-by: Adam Gutglick <[email protected]>

…xpr::check_bigger_cast (apache#23808) (apache#23809) ## Which issue does this PR close? - Closes apache#23808. ## Rationale for this change ref. apache#23807 `CastExpr::check_bigger_cast` is used to determine whether a cast is a widening (order-preserving) conversion. Currently, it classifies `Int32 -> Float32`, `UInt32 -> Float32`, `Int64 -> Float64`, and `UInt64 -> Float64` as widening casts. However, integer-to-float conversions for 32-bit and 64-bit integers lose precision when values exceed the mantissa bit limit (24 bits for `Float32`, 53 bits for `Float64`). For example: `16_777_216_i32 as f32 == 16_777_217_i32 as f32` (both yield 16777216.0f32). Because distinct integer inputs can collapse to the same float output, these casts are not strictly 1-to-1 (injective) and can break suffix sort key ordering in multi-column ordering analysis (e.g. `[CAST(a AS Float32), b]`). ## What changes are included in this PR? - Updated `CastExpr::check_bigger_cast` to exclude precision-losing integer-to-float conversions (`Int32/UInt32 -> Float32` and `Int64/UInt64 -> Float64`). - Added unit test `test_check_bigger_cast_precision_loss` to verify precision-losing casts return `false` while exact conversions (`Int16 -> Float32`, `Int32 -> Float64`, etc.) continue to return `true`. ## Are these changes tested? Yes, new unit test `test_check_bigger_cast_precision_loss` in `cast.rs`. ## Are there any user-facing changes? No breaking API changes. Internal optimizer behavior fix.

…ossible (apache#23702) ## Which issue does this PR close? N/A ## Rationale for this change `SortPreservingMergeStream` is a little complex, so add some guiding comments and make it as textbook-like as possible ## What changes are included in this PR? Added comments, reorder code While this was done this also fixed couple of bugs due to how it work: 1. leftover drain was not counted in the `elapsed_compute` 2. `limit(0)` returns 0 rows and not 1 ## Are these changes tested? Existing tests ## Are there any user-facing changes? Not API ones. `limit(0)` now returns 0 rows

## Which issue does this PR close? - Closes apache#21428. ## Rationale for this change This PR adds `BuildHasher`-based variants for `hash_utils` so callers can compute row hashes with a caller-provided hash builder instead of always using DataFusion's default `RandomState`. The main constraint is performance: `with_hashes` is a hot path, especially for string, dictionary, and nested array hashing. A previous version in apache#21429 caused measurable regressions in the default `RandomState` path, for example `large_utf8: single, no nulls` regressed from roughly `26.7us` to `36.3us`, and `large_utf8: multiple, no nulls` from roughly `112us` to `127us`. This version keeps the default path performance-oriented by avoiding a fully generic `BuildHasher` rewrite of the existing hot loops. ## What changes are included in this PR? This PR adds: - `with_hashes_with_hasher` - `create_hashes_with_hasher` - custom-hasher implementations for primitive, string, binary, byte-view, dictionary, and nested arrays - tests covering custom hashers, multi-column hashing, and dictionary equivalence The implementation intentionally uses a hybrid design: - Default `RandomState` leaf hot paths remain specialized. - Custom `BuildHasher` leaf paths live separately in `hash_utils/build_hasher.rs`. - Nested/structural logic is shared through an internal child-hashing adapter, so struct/list/map/union/run/dictionary behavior does not need to be broadly duplicated. The trade-off is that there is still some duplication for primitive/string/binary leaf loops. That duplication is intentional: those are the hottest loops, and keeping them separate prevents the existing `RandomState` path from becoming generic over `BuildHasher` or being perturbed by the custom-hasher implementation. ## Are these changes tested? Yes. --------- Co-authored-by: Dmitrii Blaginin <[email protected]> Co-authored-by: Andrew Lamb <[email protected]>

…t whether they keep the same ordering of the input (apache#23807) ## Which issue does this PR close? - Closes apache#23798 Related to: - apache#16217 ## Rationale for this change To be able to keep the same sorting order allowing for more optimizations Now comet own `cast` implementation can be recognized as not modifying sort order in the same cases that datafusion cast does. ## What changes are included in this PR? added `strictly_order_preserving` property to `ExprProperties` + varius other places and replaced the hard coded logic for cast (`substitute_cast_ordering`) about keeping input order with more generic approach that now any expression can implement and have the same advantage of sort elimination also marked from_unixtime as keeping ordering to show a case of this optimization ## Are these changes tested? yes ## Are there any user-facing changes? yes, breaking change, added `strictly_order_preserving` property to `ExprProperties` and to `FFI_ExprProperties` this property means that given expression `f` and 2 values from the input column `a` and `b` the following variants are kept: 1. `a.cmp(b) == f(a).cmp(f(b))` 2. nulls maps to nulls Example of satisfying expression: `cast(col_a as BIGINT)` where `col_a` is `INT` it is keeping the properties Example of not satisfying: `floor` - floor can not `array_repeat(my_col, 2)` which might look like at first glance as keeping the property as well but in fact it does not. the reason is that `array_repeat(null, 2)` will output list of 2 nulls which breaks the 2nd property that nulls must be kept as nulls how to migrate: Option 1 - keeping the old behavior (safest but least performant) set `strictly_order_preserving` to false Option 2 - Using the new optimization that this opens up: set `strictly_order_preserving` only if the expression keep both variants

## Which issue does this PR close? Part of apache#22330. ## Rationale for this change `ForeignScalarUDF` inherits the default `preserves_lex_ordering`, so producer overrides are lost across the FFI boundary. ## What changes are included in this PR? - Forward `preserves_lex_ordering` through `FFI_ScalarUDF`. - Reuse the existing placement UDF for unit and dynamic-library coverage. ## Are these changes tested? - `cargo test -p datafusion-ffi --features integration-tests` - `cargo clippy --all-targets --all-features -- -D warnings` ## Are there any user-facing changes? The FFI ABI changes. Foreign libraries must rebuild against the new DataFusion version. --------- Signed-off-by: Amogh Ramesh <[email protected]>

…tonic deques (apache#23826) (apache#23827) ## Which issue does this PR close? - Closes apache#23826 ### What changes are included in this PR? This PR optimizes sliding window `MIN`/`MAX` aggregate functions using a **Sequence-Numbered Monotonic Deque** instead of a Two-Stack Queue. **Revised Design:** - We store `(sequence_number, value)` pairs in a single `VecDeque`, kept in strictly monotonic order. - **`push(val)`**: Evicts dominated elements from the back, then pushes the new value with the current `push_seq` and increments `push_seq`. - **`pop()`**: Increments `pop_seq`. If the front elements sequence number equals the old `pop_seq`, it is expired and popped. - **Benefits**: No secondary FIFO queue (lower memory) and no `clone()` overhead. ### Are these changes tested? Yes, existing tests pass. Added tests for empty-window `pop()` and duplicate-heavy scenarios. ### Are there any user-facing changes? **API Change**: `MovingMin` and `MovingMax` were changed to `pub(crate)` visibility, and their `pop()` methods now return `()` instead of returning a value. Performance is significantly improved (2x-3.5x throughput). --------- Co-authored-by: Pavan <[email protected]>

## Which issue does this PR close? NA ## Rationale for this change Adds [datapress](https://docs.datap-rs.org) to the list of known users in the documentation. ## What changes are included in this PR? NA ## Are these changes tested? NA ## Are there any user-facing changes? NA

## Which issue does this PR close? - Closes apache#11748 ## Rationale for this change `EliminateGroupByConstant` removes GROUP BY expressions that are constants. If all of the GROUP BY expressions are constants, eliminating all of them results in converting a grouped aggregate into an ungrouped (global) aggregate. This changes the semantics of the query: a grouped aggregate query on an empty input returns zero rows, whereas an ungrouped aggregate query produces a single row. ## What changes are included in this PR? `EliminateGroupByConstant` now declines to eliminate constant GROUP BY expressions, if doing so would result in removing all of the grouping expressions. ## Are these changes tested? Yes. Sqllogictest reproducer for the original issue, updated unit test and optimizer SLT expectations. ## Are there any user-facing changes? Queries with all-constant GROUP BY may now return fewer (correct) rows.

…pache#23874) ## Which issue does this PR close? - Closes apache#23872 ## Rationale for this change `SlidingMinAccumulator::update_batch` skipped NULL values. This meant that if all the non-NULL values in a window frame were retracted, the window frame would not be empty but the `MovingMin` data structure by the `SlidingMinAccumulator` would not contain any values. This resulted in incorrectly returning a stale non-NULL value for a sliding `min()` over a window frame consisting of only NULL values. Along the way, optimize the min and max sliding window accumulators to make them both more efficient and more symmetric with one another. In the original coding, `SlidingMaxAccumulator` included NULL values but `SlidingMinAccumulator` omitted them, in part because omitting NULLs made the original `retract_batch` implementation more expensive. This PR optimizes `retract_batch`, so we can now use the same scheme for both the min and max sliding accumulators: * Omit NULLs on `update_batch` (this improves on the prior behavior of `max`) * Efficiently account for NULLs in `retract_batch` (this improves on the prior behavior of `min`) * Ensure correct results for all-NULL window frames (this fixes the prior bug in `min`). ## What changes are included in this PR? * Fix bug in `min()` over all-NULL window frames * Optimize `SlidingMinAccumulator::retract_batch` (avoid materializing values just to count NULLs) * Optimize `SlidingMinAccumulator::update_batch` (omit NULLs), also improving symmetry with `min` * Optimize both accumulators to stop caching the current `min` / `max`; this saves a few clones, but perhaps more importantly it is simpler and avoids the risk of inconsistency between the cached value and the underlying `MovingMin` / `MovingMax` data structure ## Are these changes tested? Yes, new tests added. ## Are there any user-facing changes? No, aside from the bug fix.

## Which issue does this PR close? - N/A ## Rationale for this change ``` $ cargo test -p datafusion-functions-aggregate [...] warning: associated function `from_parts` is never used --> datafusion/physical-expr/src/expressions/dynamic_filters/mod.rs:464:8 [...] ``` Fix this by appropriately gating the compilation of `from_parts`. ## What changes are included in this PR? * Squelch unused code warning ## Are these changes tested? Yes. ## Are there any user-facing changes? No.

…w retract (apache#23913) ## Which issue does this PR close? - Closes apache#23912. ## Rationale for this change `DistinctPercentileContAccumulator` reused the shared set-based `GenericDistinctBuffer`, which doesn't fit it: (1) the buffer asserts a single input column but `percentile_cont` passes two (value + percentile), so every `percentile_cont(DISTINCT ...)` panicked; (2) the buffer is a plain `HashSet` with no multiplicity, so sliding-window `retract_batch` dropped a value while duplicates were still in the frame. ## What changes are included in this PR? - Replace the shared buffer in this accumulator with a per-accumulator `HashMap<Hashable, usize>` count map: `update_batch` reads only the value column and increments; `retract_batch` decrements and removes a key only at zero; `state`/`merge_batch` keep the same List state shape. Other `GenericDistinctBuffer` users are untouched. - Regression tests in `aggregate.slt` for the plain distinct query and the sliding-window duplicate-retract case. ## Are these changes tested? Yes — new regression tests; the full `aggregate.slt` suite passes. ## Are there any user-facing changes? `percentile_cont(DISTINCT ...)` now works instead of panicking, and returns correct results in sliding windows.

## Which issue does this PR close? - Closes apache#23717. ## Rationale for this change Spark and Java format negative numeric values with parentheses when the `(` flag is present. The decimal formatting path always emitted a minus sign because it ignored `negative_in_parentheses`, while the floating-point path already handled the flag. This made `format_string` inconsistent across numeric input types. ## What changes are included in this PR? - Add a closing suffix when a negative decimal uses parentheses formatting. - Include the suffix when calculating width, left alignment, and zero padding. - Add regression coverage for grouped negative decimals with and without an explicit width. ## Are these changes tested? Yes. The following checks pass locally: - `cargo fmt --all -- --check` - `cargo clippy --all-targets --all-features -- -D warnings` - `cargo test -p datafusion-spark --lib` - The full workspace test command from the contributor guide with `avro,json,backtrace,extended_tests,recursive_protection,parquet_encryption` enabled ## Are there any user-facing changes? Yes. Spark-compatible formatting of negative decimal values now honors the parentheses flag. There are no public API or breaking changes.

codecov-commenter · 2026-08-12T18:49:45Z

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

Thanks for integrating Codecov - We've got you covered ☂️

…lan` API with `replace_children` (apache#24296) Backport of apache#23903 to `branch-55`. Co-authored-by: Andrew Lamb <[email protected]>

Additional commits were added into `branch-55` so a new changelog was generated.

…ache#24345) (apache#24367) This is a backport of apache#24345 for the release 55.0.0.

@timsaucer

…row groups (apache#24352) (apache#24368) Backport of apache#24354 to `branch-55` for the 55.0.0 release, per @timsaucer's request in apache#22393. ## Which issue does this PR close? - Backports the fix for apache#24352 (wrong TopK results from re-reading already-delivered row groups). ## Rationale apache#24352 is a **silent wrong-results** bug: with `pushdown_filters=true` + TopK dynamic filter pushdown (both on by default), a row group whose post-predicate selection is empty is finished by arrow-rs without handing back a reader, so DataFusion's `rg_plan` trails the decoder frontier by one and a later runtime prune rebuilds the decoder from a stale plan — re-reading an already-delivered row group, duplicating rows and dropping the true top-k tail. No error is raised. This is a clean cherry-pick of the squashed apache#24354 commit (`574fe67`); it applies to `branch-55` without conflicts. ## What changes are included? `push_decoder.rs`: sync `rg_plan` to the decoder frontier via `peek_next_row_group()` before each runtime prune/rebuild (gated on `row_group_pruner.is_some()` so ordinary scans pay nothing), with a defensive `internal_err!` if the frontier diverges from the plan. Plus the slt + rust regression tests from apache#24354. cc @timsaucer @alamb @adriangb

@timsaucer

…ection is live (apache#24355) (apache#24374) Backport of apache#24359 to `branch-55` for the 55.0.0 release, per @timsaucer's request in apache#22393. Stacks cleanly on the already-merged apache#24368 (apache#24354 backport). ## Which issue does this PR close? - Backports the fix for apache#24355 — a second, independent silent wrong-results bug in the same parquet dynamic row-group pruning path as apache#24352. ## Rationale With `pushdown_filters=true` + a TopK dynamic filter, the runtime row-group pruner rebuilds the push decoder via `into_builder().with_row_groups(...)`, which drops row groups **without slicing** the carried flat page-index `RowSelection` to match — a dropped RG's selectors are then applied to the next surviving RG, silently returning wrong rows (no error). The fix declines to build the runtime `RowGroupPruner` when a row selection is present (correctness over the pruning optimization); the proper fix that keeps both is tracked upstream in apache/arrow-rs#10624 / apache#24358. ## Notes - Clean cherry-pick of apache#24359 onto `branch-55` (which now has apache#24354 via apache#24368). No conflicts. - apache#24359 is **approved** on `main` and pending merge; opening this now so it can ride RC3. - Verified locally on this branch: the full `dynamic_row_group_pruning` rust module (9/9) and `dynamic_row_group_pruning.slt` pass; clippy clean. cc @timsaucer @alamb @adriangb

…d/prepend/replace - apache#24365 (apache#24377) This is a back port of apache#24365 into `branch-55`

New commits were added to branch-55 so the changelog is updated.

## Which issue does this PR close? - Relates to apache#22989 but does not close it. More types are coming. ## Rationale for this change - Support the Arrow type `Dictionary` for `approx_distinct` ## What changes are included in this PR? - Enable `HLLAccumulator` and `HllGroupsAccumulator` to support `Dictionary` - Tests ## Are these changes tested? Yes ## Are there any user-facing changes? Yes, `approx_distinct` supports now `Dictionary` but no breaking changes.

…anch-55 [Cherry-pick] Add support for dictionary for approx_distinct (apache#24646)

…r-defined types (apache#25015) ## Which issue does this PR close? N/A ## Rationale for this change `consume_user_defined_type` maps a Substrait user-defined type straight to a bare `DataType`, so a consumer has no way to mark the resulting `Field` for a downstream reader to recover semantic type lost in that mapping. For example, a UDT representing JSON is commonly mapped to plain `Utf8`, which is then indistinguishable from a real string column once it reaches a consumer that only sees the Arrow schema (e.g. over Arrow Flight, with no access to the original plan). ## What changes are included in this PR? - Adds `consume_user_defined_type_metadata`, a second, additive extension point on `SubstraitConsumer` defaulting to `Ok(None)`. - Applies its result to the `Field` built in `from_substrait_struct_type`, via `Field::with_metadata`. - Existing consumers are unaffected: the default keeps today's behavior exactly (no metadata attached). ## Are these changes tested? Yes, two new unit tests: a consumer that supplies metadata for a `UserDefined` field gets it attached to the field (data type unchanged), and a consumer returning `None` (the default) attaches nothing. ## Are there any user-facing changes? No behavior change for existing consumers. This is a new, optional trait method with a default implementation. (cherry picked from commit d682553)

#183) ## Which issue does this PR close?  - works towards closing apache#22682. - replacement for apache#21765 - - apache#21765 (review) ## Rationale for this change This PR introduces a specialized `GroupColumn` implementation for dictionary-typed columns inside `GroupValuesColumn`, allowing dictionary columns to participate in the columnar, vectorized aggregation path instead of the row-based fallback. **The Implementation is only about 175+ lines of code**. the remaining LOC is adding extensive test at the `GroupColumn` trait level as well as testing the `GroupValuesColumn` GroupValues trait and how it inter-opts with multi-dictionary group by's.  ## What changes are included in this PR? - Adds a `DictionaryGroupValueBuilder` struct implementing the `GroupColumn` trait for `Dictionary`-typed group-by columns, supporting a configurable subset of value types - Extends the type-check gate in `GroupValuesColumn::try_new` (the `matches!` block) to accept `Dictionary(_, value_type)` where `value_type` is already supported. - Adds schema-level support so emitted dictionary group key columns round-trip through the output schema correctly - [removes casting ](https://github.com/apache/datafusion/blob/9e8dd76d6deb6736c51962d9c97e04be4e3f1fc9/datafusion/physical-plan/src/aggregates/group_values/multi_group_by/mod.rs#L1200)thats done for each dictionary array in `emit`  ## Are these changes tested? yes. a majority of this PR is test  ## Are there any user-facing changes? no. this is a pure perf boost for users.   (cherry picked from commit c1b39bd) Co-authored-by: RIchard Baah <[email protected]>

## Which issue does this PR close?  - Closes apache#23921. - this PR was created on top of apache#24416, so once that is merged in we can rebase this branch ## Rationale for this change append_val is called once per new group on the scalar (streaming) code path. Previously it hashed a single-element slice of the dictionary values array on every call, paying create_hashes fixed overhead 65k times for a high-cardinality batch. Caching the full values array hash keyed on Arc::ptr_eq collapses that to one vectorized hash pass per batch, yielding a 2× speedup on the all-unique case with no measurable regression elsewhere.  ## What changes are included in this PR? - DictionaryGroupValuesColumn gains a cached_values: Option<ArrayRef> field - append_val: on a cache miss (Arc::ptr_eq fails), hashes the entire dict.values() array into val_hashes and stores the Arc; on a hit, reuses val_hashes[val_idx] directly — eliminating the per-call slice(val_idx, 1) allocation and create_hashes call  ## Are these changes tested? nothing logically changed.  ## Are there any user-facing changes? no  --------- (cherry picked from commit 66a901f) Co-authored-by: RIchard Baah <[email protected]> Co-authored-by: Andrew Lamb <[email protected]>

…he#25185) (#185) ## Which issue does this PR close? - Closes #. ## Rationale for this change `vectorized_append` is the path taken when we have non streaming hash aggregations (when there is no ordering in the `group by`), right now it re hashed the entire dictionary values on every batch unlike append_val which that only does this if the values array was already seen before. ## What changes are included in this PR? `vectorized_append` now reuses the cached value hashes when a batch carries the same dictionary values array as the previous one (this is the common case of a repartition or filter), instead of re hashing the whole dictionary and re-initialising the map on every batch. This extends the existing `Arc::ptr_eq` cache from apache#24418, which only covered the scalar `append_val` path. ## What is the testing strategy for this PR? All dict tests pass, I have a PR for a benchmark that exercises this path -> apache#25198 ## Are there any user-facing changes? no, this is a pure perf PR (cherry picked from commit a0631ed)

…apache#25418) (#186) ## Which issue does this PR close? - Closes apache#25411 ## Rationale for this change `create_physical_expr` required an `Expr::LambdaVariable`'s field to equal the planning schema's field and returned a plan error otherwise. The two can legitimately differ when a plan producer records a type that Substrait cannot express, such as dictionary encoding or a string view (Utf8view), since the recorded field and the field derived by `lambda_parameters` come from independent sources. ## What changes are included in this PR? Cast the variable to the recorded type instead, so the declared type is enforced rather than asserted. This is the same approach already used for `ScalarFunction` arguments, which the `TypeCoercion` analyzer casts to their coerced types instead of requiring them to match. Incompatible types still fail, as a cast error. ## What is the testing strategy for this PR? I added - Unit tests covering dictionary-encoded and `Utf8View` list elements with the lambda variable declared as plain `Utf8`, plus plan-shape tests pinning that the cast is inserted on a mismatch and omitted when the types already agree. - A Substrait integration test with a plan whose read schema declares a dictionary-encoded list element while the lambda carries its own plain `Utf8` parameter type. ## Are there any user-facing changes? No, this is a bug fix (cherry picked from commit 5da95ac)

…pache#25259) (#187) - Closes apache#25244. - Closes apache#25262. - Closes apache#25263. - Closes apache#25264. - Closes apache#21246. - Closes apache#25296. Enabling `datafusion.optimizer.enable_join_dynamic_filter_pushdown` can silently drop matching rows when the probe side of a join contains several columns with the same name, for example `a.id` and `b.id` from a nested join. Physical filter pushdown resolved a pushed filter's columns in the child schema by name, so a predicate on the second `id` column was rewritten to the first `id` column, which holds a different value. The same name-based mapping also affects TopK dynamic filters under the default configuration (apache#25296). For a nested join of orders and payments that both expose `amount`, `ORDER BY p.amount LIMIT 1` can push the payment threshold onto `orders.amount`. Later matching orders are incorrectly pruned, returning `(100, 30)` instead of `(300, 10)`. Every operator that forwards parent filters had the same weakness, in slightly different forms: - `HashJoinExec` computed which output columns belong to each side by position, but then resolved the child column by name. - `FilterExec`, `SortExec`, `RepartitionExec`, `CoalesceBatchesExec`, `UnionExec` and similar nodes used the generic name lookup even though their output positions equal their input positions. - `FilterExec` with an embedded projection resolved unconsumed parent filters back into input coordinates by name. - `ProjectionExec` looked up output aliases by name to find the expression to substitute, so two outputs aliased `id` both mapped to the first. - `AggregateExec` restricted pushdown to grouping positions but resolved the input column by name. The join-filter regression queries return one row with dynamic filtering disabled and zero rows with it enabled across these shapes. The `FilterExec`, `ProjectionExec` and `AggregateExec` cases were found while auditing the remaining name-based paths for apache#25244; they share the root cause, so this PR fixes them together. The TopK case in apache#25296 is fixed by the same positional mapping. Built-in operators now remap filter columns by position. The deprecated public API retains its historical name-based resolution for compatibility. - `FilterRemapper` uses a `ColumnMapping` enum: `Identity` preserves positions and checks that names match; `Explicit` uses a caller-supplied parent-output to child-input mapping and allows names to differ. - `ChildFilterDescription::from_child` now maps by position. New `ChildFilterDescription::from_child_with_column_mapping` takes an explicit `HashMap<usize, usize>`. - `HashJoinExec` builds the explicit mapping from its `column_indices` and output projection. For semi joins, output join keys on the emitted side are mapped to the paired key on the other side, which also supports differently named keys. - `FilterExec` maps through its embedded projection in both pushdown phases and when folding unconsumed parent filters back into its predicate. - `ProjectionExec` substitutes the expression at each output position instead of looking the alias up in the output schema. - `AggregateExec` maps each grouping output position to the input column that grouping expression reads; non-column grouping expressions are not forwarded, as before. - `ChildFilterDescription::from_child_with_allowed_indices` is kept as a deprecated wrapper that preserves name-based resolution to the first matching child field. It translates allowed parent column references into an explicit positional mapping; new callers should supply positions directly to avoid ambiguous duplicate names. - `dynamic_filter_pushdown_config.slt` gains four regression queries over two small Parquet tables, run once with join dynamic filtering disabled and once enabled, asserting identical rows: nested joins with `RepartitionExec` between them (the query from apache#25244), a `FilterExec` with an embedded projection, a `ProjectionExec` with duplicate aliases, and an `AggregateExec` grouping on same-named columns. All four return zero rows on `main` with dynamic filtering enabled. - `dynamic_filter_pushdown_config.slt` also covers apache#25296 using nested joins of orders, payments and customers, with one row per batch and one row per orders Parquet row group. The query returns `(300, 10)` with TopK dynamic filtering disabled, enabled, and enabled together with Parquet filter pushdown. On `main` at `85d4cbb0a9`, both enabled cases reproduce the incorrect `(100, 30)` result; all three pass on this branch. - `datafusion/core/tests/physical_optimizer/filter_pushdown.rs` gains focused tests that call `gather_filters_for_pushdown` directly on `RepartitionExec`, `HashJoinExec` (duplicate child columns with and without projection, both semi join directions with differently named keys, and a semi join whose key is not a plain column), `FilterExec` with a projection in both phases, `ProjectionExec` with duplicate aliases, and `AggregateExec` with reordered same-named grouping columns. Further tests cover the deprecated `from_child_with_allowed_indices` wrapper preserving the first name match, accepting allowed parent indices outside the child schema, and rejecting unresolvable names, the identity mapping rejecting a column whose name differs from the child field at that position, and `FilterExec` rejecting an unconsumed parent filter outside its projection. Passed locally on the updated implementation: - `cargo fmt --all` - `./ci/scripts/doc_prettier_check.sh --write --allow-dirty` - `cargo test --profile ci -p datafusion --test core_integration physical_optimizer::filter_pushdown` — 70 tests passed. - `cargo test --profile ci --test sqllogictests -- dynamic_filter_pushdown_config.slt` — passed. - The isolated apache#25296 regression fails on `main` in both TopK-enabled configurations with the expected wrong-result mismatch. Queries that push join or TopK dynamic filters through operators with duplicate column names now return the correct rows, including the default-configuration TopK query in apache#25296. API change in `datafusion-physical-plan`: `ChildFilterDescription::from_child_with_allowed_indices` is deprecated in favour of `from_child` and the new `from_child_with_column_mapping`; the deprecated function preserves its previous name-based behavior. Callers should migrate to explicit positions because duplicate names make name resolution ambiguous. `ChildFilterDescription::from_child` also resolves by position, which only affects callers that used it on a node whose output positions differ from its child's. (cherry picked from commit b376290) Co-authored-by: Huaijin <[email protected]>

…#25977) (#188)  Related to apache#23237 and [feat(metric): Add output skewness metric to detect skewed plans easier](apache#21211) - Closes #. I think it would be useful to add the skewness metric for nodes that execute in partitioned mdoe -- right now the only node that includes that metric in the explain analyze is DataSourceExec for parquet. This is specially useful to check how many partitions were idle during Aggregations or HashJoins for example. The PR adds a new metric `output_rows_skew` in the Explain Analyze output for the following nodes: - RepartitionExec - HashJoinExec - AggregateExec - BoundedWindowAggExec - WindowAggExec Note that for a plan that has a Parquet DataSource the skewness might not be the same as for the rest of the nodes, for example we might have the following scenario: ``` DataSourceExec [100, 100, 100, 100] → 0% skewness, data is even FilterExec [100, 0, 0, 0] → 100% (filter only matches in partition 0) AggregateExec [ 3, 0, 0, 0] → 100% (Partial: groups, not rows) RepartitionExec Hash [1, 1, 1, 0] → ~11% (3 groups re-spread by key) ``` I added a Explain Analyze test Yes, a new metric will show in the explain analyze plan for the nodes mentioned above. (cherry picked from commit c2baac5)

- Closes apache#24337. When using a projection as the percentile argument to `approx_percentile_cont` or `percentile_cont`, planning fails because only literals are supported. It makes sense to accept projections that are constant across all batches but not labelled as literals (although they practically are). For example, this query cannot be planned, even though the percentile is constant: ``` SELECT approx_percentile_cont(y, m) AS median FROM ( SELECT t.x + 1 as y, 0.5 as m FROM ( VALUES (10) ) AS t(x) ) ```  Introduce `PercentileParam` and `PercentileParamState` to handle column references or projections for the percentile argument. Resolution of `percentile` cannot always be done: for example, on an empty record batch, we won't be able to get the value from the columns. `PercentileParamState` makes sure the percentile resolution is both flexible enough to wait to be able to resolve the parameter AND applies the strict constraints that are required (constant `Float32` or `Float64`). The percentile is also included in state fields so that the merge_batch in the final Accumulator can process without any issues. This new feature is covered by the `sqllogictest` cases added in `aggregate.slt`. Unit tests have also been added to `approx_percentile_cont.rs` and `percentile_cont.rs`. No breaking changes. | Query | main | annarose_approx_percentile | Change | | ---------- | ---------- | ---------- | ---------- | | QQuery 0 | 0.73 ms | 0.79 ms | 1.08x slower | | QQuery 1 | 8.10 ms | 8.91 ms | 1.10x slower | | QQuery 2 | 27.90 ms | 30.59 ms | 1.10x slower | | QQuery 3 | 23.27 ms | 23.89 ms | no change | | QQuery 4 | 157.25 ms | 157.70 ms | no change | | QQuery 5 | 197.65 ms | 201.26 ms | no change | | QQuery 6 | 0.72 ms | 0.67 ms | +1.08x faster | | QQuery 7 | 8.20 ms | 8.28 ms | no change | | QQuery 8 | 230.84 ms | 215.32 ms | +1.07x faster | | QQuery 9 | 296.27 ms | 312.37 ms | 1.05x slower | | QQuery 10 | 49.12 ms | 47.66 ms | no change | | QQuery 11 | 58.86 ms | 55.26 ms | +1.07x faster | | QQuery 12 | 198.45 ms | 192.95 ms | no change | | QQuery 13 | 285.28 ms | 280.02 ms | no change | | QQuery 14 | 195.67 ms | 204.80 ms | no change | | QQuery 15 | 184.29 ms | 180.74 ms | no change | | QQuery 16 | 434.20 ms | 457.81 ms | 1.05x slower | | QQuery 17 | 476.52 ms | 427.84 ms | +1.11x faster | | QQuery 18 | 912.71 ms | 881.69 ms | no change | | QQuery 19 | 18.14 ms | 19.41 ms | 1.07x slower | | QQuery 20 | 456.87 ms | 463.23 ms | no change | | QQuery 21 | 468.67 ms | 482.15 ms | no change | | QQuery 22 | 892.28 ms | 896.13 ms | no change | | QQuery 23 | 2520.93 ms | 2558.49 ms | no change | | QQuery 24 | 34.14 ms | 33.50 ms | no change | | QQuery 25 | 99.14 ms | 99.83 ms | no change | | QQuery 26 | 35.25 ms | 34.51 ms | no change | | QQuery 27 | 467.74 ms | 484.36 ms | no change | | QQuery 28 | 1931.71 ms | 1945.50 ms | no change | | QQuery 29 | 29.16 ms | 30.35 ms | no change | | QQuery 30 | 214.69 ms | 209.17 ms | no change | | QQuery 31 | 196.43 ms | 199.65 ms | no change | | QQuery 32 | 583.78 ms | 614.36 ms | 1.05x slower | | QQuery 33 | 1078.73 ms | 1145.79 ms | 1.06x slower | | QQuery 34 | 1073.15 ms | 1117.56 ms | no change | | QQuery 35 | 182.87 ms | 171.52 ms | +1.07x faster | | QQuery 36 | 46.27 ms | 47.18 ms | no change | | QQuery 37 | 24.27 ms | 24.03 ms | no change | | QQuery 38 | 29.58 ms | 29.76 ms | no change | | QQuery 39 | 85.29 ms | 86.14 ms | no change | | QQuery 40 | 8.23 ms | 8.41 ms | no change | | QQuery 41 | 8.12 ms | 8.47 ms | no change | | QQuery 42 | 6.72 ms | 6.81 ms | no change | | Benchmark Summary | | | ---- | ---- | | Total Time (main) | 14238.19ms | | Total Time (annarose_approx_percentile) | 14404.87ms | | Average Time (main) | 331.12ms | | Average Time (annarose_approx_percentile) | 335.00ms | | Queries Faster | 5 | | Queries Slower | 8 | | Queries with No Change | 30 | | Queries with Failure | 0 | |Query | Time (ms) | Peak RSS | Peak Commit | Major Page Faults| |--------|----------------|-------------|--------------|-------------| |0|5.43|84.0 MB|171.8 MB|0| |1|11.45|119.6 MB|313.9 MB|0| |2|27.44|240.8 MB|1536.1 MB|0| |3|30.52|412.3 MB|1227.9 MB|0| |4|173.88|2.8 GB|3.2 GB|0| |5|193.60|2.5 GB|2.9 GB|0| |6|3.36|93.0 MB|167.9 MB|0| |7|10.66|128.1 MB|318.8 MB|0| |8|219.80|3.4 GB|4.1 GB|0| |9|268.65|2.4 GB|3.2 GB|0| |10|45.84|780.3 MB|1809.8 MB|0| |11|52.68|788.7 MB|1800.9 MB|0| |12|183.80|2.5 GB|2.9 GB|0| |13|266.55|3.0 GB|3.5 GB|0| |14|191.93|2.4 GB|3.2 GB|0| |15|185.03|3.0 GB|3.3 GB|0| |16|422.42|6.4 GB|7.1 GB|0| |17|404.75|6.1 GB|6.9 GB|0| |18|861.29|12.2 GB|13.1 GB|0| |19|19.20|362.3 MB|1118.0 MB|24| |20|442.22|1171.1 MB|1429.2 MB|0| |21|407.55|1836.8 MB|2.1 GB|0| |22|763.73|2.5 GB|2.8 GB|0| |23|2313.37|4.8 GB|5.5 GB|0| |24|33.92|569.7 MB|959.2 MB|5| |25|82.99|798.0 MB|1517.9 MB|0| |26|35.10|599.1 MB|986.8 MB|0| |27|392.34|1234.9 MB|1455.8 MB|0| |28|1714.18|2.5 GB|2.7 GB|0| |29|28.43|229.9 MB|1497.1 MB|0| |30|191.47|2.2 GB|3.3 GB|0| |31|182.56|2.8 GB|3.7 GB|0| |32|1263.74|13.3 GB|16.2 GB|0| |33|958.16|10.7 GB|10.9 GB|5| |34|956.65|10.6 GB|10.9 GB|0| |35|171.08|2.1 GB|2.6 GB|0| |36|47.33|452.0 MB|800.8 MB|0| |37|26.89|229.4 MB|544.9 MB|0| |38|33.62|270.9 MB|503.8 MB|0| |39|90.83|756.9 MB|1062.6 MB|0| |40|11.41|208.3 MB|433.3 MB|0| |41|11.47|170.3 MB|393.8 MB|0| |42|10.48|142.8 MB|265.1 MB|0| |Query | Time (ms) | Peak RSS | Peak Commit |Major Page Faults| |----------|--------------|------------|----------------|------------| |0|4.76|94.7 MB|171.5 MB|0| |1|11.22|120.7 MB|296.7 MB|0| |2|32.05|244.2 MB|1533.8 MB|0| |3|24.33|417.7 MB|1229.2 MB|0| |4|175.85|2.8 GB|3.2 GB|0| |5|207.07|2.5 GB|3.0 GB|0| |6|3.46|94.3 MB|171.6 MB|0| |7|11.08|129.4 MB|323.5 MB|0| |8|223.08|3.4 GB|4.1 GB|0| |9|288.63|2.4 GB|3.4 GB|0| |10|48.79|792.1 MB|1767.0 MB|0| |11|53.32|760.8 MB|1779.6 MB|0| |12|189.17|2.6 GB|3.1 GB|0| |13|274.96|3.0 GB|3.7 GB|0| |14|202.74|2.6 GB|3.3 GB|0| |15|208.83|3.0 GB|3.4 GB|0| |16|576.37|6.3 GB|7.1 GB|0| |17|623.87|6.5 GB|7.3 GB|0| |18|1140.54|11.3 GB|13.1 GB|0| |19|21.20|370.8 MB|1124.9 MB|26| |20|507.91|1149.3 MB|1364.8 MB|0| |21|479.85|1824.8 MB|2.1 GB|0| |22|914.26|2.3 GB|2.7 GB|0| |23|2757.47|4.6 GB|5.3 GB|0| |24|41.85|591.0 MB|999.1 MB|0| |25|91.00|763.3 MB|1500.8 MB|0| |26|38.90|560.1 MB|980.6 MB|0| |27|506.23|1319.5 MB|1550.4 MB|0| |28|2119.47|2.6 GB|2.7 GB|0| |29|34.79|213.7 MB|1267.2 MB|0| |30|218.63|2.1 GB|3.1 GB|0| |31|212.42|2.8 GB|3.7 GB|0| |32|821.63|13.0 GB|16.1 GB|0| |33|1162.42|10.5 GB|10.7 GB|14| |34|1212.91|10.4 GB|10.6 GB|0| |35|184.71|2.0 GB|2.5 GB|0| |36|58.35|469.5 MB|845.9 MB|0| |37|30.78|240.2 MB|545.1 MB|0| |38|35.43|287.3 MB|485.4 MB|0| |39|92.89|730.8 MB|1074.5 MB|0| |40|11.87|204.7 MB|420.5 MB|0| |41|11.43|171.2 MB|401.5 MB|0| |42|10.57|146.6 MB|275.6 MB|0| (cherry picked from commit 9504942)

feat: support col reference for percentiles (apache#25337)

…pache#26150) ## Which issue does this PR close? - None yet. The bug and its repro are below. ## Rationale for this change If `x` is a primary key, the optimizer treats `CAST(x AS ...)` as `x`. It then drops the other ORDER BY or GROUP BY keys that `x` determines, so the query returns wrong results: ```sql CREATE TABLE t (x DOUBLE, y VARCHAR, PRIMARY KEY (x)) AS VALUES (1.1, 'b'), (1.2, 'a'), (2.1, 'c'); SELECT x, y FROM t ORDER BY CAST(x AS INT), y; -- main: 1.1 b / 1.2 a / 2.1 c (`y` is dropped from the sort) -- expected: 1.2 a / 1.1 b / 2.1 c SELECT CAST(x AS INT) k, count(*) n FROM t GROUP BY CAST(x AS INT), y; -- main: (1, 2) / (2, 1) (`y` is dropped from the grouping) -- expected: (1, 1) / (1, 1) / (2, 1) SELECT y, count(*) FROM t GROUP BY CAST(x AS INT); -- main: accepted -- expected: Error: Column in SELECT must be in GROUP BY or an aggregate function ``` `CAST(x AS INT)` is named `t.x`, because casts are left out of expression names. The functional dependency helpers matched GROUP BY and ORDER BY expressions to key columns by comparing names, so the cast matched the key `t.x`. The same happens with `TRY_CAST`, with a `UNIQUE NOT NULL` key, and with a key that comes from an inner GROUP BY. The ORDER BY case comes from the sort key pruning added in apache#21362 (54.0.0). ## What changes are included in this PR? - **Index-based helpers.** The four helpers in `functional_dependencies.rs` now take, for each GROUP BY or ORDER BY expression, the index of the input field it references (`Option<usize>`) instead of its name. A computed expression has no index, so it never matches a key. The new `datafusion_expr::utils::passthrough_field_index` returns the index for a column reference, aliased or not. - **One GROUP BY list.** The dependencies of an `Aggregate`'s output use the GROUP BY list that its schema is built from (`grouping_set_to_exprlist`). Before, they used a list de-duplicated by name, which merged `CAST(x AS INT)` and `x`. - **Projections resolve columns.** A projection now finds each input column with `DFSchema::index_of_column_by_name` instead of comparing `"qualifier.name"` strings. The unit test `projection_duplicate_flattened_name_uses_first_input_index` asserted the old string behaviour: a column named `"orders.id"` got the dependency of the different field `orders.id`. It is renamed and now asserts that each column keeps its own dependency. ## What is the testing strategy for this PR? - **New tests.** Section 6 of `functional_dependencies.slt` has one query for each user of the helpers: - ORDER BY pruning; - GROUP BY pruning; - `Aggregate` output dependencies; - GROUP BY expansion. On `main`, each of these returns a wrong result or accepts an invalid query. The same queries also run over `TRY_CAST` on a `UNIQUE NOT NULL` key and over a key from an inner GROUP BY. One more query pins that a widening cast (`INT` to `BIGINT`) of a key does not carry its dependencies. - **New unit tests.** `passthrough_field_index` resolves an ambiguous unqualified column to nothing. A key that is in the GROUP BY list two times (`id, id AS k`, builder API only) still determines the row. - **Existing tests** are unchanged, apart from the rewritten unit test. - **Planning time.** Planning is not slower. The `sql_planner` TPC-H and TPC-DS benchmarks, whose tables have primary keys, are about 5% faster, because the new code no longer renders expression names. <details><summary>Benchmark numbers (3 interleaved runs per side)</summary> | Benchmark | `main` | This PR | |---|---|---| | `physical_plan_tpch_all` | 17.3 / 17.8 / 18.0 ms | 16.1 / 17.0 / 17.2 ms | | `physical_plan_tpcds_all` | 291 / 300 / 301 ms | 276 / 280 / 282 ms | </details> ## Are there any user-facing changes? - **Results.** The queries above return correct results. - **Rejected queries.** Only a column reference to a key carries its dependencies. A query that selects other columns while grouping by an expression over the key is now rejected, even when the expression keeps distinct values distinct, like `GROUP BY CAST(x AS BIGINT)` over an `INT` key. `main` accepts it and its result is correct. PostgreSQL and DuckDB reject it too. Group by the key column itself instead. - **API change.** Four `pub` functions in `datafusion_common` take `&[Option<usize>]` instead of `&[String]`: - `aggregate_functional_dependencies` - `get_target_functional_dependencies` - `get_required_group_by_exprs_indices` - `get_required_sort_exprs_indices` The upgrade guide has a migration note. (cherry picked from commit 28cefb7) Backport to branch-55: preserve the release's functional-dependency null semantics and projection layout, use Arrow 59 metadata types in tests, and place the migration note in the 55.0.0 upgrade guide.

Backport apache#26242 to branch-55 as a single commit. Bound preparation across sort partitions before spawning, retain reserved output until consumption, and preserve cancellation and run ordering. Adapt the change to branch-55's per-batch reservation model and inline test layout without importing the newer spill-workspace refactor. Source-PR: apache#26242 (cherry picked from commit 5abfeb9) (cherry picked from commit 702ed31) (cherry picked from commit f0ce6ef) (cherry picked from commit 69fb4e0) (cherry picked from commit 8178f21) (cherry picked from commit 42309e4) (cherry picked from commit c1a35a7) (cherry picked from commit f25c93e) (cherry picked from commit bb1ba6d) (cherry picked from commit ba797c0) (cherry picked from commit e015ef7) (cherry picked from commit aba15cd)

Cherry-pick apache#26242

…he-pr-26150 Cherry-pick apache#26150

## Which issue does this PR close? - Closes apache#24241 ## Rationale for this change This is a bugfix for a specific query shape. ## What changes are included in this PR? Changes the way leaf expressions are extracted when the alias is essentially the same as the column ## Are these changes tested? Yes, with a test that fails on main currently ## Are there any user-facing changes? Nope (cherry picked from commit 0fc962f)

…xtraction projection (apache#25412) ## Which issue does this PR close? None filed. ## Rationale for this change This query fails to plan on `main`: ```sql create table t(v int, s struct<a int>, env varchar) as values (1, {a: 10}, 'prod'), (2, {a: 20}, 'dev'); with samples as ( select v, s, env from t ), expanded as ( select v, s, env from samples union all select v, s, env from samples where 1 = 2 ) select env, sum(s['a']) from expanded group by env; ``` ``` Optimizer rule 'push_down_leaf_projections' failed caused by Schema error: Schema contains qualified field name samples.env and unqualified field name env which would be ambiguous ``` `enable_leaf_expression_pushdown` is on by default, so this is a plain planning failure for a valid statement. A `union all` whose one side is provably empty is what produces the shape, and query generators emit that: a dashboard panel adds a second branch behind a comparison of two constants and the panel stops working. ## What changes are included in this PR? `build_extraction_projection_impl` merges an extraction projection into the projection below it, and then adds the pass-through columns the merged projection does not already carry. It compared the columns it was about to add against the projection's own expressions without putting the two in the same space. A projection can list bare column names over a qualified input. Removing the empty side of the union leaves exactly that. The comparison then misses, the column is added a second time under its bare name, and `Projection::try_new` rejects a schema that holds `samples.env` and a bare `env` together. This PR resolves both sides against the input schema before the comparison, and pushes the column under the name the input gives it. A needed column that is not in the input, or that is ambiguous there, is skipped. Before this change such a column was added and `Projection::try_new` failed at the merge. A same-name alias such as `t.c AS c` counts as a pass-through on the existing side too, the same as a bare `t.c`. ## Are these changes tested? Yes. The statement above is added to `struct.slt`, with a result check and an `EXPLAIN` plan pin. It fails on `main` with the error above and passes with this change. Three unit tests in `extract_leaf_expressions.rs` cover the merge directly. All three fail on `main`: - `test_merge_bare_column_into_qualified_projection`: a filter names `a` over a projection that outputs `test.a`. - `test_merge_into_projection_with_same_name_alias`: the projection spells the pass-through as `test.a AS a`. - `test_merge_appends_qualified_column_for_bare_request`: the projection keeps only `test.a` and the merge needs a bare `b`. The appended column must be `test.b`. On `main` it is a bare `b`. `cargo test --workspace --exclude datafusion-sqllogictest` passes: 126 suites, 12479 tests, 0 failures. The sqllogictest suite passes too: 520 files, 0 failures. `cargo clippy -p datafusion-optimizer --all-targets` and `cargo fmt --all -- --check` are clean. ## Are there any user-facing changes? A statement of this shape plans instead of failing. No API change. ## Note on overlapping work apache#25388 (draft) edits the same arm of `build_extraction_projection_impl`, for a different problem: duplicated evaluation of `KeepInPlace` expressions, apache#25329. It does not touch the qualifier comparison this PR changes, so the two are independent in behaviour, but whichever lands second will need a small rebase at the tail of the `columns_needed` loop. apache#25282 also appends to the end of `struct.slt`, as this PR does. Keeping both blocks is the whole resolution. Part of the leaf-pushdown EPIC: apache#25459 --------- Co-authored-by: Claude Opus 5.5 <[email protected]> (cherry picked from commit 47135be)

…-pr-25412-20261103 Cherry-pick apache#25412

Phoenix500526 and others added 30 commits July 25, 2026 02:18

JSOD11 and others added 22 commits August 12, 2026 16:24

[branch-55] Backport of refactor(physical-plan): Simplify `ExecutionP…

83d3489

…lan` API with `replace_children` (apache#24296) Backport of apache#23903 to `branch-55`. Co-authored-by: Andrew Lamb <[email protected]>

[branch-55] Update changelog (apache#24314)

209fd94

Additional commits were added into `branch-55` so a new changelog was generated.

[branch-55] fix: correct list field inner type in array functions (ap…

85406f3

…ache#24345) (apache#24367) This is a backport of apache#24345 for the release 55.0.0.

[branch-55] fix: preserve the input list's inner field in array_appen…

520f389

…d/prepend/replace - apache#24365 (apache#24377) This is a back port of apache#24365 into `branch-55`

update changelog (apache#24385)

d555234

New commits were added to branch-55 so the changelog is updated.

fix: preserve projection metadata during optimization

525df19

Merge pull request #177 from DataDog/richard.baah/cherry-pick-dict-br…

3a95ddd

…anch-55 [Cherry-pick] Add support for dictionary for approx_distinct (apache#24646)

Merge pull request #190 from DataDog/annarose/approx-percentile

68678e2

feat: support col reference for percentiles (apache#25337)

Merge pull request #192 from DataDog/codex/cherry-pick-datafusion-26242

85faa92

Cherry-pick apache#26242

gh-worker-devflow-routing-ef8351 Bot mentioned this pull request Nov 3, 2026

Cherry-pick apache/datafusion#26150 #193

Merged

pabadrubio and others added 4 commits November 3, 2026 11:17

Merge pull request #193 from DataDog/pablo.abadrubio/cherry-pick/apac…

3363f79

…he-pr-26150 Cherry-pick apache#26150

Merge pull request #197 from DataDog/filippo.rossi/cherry-pick/apache…

9eda67e

…-pr-25412-20261103 Cherry-pick apache#25412

notfilippo closed this Nov 4, 2026

Rich-T-kid mentioned this pull request Nov 12, 2026

Branch 56 #211

Draft

Navigation Menu

Repository navigation

Branch 55 - #167

Branch 55#167
gene-bordegaray wants to merge 897 commits into
branch-54from
branch-55

gene-bordegaray commented Aug 12, 2026

Uh oh!

codecov-commenter commented Aug 12, 2026

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants

Conversation

gene-bordegaray commented Aug 12, 2026

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Uh oh!

codecov-commenter commented Aug 12, 2026

Welcome to Codecov 🎉

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

20 participants