feat(hash-aggr): Migrate Single/SinglePartitioned aggregation#5
Open
2010YOUY01 wants to merge 56 commits into
Open
feat(hash-aggr): Migrate Single/SinglePartitioned aggregation#52010YOUY01 wants to merge 56 commits into
2010YOUY01 wants to merge 56 commits into
Conversation
…Exec` on pushdown (apache#23199) ## Which issue does this PR close? None (Happy to open a tracking issue if preferred). ## Rationale for this change `EnforceSorting`/`EnsureRequirements` can produce a physically invalid plan that `SanityCheckPlan` rejects: ``` ... does not satisfy order requirements: [score@1 DESC NULLS LAST, id@0 ASC]. Child-0 order: [] ``` **Root cause.** In `sort_pushdown.rs::pushdown_requirement_to_children`, the fetch-forwarding branch ```rust } else if plan.fetch().is_some() && plan.supports_limit_pushdown() && plan.maintains_input_order().into_iter().all(|m| m) { ... Ok(Some(vec![Some(parent_required)])) // forwards the requirement UNCHANGED } ``` forwards the parent ordering requirement to the child **unchanged**. A `ProjectionExec` reports a `fetch()` (forwarded from its input) and can renumber/reorder columns, so the requirement, expressed in the projection's **output** schema, is pushed into the **child** schema verbatim. For a projection that reorders the sort key (e.g. output `[id, score, value]` over input `[id, value, score]`), `score@1` (valid above) is pushed down as `score@1` (a different column below). The relocated `SortExec` then advertises an ordering its child cannot provide, and `SanityCheckPlan` fails. This reproduces whenever a fetch-bearing global sort sits above a `CoalescePartitionsExec` over a column-reordering, order-preserving `ProjectionExec` whose input is already ordered, `parallelize_sorts` sinks the per-partition `SortExec` below the projection without remapping the key index. ## What changes are included in this PR? Remap the ordering requirement through the projection's column mapping before pushing it down (new `remap_requirement_through_projection` helper): - Each required column at output index `i` is rewritten to the column the projection produces at that index (`projection.expr()[i]`). - If any required column maps to a **computed** (non-`Column`) expression, the ordering cannot be expressed below the projection, so pushdown is declined and the sort is kept **above** the projection. - Hard/soft-ness and all alternatives of the `OrderingRequirements` are preserved; unsatisfiable alternatives are dropped. For non-reordering projections the remap is the identity, so existing plans are unchanged. ## Are these changes tested? Yes. - Unit tests for the remap helper: reordering remap, softness preservation, multiple hard alternatives, dropping an unsatisfiable alternative while keeping others, and declining when a required column is computed. - An end-to-end regression test (`test_parallelize_sorts_remaps_index_through_reordering_projection`) builds the multi-partition reordering-projection plan, runs `EnsureRequirements` with `repartition_sorts` enabled, and asserts the result passes `SanityCheckPlan`. It fails without this fix and passes with it. ## Are there any user-facing changes? No API changes. Plans that previously failed `SanityCheckPlan` (or produced incorrect orderings) for this shape are now valid; all other plans are unchanged. --------- Signed-off-by: jeadie <jack@spice.ai>
Bumps [cmov](https://github.com/RustCrypto/utils) from 0.5.3 to 0.5.4. <details> <summary>Commits</summary> <ul> <li><a href="https://github.com/RustCrypto/utils/commit/5c7e4f9bb31af81bf766360e836b6d633b84dbff"><code>5c7e4f9</code></a> cmov v0.5.4 (<a href="https://redirect.github.com/RustCrypto/utils/issues/1485">#1485</a>)</li> <li><a href="https://github.com/RustCrypto/utils/commit/87cadbce34655ac3c78efa7290f37d942d551b2c"><code>87cadbc</code></a> cmov: fix clippy (<a href="https://redirect.github.com/RustCrypto/utils/issues/1484">#1484</a>)</li> <li><a href="https://github.com/RustCrypto/utils/commit/85600e91cdf73115c48fafa64650ba9ed9285a12"><code>85600e9</code></a> rustfmt</li> <li><a href="https://github.com/RustCrypto/utils/commit/dba6c355c9f241e3726d5ec2a68f9f3b519f6063"><code>dba6c35</code></a> Merge commit from fork</li> <li><a href="https://github.com/RustCrypto/utils/commit/dad5e3b9e66d929e86144fe7c8f25371892e35f3"><code>dad5e3b</code></a> block-buffer: pin to <code>zeroize</code> v1.8 (<a href="https://redirect.github.com/RustCrypto/utils/issues/1483">#1483</a>)</li> <li><a href="https://github.com/RustCrypto/utils/commit/66cb272d00988520043aa34299a402abb885f461"><code>66cb272</code></a> ctutils: bump <code>subtle</code> version requirement to v2.6 (<a href="https://redirect.github.com/RustCrypto/utils/issues/1482">#1482</a>)</li> <li><a href="https://github.com/RustCrypto/utils/commit/34881f258468cc06c037ba53706429ba603ef63e"><code>34881f2</code></a> build(deps): bump hybrid-array from 0.4.11 to 0.4.12 (<a href="https://redirect.github.com/RustCrypto/utils/issues/1480">#1480</a>)</li> <li><a href="https://github.com/RustCrypto/utils/commit/c211865d9a51f42881d30c5e05070d53cb0373b7"><code>c211865</code></a> Update crates table (<a href="https://redirect.github.com/RustCrypto/utils/issues/1479">#1479</a>)</li> <li><a href="https://github.com/RustCrypto/utils/commit/9c8674f4fdf00fb524cd06d89da29d125db07582"><code>9c8674f</code></a> sponge-cursor: initial implementation (<a href="https://redirect.github.com/RustCrypto/utils/issues/1477">#1477</a>)</li> <li><a href="https://github.com/RustCrypto/utils/commit/a00167aa5bc1fcde165b8b02ca2f657e2ca08669"><code>a00167a</code></a> ctutils: fixup homepage url (<a href="https://redirect.github.com/RustCrypto/utils/issues/1478">#1478</a>)</li> <li>Additional commits viewable in <a href="https://github.com/RustCrypto/utils/compare/cmov-v0.5.3...cmov-v0.5.4">compare view</a></li> </ul> </details> <br /> [](https://docs.github.com/en/github/managing-security-vulnerabilities/about-dependabot-security-updates#about-compatibility-scores) Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting `@dependabot rebase`. [//]: # (dependabot-automerge-start) [//]: # (dependabot-automerge-end) --- <details> <summary>Dependabot commands and options</summary> <br /> You can trigger Dependabot actions by commenting on this PR: - `@dependabot rebase` will rebase this PR - `@dependabot recreate` will recreate this PR, overwriting any edits that have been made to it - `@dependabot show <dependency name> ignore conditions` will show all of the ignore conditions of the specified dependency - `@dependabot ignore this major version` will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this minor version` will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself) - `@dependabot ignore this dependency` will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself) You can disable automated security fix PRs for this repo from the [Security Alerts page](https://github.com/apache/datafusion/network/alerts). </details> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…une_by_bloom_filters` public (apache#23302) ## Which issue does this PR close? - N/A ## Rationale for this change `RowGroupAccessPlanFilter` is already public and re-exported at the crate root, and its `prune_by_statistics`, `prune_by_range`, and `prune_by_limit` methods are all `pub` — but `prune_by_bloom_filters` is still `pub(crate)`, and `BloomFilterStatistics` (the `PruningStatistics` adapter it consumes) is crate-private. So a custom `FileOpener` / `TableProvider` that builds its own `ParquetAccessPlan` can prune row groups by statistics, range, and limit through the public API, but has to carry a copy of the bloom-filter evaluation code to prune by bloom filters. We hit this downstream: a custom parquet opener that scans row groups in reverse order (for `ORDER BY ... DESC LIMIT k` workloads) currently maintains a private copy of this logic. Making these two symbols public lets such implementations reuse DataFusion's bloom-filter predicate evaluation and brings `prune_by_bloom_filters` to parity with its sibling `prune_by_*` methods. ## What changes are included in this PR? - Make `BloomFilterStatistics` and its `new` / `with_capacity` / `insert` methods `pub`, and re-export the type at the crate root. The `bloom_filter` module itself stays private (matching how `row_group_filter` is handled) to keep the added API surface minimal. - Make `RowGroupAccessPlanFilter::prune_by_bloom_filters` `pub`. - Drop the now-redundant `pub(crate)` re-export in `row_group_filter` and route the `opener` import through the crate root. Visibility only — no behavior change. ## Are these changes tested? Covered by existing tests (`cargo test -p datafusion-datasource-parquet` passes, including the `bloom_filter` pruning tests). No new behavior to test. ## Are there any user-facing changes? Two additions to the public API of `datafusion-datasource-parquet`: `BloomFilterStatistics` (crate-root re-export) and `RowGroupAccessPlanFilter::prune_by_bloom_filters`. No breaking changes.
## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes apache#123` indicates that this PR will close issue apache#123. --> - Part of apache#21937. ## Rationale for this change Running sql benchmarks using environment variables for configuration is awkward and error prone and strictly using criterion, while statistically much better, is quite slow compared to using simple iterations. This PR is the first version of a benchmark runner for sql benchmarks that will eventually use arguments for all benchmark configuration options. ## What changes are included in this PR? A simple benchmark runner that can list out the sql benchmarks and run a benchmark using iterations or criterion allowing for specifying a single query if desired. Future enhancements will use arguments for benchmark configuration vs just using environment variables as well as providing help and tying this into bench.sh ## Are these changes tested? Yes. I have a script that tests all current sql benchmarks both with and without criterion. Here is an portion of it for the single clickbench benchmark: ``` # clickbench single basic long flags env DATA_DIR=data CLICKBENCH_TYPE=single cargo run -p datafusion-benchmarks --bin benchmark_runner -- clickbench --query 0 --iterations 5 --output results/benchmark_runner/clickbench_single_long.json # clickbench single basic short flags env DATA_DIR=data CLICKBENCH_TYPE=single cargo run -p datafusion-benchmarks --bin benchmark_runner -- clickbench --query 0 -i 5 -o results/benchmark_runner/clickbench_single_short.json # clickbench single basic env iterations env DATA_DIR=data CLICKBENCH_TYPE=single ITERATIONS=5 cargo run -p datafusion-benchmarks --bin benchmark_runner -- clickbench --query 0 --output results/benchmark_runner/clickbench_single_env_iterations.json # clickbench single criterion with baseline env DATA_DIR=data CLICKBENCH_TYPE=single cargo run -p datafusion-benchmarks --bin benchmark_runner -- clickbench --query 0 --criterion --save-baseline benchmark_runner_acceptance # clickbench single criterion without baseline env DATA_DIR=data CLICKBENCH_TYPE=single cargo run -p datafusion-benchmarks --bin benchmark_runner -- clickbench --query 0 --criterion ``` The existing `cargo bench` approach still works the same (criterion only): ``` env DATA_DIR=data CLICKBENCH_TYPE=single BENCH_NAME=clickbench BENCH_QUERY=0 cargo bench -p datafusion-benchmarks --bench sql` ``` ## Are there any user-facing changes? No. --------- Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
## Which issue does this PR close? N/A ## Rationale for this change Drop finished stream that might hold on memory, this will reduce merge sort memory that one of the stream hold on memory reservation ## What changes are included in this PR? replaced finished stream in `FusedStreams` with `EmptyRecordBatchStream` once finished ## Are these changes tested? yes ## Are there any user-facing changes? no
## Which issue does this PR close? - Part of apache#19241 - closes apache#23013 ## Rationale for this change This is an alternate implementation proposal for optimizing small integer `IN` list from @geoffreyclaude - apache#23013 The in apache#23013 does some shenangans, including rebuilding the Arrow array data directly in order to probe a bitmap keyed by an unsigned type. Handling signed small integer types directly keeps the implementation simpler and avoids the extra array-data path while preserving the compact bitmap representation for these small domains. ## What changes are included in this PR? This updates the specialized `IN` list bitmap filter to handle `Int8`, `UInt8`, `Int16`, and `UInt16` directly. Signed small integer filters now map values to bitmap indexes by their bit pattern, so negative values use the same compact bitmap domain as their unsigned counterparts. ## Are these changes tested? Yes. This adds coverage for signed `Int8` and `Int16` bitmap filters, including boundary values and sliced arrays. ## Are there any user-facing changes? No. This is an internal physical expression optimization for small integer `IN` list filters.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48b2db5d3a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| )); | ||
| } | ||
|
|
||
| ControlFlow::Continue(original_state) |
There was a problem hiding this comment.
When enable_migration_aggregate routes a Single/SinglePartitioned DISTINCT aggregate with lim=[N] to this stream, this path always continues polling input after aggregating each batch and never checks agg.limit_options(). The older GroupedHashAggregateStream and the migrated partial/final streams stop reading and start output once the soft group limit is reached, so a single-stage SELECT DISTINCT a LIMIT 10 plan now scans all input before producing rows and an AggregateExec consumed directly can emit more than the pushed-down limit. Add the same soft-limit state/check here or avoid selecting this stream when limit_options is set.
Useful? React with 👍 / 👎.
## Which issue does this PR close? - Closes apache#22848. ## Rationale for this change External sort merge phases currently select spill files based only on memory reservation. With many small spills, a single phase can open enough files to exceed the process file-descriptor limit. ## What changes are included in this PR? - Add `datafusion.runtime.max_spill_merge_fan_in` (`0` preserves the current unlimited behavior). - Clamp non-zero values to at least 2 during merge selection so each pass makes progress. - Support builder configuration and dynamic SQL `SET` / `RESET` / `SHOW`. - Add unit, runtime SQL, SQLLogicTest, information schema, and generated documentation coverage. ## Are there any user-facing changes? Users can cap the number of spill files opened in one external merge pass. The default remains unchanged. ## How was this change tested? - `cargo test -p datafusion-execution test_max_spill_merge_fan_in_builder_and_dynamic_update --lib` - `cargo test -p datafusion-physical-plan spill_merge_fan_in --lib` - `cargo test -p datafusion --test core_integration test_max_spill_merge_fan_in_runtime_config` - `cargo test -p datafusion-sqllogictest --test sqllogictests -- set_variable.slt` - `cargo check -p datafusion` - `cargo clippy -p datafusion-execution -p datafusion-physical-plan -p datafusion --lib -- -D warnings` - `cargo fmt --all -- --check` - `dev/update_config_docs.sh` --------- Signed-off-by: Kevin-Li-2025 <2242139@qq.com> Co-authored-by: Kevin-Li-2025 <2242139@qq.com> Co-authored-by: Andrew Lamb <andrew@nerdnetworks.org>
…pache#23281) ## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes apache#123` indicates that this PR will close issue apache#123. --> - Closes apache#23261 . ## Rationale for this change Expand windows coverage to support `collect_set` `collect_list` functions. <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> ## What changes are included in this PR? <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> ## Are these changes tested? <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> ## Are there any user-facing changes? <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> <!-- If there are any breaking changes to public APIs, please add the `api change` label. -->
## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes apache#123` indicates that this PR will close issue apache#123. --> - Closes apache#22455 . ## Rationale for this change Extend pre commit instructions for AI agents to involve contributor machine to find issues before being committed. Each commit triggers a CI which in its turn asks ASF Infra for their precious resources and if we can find and fix as early as possible, much before utilizing ASF resources, would be amazing <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> ## What changes are included in this PR? <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> ## Are these changes tested? <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> ## Are there any user-facing changes? <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> <!-- If there are any breaking changes to public APIs, please add the `api change` label. -->
## Which issue does this PR close? - Closes apache#22187 ## Rationale for this change Presence of a non-ascii character in the value while setting any datafusion runtime variable was panicking. ## What changes are included in this PR? Better handling/iteration on character-boundary, and emits plan error now instead of panicking in case of such characters. ## Are these changes tested? Yes. UTs have been updated to include such cases. ## Are there any user-facing changes? No.
## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes apache#123` indicates that this PR will close issue apache#123. --> - Closes #. ## Rationale for this change CI more and more often fails with HTTP errors, during downloading from Cargo. This leads the entire CI needs to be rerun in the merge queue. Adding retry for Cargo HTTP and network settings. Example https://github.com/apache/datafusion/actions/runs/28677739346/job/85055401549?pr=23313 ``` error: failed to get `windows` as a dependency of package `sysinfo v0.39.5` ... which satisfies dependency `sysinfo = "^0.39.3"` (locked to 0.39.5) of package `datafusion v54.0.0 (/__w/datafusion/datafusion/datafusion/core)` Caused by: failed to load source for dependency `windows` Caused by: unable to update registry `crates-io` Caused by: download of wi/nd/windows failed Caused by: curl failed Caused by: [55] Failed sending data to the peer (OpenSSL SSL_read: SSL_ERROR_SYSCALL, errno 0) Error: Process completed with exit code 101. ``` <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> ## What changes are included in this PR? <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> ## Are these changes tested? <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> ## Are there any user-facing changes? <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> <!-- If there are any breaking changes to public APIs, please add the `api change` label. --> --------- Co-authored-by: Yongting You <2010youy01@gmail.com>
## Which issue does this PR close? - Closes apache#23246. ## Rationale for this change `datafusion-sql` used `StackGuard` to temporarily raise `recursive`'s process-global minimum stack size while planning or unparsing deep SQL structures. Because that global is shared across threads, one guard could restore a lower value while another thread was still relying on DataFusion's larger SQL red zone. ## What changes are included in this PR? This PR replaces the scoped global mutation with local `stacker::maybe_grow` checkpoints for the SQL planner/unparser recursion paths that need a 256 KiB red zone. The public planning and unparsing APIs are unchanged. It also adds regressions covering both non-mutating stack growth and deep unparsing while another thread repeatedly lowers `recursive`'s global minimum to the default value. ## Are these changes tested? Yes. - `cargo test -p datafusion-sql --features recursive_protection` - `cargo fmt --all --check` - `cargo clippy -p datafusion-sql --all-targets --all-features -- -D warnings` - `cargo clippy --all-targets --all-features -- -D warnings` ## Are there any user-facing changes? No.
## Which issue does this PR close? - None. ## Rationale for this change Inexact sort pushdown keeps the `SortExec` because the source can optimize for the requested ordering but cannot guarantee exact global ordering. When the rewritten standalone `SortExec` is a partition-preserving TopK over multiple partitions, each partition applies a local TopK. A later coalesce can then concatenate those local results, which can violate `ORDER BY ... LIMIT` semantics. For example, with two partitions containing `[1, 100]` and `[2, 3]`, `ORDER BY a ASC LIMIT 3` should return `1, 2, 3`, but the unmerged local TopK path can return `1, 100, 2, 3`. ## What changes are included in this PR? - Wrap standalone multi-partition inexact partition-preserving TopK pushdown with `SortPreservingMergeExec` and preserve the TopK fetch on the merge. - Stop traversal after rewriting inexact `SortPreservingMergeExec -> SortExec` and after inserting the standalone merge, so the newly inserted child `SortExec` is not processed again as a standalone TopK. - Add a plan regression test that asserts the final merge is inserted. - Add an execution regression test using an inexact executable memory source to assert the global TopK result is returned. ## Are these changes tested? Yes: I also verified the new execution regression fails on unmodified `upstream/main`: the plan returns `1, 100, 2, 3` instead of the expected global TopK result `1, 2, 3`. ## Are there any user-facing changes? Yes. This fixes incorrect results for `ORDER BY ... LIMIT` queries when inexact sort pushdown produces a standalone partition-preserving TopK over multiple input partitions.
## Which issue does this PR close? - Part of apache#19241 - part of apache#23307 - Follow on to apache#23299 ## Rationale for this change We (mostly @geoffreyclaude ) are in the process of optimizing IN lists with specialized implementations for various different data types. @kosiew suggested in apache#23299 (comment) that we add SQL level coverage for the IN list optimizations to both cover the dispatch logic as well as ensure everything works end to end. > One small follow-up idea: it could be useful to add a public-path regression through InListExpr::try_new_from_array, or a SQL-level test, for Int8 and Int16 signed values. ## What changes are included in this PR? 1. Add `in_list.slt` file to specifically target the IN lists 2. Only add test coverage for integer types note I think we should have significiantly more SLT coverage (list on apache#23307) but to keep the review small / understandable I plan to do it with several PRs ## Are these changes tested? Only tests ## Are there any user-facing changes? No
…it (apache#23277) ## Which issue does this PR close? - Closes apache#23219. ## Rationale for this change The query from the issue: ```sql SELECT (((Cast(id AS BIGINT) % 1024) + 1024) % 1024) AS computed_bucket FROM profile ORDER BY computed_bucket, Cast(id AS BIGINT) limit 10; ``` panics: ``` thread 'main' panicked at .../datafusion-datasource-54.0.0/src/statistics.rs:100:48: index out of bounds: the len is 0 but the index is 0 ``` The underlying issue is that the current code panics when files are split by statistics and there are no statistics available for the column where the sort order is defined in this case `computed_bucket`. ## What changes are included in this PR? - Fix in `MinMaxStatistics` to check if there are stats available for a given column - Test ## Are these changes tested? Yes ## Are there any user-facing changes? No
…ation_invariant_checker (apache#23323) ## Which issue does this PR close? - Closes apache#20786 ## Rationale for this change `test_optimization_invariant_checker` panics under `release-nonlto` profile which has debug_assertions=false. Thus, we need to gate ## What changes are included in this PR? Make sure debug_assertion required places are behind that gate ## Are these changes tested? Yes and this is test only change. ``` cargo test physical_planner --profile release-nonlto ``` now passes since we skip these checks as they are not supported in that profile ## Are there any user-facing changes? No
## 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 `Duration` type for `approx_distinct` - The Arrow type `Duration` can be directly supported for `NumericHLLAccumulator` and `HllGroupsAccumulator` ## What changes are included in this PR? - Enable `NumericHLLAccumulator` and `HllGroupsAccumulator` to support `Duration` - Tests for the non-grouped and grouped path as part of `aggregate.slt` ## Are these changes tested? Yes ## Are there any user-facing changes? Yes, `approx_distinct` supports now `Duration` but no breaking changes.
## Which issue does this PR close? N/A ## Rationale for this change `datafusion-ffi` inherits `datafusion-proto` from the workspace. Because `datafusion-proto` enables its `parquet` feature by default, downstream crates that depend on `datafusion-ffi` also pull in `datafusion-datasource-parquet`, `parquet`, and parquet compression dependencies even when they do not need parquet-aware proto support. Cargo features are additive, so downstream users cannot opt out by disabling default features on their own direct `datafusion-proto` dependency if the `datafusion-ffi -> datafusion-proto` edge has already enabled defaults. ## What changes are included in this PR? This PR makes the workspace `datafusion-proto` dependency default to `default-features = false`, then lets crates opt into the previous behavior explicitly. For `datafusion-ffi`, the previous default behavior is preserved with a new default `parquet` feature. Downstream users that want to avoid parquet dependencies can now use `datafusion-ffi` with `default-features = false`. The FFI session table option bridge now gates `ConfigFileType::PARQUET` usage behind the `datafusion-ffi/parquet` feature, while still preserving parquet table option values in no-default builds. ## Are these changes tested? Yes. Existing `datafusion-ffi` tests pass with default features and with `--no-default-features`. The feature graph was also checked to confirm that default `datafusion-ffi` still enables parquet, while `datafusion-ffi --no-default-features` does not include `datafusion-datasource-parquet`. ## Are there any user-facing changes? Default behavior is unchanged. Users can now opt out of parquet-aware proto support from `datafusion-ffi` by disabling `datafusion-ffi` default features.
## Which issue does this PR close? - Part of apache#23334. ## Rationale for this change In order to correctly characterize a performance improvement for array_has with column needle, needed more benchmark cover. ## What changes are included in this PR? Adds criterion benchmarks for `array_has` with an **array (column) needle** — the path through `array_has_dispatch_for_array`, with no functional change. Groups added in `datafusion/functions-nested/benches/array_has.rs`: - `array_has_array_i64` / `array_has_array_strings` — found / not-found over list sizes 10/100/500. - `array_has_array_null_patterns` — null patterns at list length 64: for i64, no nulls (found + not found) / 30% nulls found / 30% nulls not found / all null / null-fill collision; for Utf8, LargeUtf8, and Utf8View, no-nulls / 30% nulls at both short (inline, ≤ 12 byte) and long (> 12 byte, shared-prefix) element lengths, plus all-null. - `array_has_array_by_size` — i64, 30% element nulls, not found, list length 8..1024. - `array_has_array_by_rows` — i64, 8 elems/row, 30% nulls, not found, 10K / 100K / 1M rows. ## Are these changes tested? no functional change here ## Are there any user-facing changes? no --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
## 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 `BinaryView` type for `approx_distinct` - The Arrow type `BinaryView` can be directly supported for `HLLAccumulator` and `HllGroupsAccumulator` ## What changes are included in this PR? - Enable `HLLAccumulator` and `HllGroupsAccumulator` to support `BinaryView` - Tests for the non-grouped and grouped path as part of `aggregate.slt` ## Are these changes tested? Yes ## Are there any user-facing changes? Yes, `approx_distinct` supports now `BinaryView` but no breaking changes.
## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes apache#123` indicates that this PR will close issue apache#123. --> - Closes apache#23072. ## Rationale for this change <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> File statistics are computed against a specific `file_schema` (their `column_statistics` are positional, one per column), but the file-statistics cache was keyed only by table and path. Reading the same path under a different schema could therefore reuse statistics whose columns no longer line up, panicking during statistics projection. apache#22950 worked around this by **bypassing** the file-statistics cache entirely for anonymous explicit-schema reads — correct, but it gave up cache reuse for them (every such read recomputes statistics). apache#23072 asks to make the cache itself schema-aware so those reads can reuse the cache safely instead of skipping it. ## What changes are included in this PR? <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> - Add `SchemaFingerprint` — the per-column `(name, data_type, nullable)` of a `file_schema`, in order — and `FileStatisticsCacheKey { table, path, schema }`, and key the file-statistics cache on it (`FileStatisticsCache` is now `dyn Cache<FileStatisticsCacheKey, CachedFileMetadata>`). - `ListingTable::do_collect_statistics_and_ordering` builds the key with the `file_schema` fingerprint and uses the shared cache directly. The apache#22950 bypass (`statistics_cache` helper / `schema_source`-based skip) is removed: different schemas now land in distinct entries (no stale cross-schema reuse), while a repeated read of the same schema reuses its entry. - The fingerprint deliberately **excludes** field/schema metadata (it cannot affect statistics, and including it would needlessly fragment the cache) and partition columns (partition statistics are computed separately, outside this cache). - Table-drop invalidation is unchanged: `drop_table_entries` matches on `CacheKey::table_ref()`, which still returns the table, so all schema variants for a dropped table are removed together. - The list-files cache continues to key on `TableScopedPath`. ## Are these changes tested? <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> Yes. - Updated the apache#22950 regression test (`anonymous_parquet_stats_cache_with_explicit_wider_schema`): the wider explicit-schema read now lands in its own cache entry (2 entries, was 1 under the bypass) with correct statistics and no panic, and a repeated read of that schema is served from the cache (a cache hit, no new entry). - Added unit tests for `SchemaFingerprint`: it distinguishes nullability and field order, and ignores field/schema metadata. - `cargo test` for the `file_statistics` integration module and the `datafusion-execution` cache tests (including `drop_table_entries`) pass, along with `cargo fmt --all` and `cargo clippy --all-targets --all-features -- -D warnings` for the touched crates. ## Are there any user-facing changes? <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> <!-- If there are any breaking changes to public APIs, please add the `api change` label. --> No change to query results, physical plans, or the serialized (proto) wire format; file statistics are computed exactly as before. One public API change (please add the `api change` label): the `FileStatisticsCache` type alias now uses `FileStatisticsCacheKey` instead of `TableScopedPath` as its key. Code that constructed keys for this cache directly must switch to `FileStatisticsCacheKey`. `SchemaFingerprint` and `FileStatisticsCacheKey` are newly public; `TableScopedPath` remains (still used by the list-files cache). `cargo-semver-checks` will flag the key-type change, which is expected. --------- Signed-off-by: Jiawei Zhao <Phoenix500526@163.com>
## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes apache#123` indicates that this PR will close issue apache#123. --> - Closes apache#22806 . ## What changes are included in this PR? <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> Dataframe::fill_null to take its arguments by reference (&scalarvalue, &[&str]) instead of by value, aligning its signature with fill_nan. Documented the migration in. ## Are there any user-facing changes? <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> This is a breaking change to the public DataFrame::fill_null API. Could whoever merges please add the `api change` label. <!-- If there are any breaking changes to public APIs, please add the `api change` label. -->
…pache#23319) ## Which issue does this PR close? * Part of apache#22772 ## Rationale for this change This change adds focused regression coverage for the current dynamic-filter expression assembly performed by `SharedBuildAccumulator::build_filter`. The intent is to make the existing expression policy explicit before refactoring, ensuring that future changes preserve the current pruning semantics for both `CollectLeft` and `Partitioned` finalize paths. ## What changes are included in this PR? * Add reusable test helpers for constructing `SharedBuildAccumulator` instances, partition state, bounds, and pushdown strategies. * Add unit tests covering `CollectLeft` dynamic-filter expression assembly for: * membership-only filters, * bounds-only filters, * empty build data that should not update the dynamic filter. * Add unit tests covering `Partitioned` dynamic-filter expression assembly for: * a single real partition with otherwise empty partitions (ensuring no unnecessary `CASE` expression), * canceled/unknown partitions (ensuring permissive fallback for unknown routes). * Add helper assertions that validate the structural shape of the generated expressions (for example, `InListExpr`, `BinaryExpr`, `CaseExpr`, and literal boolean expressions) rather than relying on brittle textual representations. ## Are these changes tested? Yes. This PR adds the following unit tests: * `collect_left_updates_with_membership_only` * `collect_left_updates_with_bounds_only` * `collect_left_empty_build_data_does_not_update_filter` * `partitioned_one_real_partition_with_rest_empty_skips_case` * `partitioned_canceled_unknown_partitions_keep_unknown_routes_permissive` These tests exercise the dynamic-filter expression policy without changing production behavior. ## Are there any user-facing changes? No. This PR only adds regression tests and does not change runtime behavior. ## LLM-generated code disclosure This PR includes LLM-generated code and comments. All LLM-generated content has been manually reviewed.
## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes apache#123` indicates that this PR will close issue apache#123. --> - Closes apache#23360 . ## Rationale for this change <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> Fix the audit issue of main branch. ## What changes are included in this PR? <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> ## Are these changes tested? <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> ## Are there any user-facing changes? <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> <!-- If there are any breaking changes to public APIs, please add the `api change` label. --> Signed-off-by: Jiawei Zhao <Phoenix500526@163.com>
Fixes a few docs typos
## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes apache#123` indicates that this PR will close issue apache#123. --> - Closes #. ## Rationale for this change <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> 54 is released so we can remove the note ## What changes are included in this PR? <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> ## Are these changes tested? <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> ## Are there any user-facing changes? <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> <!-- If there are any breaking changes to public APIs, please add the `api change` label. -->
## Which issue does this PR close? - Part of apache#19241. - Based on apache#23299. - Next in stack: apache#23014, after it is rebased onto this PR. ## Rationale for this change apache#23299 extends the bitmap `IN` filter to the signed 1-byte and 2-byte integer types by handling each logical Arrow type directly. `Float16` is the remaining 2-byte primitive type that can use the same compact bitmap idea: it has 65,536 possible bit patterns, so an 8 KiB bitmap can represent every possible value. This PR follows the same direct typed shape as apache#23299. It does not reinterpret whole arrays as `UInt16`; instead, the `Float16` bitmap filter maps each value to its IEEE-754 half-precision bit pattern with `to_bits()`. That keeps the logical array type intact while preserving bit-pattern equality semantics, including distinct NaN payloads and `+0.0` versus `-0.0`. ## What changes are included in this PR? - Adds `Float16Type` support to the existing `BitmapFilter`. - Routes `DataType::Float16` constant-list filtering to that bitmap path. - Extends the existing type-combination coverage to include `Float16`. - Adds focused coverage for slices, nulls, `NOT IN`, `+0.0` / `-0.0`, and NaN payload bit patterns. - Adds focused `in_list_strategy` benchmark rows for `Float16`. ## Are these changes tested? Yes. - `cargo fmt --all` - `cargo test -p datafusion-physical-expr bitmap_filter_f16 --lib` - `cargo test -p datafusion-physical-expr test_in_list_from_array_type_combinations --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 new Float16 rows: ```bash cargo bench -p datafusion-physical-expr --profile release-nonlto --bench in_list_strategy -- narrow_integer/f16 --save-baseline <baseline> ``` Compared baselines: [apache#23299](apache#23299) -> [apache#23311](apache#23311) Method: directly compared Criterion's raw sample minima (`min(time / iterations)`) from `sample.json`. Lower is better; changes within +/-5% are treated as noise. Summary: 6 relevant rows, 6 faster, 0 slower, 0 within +/-5%. | Benchmark | [apache#23299](apache#23299) | [apache#23311](apache#23311) | Change | |---|---:|---:|---:| | `narrow_integer/f16/list=4/match=0%` | 19.582 us | 3.911 us | -80.0% (5.01x faster) | | `narrow_integer/f16/list=4/match=50%` | 44.138 us | 3.871 us | -91.2% (11.40x faster) | | `narrow_integer/f16/list=64/match=0%` | 19.977 us | 3.878 us | -80.6% (5.15x faster) | | `narrow_integer/f16/list=64/match=50%` | 55.792 us | 3.903 us | -93.0% (14.29x faster) | | `narrow_integer/f16/list=256/match=0%` | 21.727 us | 3.885 us | -82.1% (5.59x faster) | | `narrow_integer/f16/list=256/match=50%` | 51.737 us | 3.918 us | -92.4% (13.21x faster) |
## Which issue does this PR close? - Closes apache#22512 ## Rationale for this change Introduce Decimal support for `trunc` - for feature parity and to solve overflowing issues with large integers ## What changes are included in this PR? - Decimal support for scalar and array cases - A helper for decimals - SLTs ## Are these changes tested? - Unit test for new math - SLTs - One specific SLT to showcase @neilconway case from apache#22512 ## Are there any user-facing changes?
## Which issue does this PR close? - Closes #. ## Rationale for this change DataFusion generally uses the latest stable Rust release for the workspace toolchain. `rustup check` reports an available stable update from `1.96.0` to `1.96.1`. ## What changes are included in this PR? - Updates the root `rust-toolchain.toml` channel from `1.96.0` to `1.96.1`. - Updates the contributor guide rust-analyzer command example to match the pinned toolchain. ## Are there any user-facing changes? No runtime or API changes. Developers and CI will use Rust `1.96.1` as the pinned workspace toolchain.
…zing low-cardinality columns (apache#22905) ## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes apache#123` indicates that this PR will close issue apache#123. --> - Related to apache#19458 - Related to apache#20935 ## Rationale for this change <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> When a `Dictionary(K, Utf8)` column is passed to a string scalar function, the type-coercion layer currently materializes it to flat Utf8/Utf8View before the function runs, so the operation is applied to every row instead of just the unique dictionary values, and the dictionary encoding is lost on the output. See apache#19458 for the underlying coercion behavior and apache#20935 for the string-function-specific impact. This is wasteful for low-cardinality columns and inflates Arrow IPC/Flight message sizes downstream. ## What changes are included in this PR? <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> - Add `EncodingPreservation { None, Dictionary }` and opt-in constructors on `Coercion` (`new_exact_preserving_encoding`, `with_encoding_preservation`). - In `get_valid_types`, when a `Coercible` arg requests `Dictionary` preservation, run coercion against the dictionary's value type and re-wrap the result as `Dictionary(K, V')`, so the function receives a `DictionaryArray`. - Make `lower/upper` opt in and handle dictionary inputs (array + scalar) by converting only the dictionary values and re-wrapping with the original keys. ## Are these changes tested? Yes <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> ## Are there any user-facing changes? Yes. upper/lower now return Dictionary(...) for dictionary inputs instead of the previously materialized Utf8View. New public API (EncodingPreservation, new Coercion constructors) is added — please add the api change label. <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> <!-- If there are any breaking changes to public APIs, please add the `api change` label. -->
## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes apache#123` indicates that this PR will close issue apache#123. --> - Closes apache#9158 ## Rationale for this change <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> See issue ## What changes are included in this PR? <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> Add test coverage, also fix issue with constructing scalars of FixedSizeLists with 0 size since we need to explicitly specify the length. ## Are these changes tested? <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> Yes ## Are there any user-facing changes? <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> No <!-- If there are any breaking changes to public APIs, please add the `api change` label. -->
## Which issue does this PR close? - Closes apache#23137 ## Rationale for this change Range repartitioning was already planned and serialized into physical plans, but `RepartitionExec` could not execute it. This PR completes the core execution path so rows in an input batch are routed to the correct output partition based on range split points and the ordering defined on the partitioning scheme. ## What changes are included in this PR? This PR adds a `Range` variant to `BatchPartitioner` that evaluates the ordering expressions on each input batch, compares each row's key against split points using `compare_rows` (respecting ASC/DESC and null ordering), and assigns row indices to output partitions via binary search. The partitioned row indices are then materialized into sub-batches using the same `partition_grouped_take` path as hash repartitioning. `pull_from_input` is wired to construct a range partitioner for `Partitioning::Range`, replacing the previous `not_impl_err!` at execution time. Optimizer-related paths remain intentionally unimplemented and are tracked in [apache#23230](apache#23230): projection pushdown through `RepartitionExec` (`try_swapping_with_projection`), sort pushdown (`try_pushdown_sort`), and changing partition counts via `repartitioned()`. ## Are these changes tested? Yes! ## Are there any user-facing changes? No public API changes
Allow `EnforceSorting` sort pushdown to remap ordering requirements through `ProjectionExec` when the required output columns are simple aliases or reordered input columns. ## Which issue does this PR close? Prior this patch sort pushdown stopped at `ProjectionExec`, even when the projection only changed column order. This could force a `SortExec` above the projection and therefore above larger subplans such as `UnionExec`, preventing branch-local sorts from being used. The new logic conservatively handles only column-to-column projections and leaves computed expressions untouched, preserving correctness while enabling better plans for reordered projection cases. - Closes #. ## Are these changes tested? Yes: * new test `test_push_sort_through_reordered_projection_to_union` added. * Existing tests are updated accordingly. ## Are there any user-facing changes? No Co-authored-by: Qi Zhu <zhuqilucas@gmail.com>
…tions (apache#23324) ## Which issue does this PR close? <!-- We generally require a GitHub issue to be filed for all bug fixes and enhancements and this helps us generate change logs for our releases. You can link an issue to this PR using the GitHub syntax. For example `Closes apache#123` indicates that this PR will close issue apache#123. --> - Part of apache#22710 - An alternative for and closes apache#23309 ## Rationale for this change <!-- Why are you proposing this change? If this is already explained clearly in the issue then this section is not needed. Explaining clearly why changes are proposed helps reviewers understand your changes and offer better suggestions for fixes. --> See apache#23309 and apache#23309 (comment) for background. I prefer this approach, but I want to point out the tradeoff for this PR's approach: the shared utility includes a complex lambda function argument, this makes them harder to extend. But I think it's okay since most functionality has been implemented for the refactor, and there are not likely to have new functional requirements. ## What changes are included in this PR? <!-- There is no need to duplicate the description in the issue here but it is sometimes worth providing a summary of the individual changes in this PR. --> ## Are these changes tested? <!-- We typically require tests for all PRs in order to: 1. Prevent the code from being accidentally broken by subsequent changes 2. Serve as another way to document the expected behavior of the code If tests are not included in your PR, please explain why (for example, are they covered by existing tests)? --> ## Are there any user-facing changes? <!-- If there are user-facing changes then we may require documentation to be updated before approving the PR. --> <!-- If there are any breaking changes to public APIs, please add the `api change` label. -->
…e impls (apache#23349) ## Which issue does this PR close? - Closes apache#22867 ## Rationale for this change DFHeapSize depends on a critical invariant: shared heap allocations must be counted once per traversal context. If Arc implementations drift, memory accounting becomes inconsistent and hard to reason about. Consolidating the one-time-accounting logic improves: Correctness durability (single source of truth for Arc dedup behavior) Maintainability (less repeated logic) Reviewability (future changes touch one helper) ## What changes are included in this PR? Helper functions for `Arc` allocation identity and deduplication, namely for providing pointer extraction & `DFHeapSizeCtx` set membership checks, with the `DFHeapSize` implementations on `Arc` backed types using the new helpers ## Are these changes tested? Yes, the below are passing: ``` cargo test -p datafusion-common heap_size --lib cargo test -p datafusion-common --doc heap_size ``` ## Are there any user-facing changes? No
2010YOUY01
force-pushed
the
split-aggr-single
branch
from
48b2db5 to
34baaae
Compare
github-actions
Bot
added
documentation
2010YOUY01
force-pushed
the
split-aggr-single
branch
from
34baaae to
3894ebe
Compare
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
20 participants
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?
Part of apache#22710
Rationale for this change
See issue for details.
This PR is stacked on apache#23309
What changes are included in this PR?
Migrate single/signle-partitioned aggregation paths
Are these changes tested?
Ut for planning the new implementation.
Functionality tests are covered by existing tests.
Are there any user-facing changes?
No