You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "goutamadwant (via GitHub)" <gi...@apache.org> on 2026/09/07 06:53:46 UTC
[PR] perf: reuse column normalization context across expressions [datafusion]
goutamadwant opened a new pull request, #25010:
URL: https://github.com/apache/datafusion/pull/25010
## Which issue does this PR close?
- Addresses fix (1) in #24777. Constructor-based logical-plan decoding remains separate.
## Rationale for this change
Column normalization repeatedly collects fallback schemas and traverses the input plan for USING columns. Wide expression lists and projections repeat that work for the same immutable plan.
## What changes are included in this PR?
- Introduce a private, lazy normalization context reused across columns and expression lists.
- Share the context in sort normalization and validated projection construction, including wildcard expansion.
- Keep already-qualified columns and expressions that do not need normalization on the existing fast path.
- Add benchmarks for qualified and unqualified expressions and projection construction at several schema widths.
## What is the testing strategy for this PR?
- Add `normalize_batch_schema_precedence`, `normalize_batch_using_join`, and `normalize_batch_skips_unused_plan_context` to cover schema precedence, USING joins, ambiguity/error order, sort options, and lazy handling of qualified columns and literals.
- In balanced local `release-nonlto` runs, constructing a 2,000-column unqualified projection falls from about 100 ms to 35 ms. This measures projection construction, not full protobuf decoding; small controls remain noisy.
- Reproduce with `cargo bench -p datafusion-expr --bench normalize_columns --profile release-nonlto`.
- Focused expression and SQL tests pass. The required extended workspace test command also passes, including all 511 SQL logic-test files.
- Expression-crate Clippy passes with all targets and features enabled. The complete documented `dev/rust_lint.sh` also passes, including strict workspace documentation checks.
- Full-workspace Clippy with all features enabled hits the existing PostgreSQL decimal-formatting lint in #24974; the affected source is unchanged here.
## Are there any user-facing changes?
No public API or name-resolution behavior changes are intended. Normalization reuses plan context instead of collecting it for each column; existing schema lookup costs remain.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: reuse column normalization context across expressions [datafusion]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #25010:
URL: https://github.com/apache/datafusion/pull/25010#issuecomment-5566751607
## [Codecov](https://app.codecov.io/gh/apache/datafusion/pull/25010?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `75.45455%` with `27 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 81.70%. Comparing base ([`e1ca94f`](https://app.codecov.io/gh/apache/datafusion/commit/e1ca94fb11ab0f2ead9bddd2b5be6666e5d75c62?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`2fcda6c`](https://app.codecov.io/gh/apache/datafusion/commit/2fcda6c99114bb67102f32e25c7386142c20cfde?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 2 commits behind head on main.
| [Files with missing lines](https://app.codecov.io/gh/apache/datafusion/pull/25010?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [datafusion/expr/src/expr\_rewriter/mod.rs](https://app.codecov.io/gh/apache/datafusion/pull/25010?src=pr&el=tree&filepath=datafusion%2Fexpr%2Fsrc%2Fexpr_rewriter%2Fmod.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9leHByL3NyYy9leHByX3Jld3JpdGVyL21vZC5ycw==) | 77.14% | [1 Missing and 23 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/25010?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [datafusion/expr/src/logical\_plan/builder.rs](https://app.codecov.io/gh/apache/datafusion/pull/25010?src=pr&el=tree&filepath=datafusion%2Fexpr%2Fsrc%2Flogical_plan%2Fbuilder.rs&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-ZGF0YWZ1c2lvbi9leHByL3NyYy9sb2dpY2FsX3BsYW4vYnVpbGRlci5ycw==) | 40.00% | [0 Missing and 3 partials :warning: ](https://app.codecov.io/gh/apache/datafusion/pull/25010?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #25010 +/- ##
========================================
Coverage 81.69% 81.70%
========================================
Files 1127 1127
Lines 415471 415743 +272
Branches 415471 415743 +272
========================================
+ Hits 339424 339672 +248
+ Misses 56108 56107 -1
- Partials 19939 19964 +25
```
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/datafusion/pull/25010?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: reuse column normalization context across expressions [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #25010:
URL: https://github.com/apache/datafusion/pull/25010
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: reuse column normalization context across expressions [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #25010:
URL: https://github.com/apache/datafusion/pull/25010#issuecomment-5749251978
Thank you @goutamadwant and @kosiew
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: reuse column normalization context across expressions [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on code in PR #25010:
URL: https://github.com/apache/datafusion/pull/25010#discussion_r4024885867
##########
datafusion/expr/benches/normalize_columns.rs:
##########
@@ -0,0 +1,97 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::hint::black_box;
+
+use arrow::datatypes::{DataType, Field, Schema};
+use criterion::{BenchmarkId, Criterion, criterion_group, criterion_main};
+use datafusion_expr::expr_rewriter::normalize_cols;
+use datafusion_expr::logical_plan::table_scan;
+use datafusion_expr::{Expr, LogicalPlanBuilder, col, lit};
+
+fn normalize_columns(c: &mut Criterion) {
+ let mut group = c.benchmark_group("normalize_columns");
+ for width in [10, 100, 500, 2000] {
+ let schema = Schema::new(
+ (0..width)
+ .map(|i| Field::new(format!("c{i}"), DataType::Int32, false))
+ .collect::<Vec<_>>(),
+ );
+ let input = table_scan(Some("t"), &schema, None)
Review Comment:
Nice to see benchmark coverage added here. The unqualified case already exercises the cached `using_columns()` traversal, but this plan does not include any `JOIN ... USING` nodes. It could be useful to add a wide `JOIN ... USING` input with unqualified projected expressions as well. That would measure the populated USING-column path directly and help protect that performance improvement going forward. Non-blocking.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: reuse column normalization context across expressions [datafusion]
Posted by "kosiew (via GitHub)" <gi...@apache.org>.
kosiew commented on PR #25010:
URL: https://github.com/apache/datafusion/pull/25010#issuecomment-5695824220
[benchmark run](https://github.com/apache/datafusion/pull/25370#issuecomment-5695793763)
```
group HEAD bench-25010
----- ---- -----------
normalize_columns/expressions/qualified/10 1.02 4.2±0.03µs ? ?/sec 1.00 4.1±0.04µs ? ?/sec
normalize_columns/expressions/qualified/100 1.00 42.4±0.32µs ? ?/sec 1.00 42.1±0.52µs ? ?/sec
normalize_columns/expressions/qualified/2000 1.01 864.4±7.16µs ? ?/sec 1.00 854.6±8.82µs ? ?/sec
normalize_columns/expressions/qualified/500 1.01 212.1±1.85µs ? ?/sec 1.00 209.9±2.25µs ? ?/sec
normalize_columns/expressions/unqualified/10 1.54 6.5±0.05µs ? ?/sec 1.00 4.2±0.04µs ? ?/sec
normalize_columns/expressions/unqualified/100 3.37 231.2±0.51µs ? ?/sec 1.00 68.6±0.39µs ? ?/sec
normalize_columns/expressions/unqualified/2000 11.90 74.7±0.18ms ? ?/sec 1.00 6.3±0.02ms ? ?/sec
normalize_columns/expressions/unqualified/500 7.45 4.7±0.01ms ? ?/sec 1.00 631.2±2.81µs ? ?/sec
normalize_columns/projection/qualified/10 1.01 36.2±0.61µs ? ?/sec 1.00 35.7±0.29µs ? ?/sec
normalize_columns/projection/qualified/100 1.00 403.8±1.39µs ? ?/sec 1.02 410.5±3.15µs ? ?/sec
normalize_columns/projection/qualified/2000 1.00 26.9±0.17ms ? ?/sec 1.06 28.5±0.29ms ? ?/sec
normalize_columns/projection/qualified/500 1.00 3.2±0.01ms ? ?/sec 1.03 3.3±0.01ms ? ?/sec
normalize_columns/projection/unqualified/10 1.06 40.1±0.36µs ? ?/sec 1.00 37.6±0.20µs ? ?/sec
normalize_columns/projection/unqualified/100 1.39 608.7±1.86µs ? ?/sec 1.00 437.7±1.92µs ? ?/sec
normalize_columns/projection/unqualified/2000 3.74 94.2±0.46ms ? ?/sec 1.00 25.2±0.07ms ? ?/sec
normalize_columns/projection/unqualified/500 2.31 7.3±0.01ms ? ?/sec 1.00 3.2±0.01ms ? ?/sec
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: reuse column normalization context across expressions [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #25010:
URL: https://github.com/apache/datafusion/pull/25010#discussion_r4056733945
##########
datafusion/expr/benches/normalize_columns.rs:
##########
@@ -0,0 +1,97 @@
+// Licensed to the Apache Software Foundation (ASF) under one
+// or more contributor license agreements. See the NOTICE file
+// distributed with this work for additional information
+// regarding copyright ownership. The ASF licenses this file
+// to you under the Apache License, Version 2.0 (the
+// "License"); you may not use this file except in compliance
+// with the License. You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing,
+// software distributed under the License is distributed on an
+// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+// KIND, either express or implied. See the License for the
+// specific language governing permissions and limitations
+// under the License.
+
+use std::hint::black_box;
+
+use arrow::datatypes::{DataType, Field, Schema};
+use criterion::{BenchmarkId, Criterion, criterion_group, criterion_main};
+use datafusion_expr::expr_rewriter::normalize_cols;
+use datafusion_expr::logical_plan::table_scan;
+use datafusion_expr::{Expr, LogicalPlanBuilder, col, lit};
+
+fn normalize_columns(c: &mut Criterion) {
+ let mut group = c.benchmark_group("normalize_columns");
+ for width in [10, 100, 500, 2000] {
+ let schema = Schema::new(
+ (0..width)
+ .map(|i| Field::new(format!("c{i}"), DataType::Int32, false))
+ .collect::<Vec<_>>(),
+ );
+ let input = table_scan(Some("t"), &schema, None)
Review Comment:
@goutamadwant perhaps you can add this new benchmark as a follow on PR
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] perf: reuse column normalization context across expressions [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #25010:
URL: https://github.com/apache/datafusion/pull/25010#issuecomment-5749251347
Merging this one in to try and keep the code flowing (our backlog of now approved PRs is getting quite large)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org