You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "AdamGS (via GitHub)" <gi...@apache.org> on 2026/02/09 11:29:29 UTC
[PR] Remove recursive const check in `simplify_const_expr` [datafusion]
AdamGS opened a new pull request, #20234:
URL: https://github.com/apache/datafusion/pull/20234
## Which issue does this PR close?
- Closes #20134 .
## Rationale for this change
The check for simplifying const expressions was recursive and expensive, repeatedly checking the expression's children in a recursive way. I've tried other approached like pre-computing the result for all expressions outside of the loop and using that cache during the traversal, but I've found that it only yielded between 5-8% improvement while adding complexity, while this approach simplifies the code and has shown to be more performant in my benchmarks (change is compared to current main branch):
```
tpc-ds/q76/cs/16 time: [27.112 µs 27.159 µs 27.214 µs]
change: [−13.533% −13.167% −12.801%] (p = 0.00 < 0.05)
Performance has improved.
Found 7 outliers among 100 measurements (7.00%)
1 (1.00%) low mild
4 (4.00%) high mild
2 (2.00%) high severe
tpc-ds/q76/ws/16 time: [26.175 µs 26.280 µs 26.394 µs]
change: [−14.312% −13.833% −13.346%] (p = 0.00 < 0.05)
Performance has improved.
Found 1 outliers among 100 measurements (1.00%)
1 (1.00%) low mild
tpc-ds/q76/cs/128 time: [195.79 µs 196.17 µs 196.56 µs]
change: [−14.362% −14.080% −13.816%] (p = 0.00 < 0.05)
Performance has improved.
Found 5 outliers among 100 measurements (5.00%)
1 (1.00%) low severe
1 (1.00%) low mild
3 (3.00%) high mild
tpc-ds/q76/ws/128 time: [197.08 µs 197.61 µs 198.23 µs]
change: [−13.531% −13.142% −12.737%] (p = 0.00 < 0.05)
Performance has improved.
Found 3 outliers among 100 measurements (3.00%)
1 (1.00%) low mild
2 (2.00%) high mild
```
## What changes are included in this PR?
1. `simplify_const_expr` now only checks itself and whether all of its children are literals, because it assumes the order of simplification is bottoms-up.
2. Removes some code from the public API, see the last section for the full details.
## Are these changes tested?
Existing test suite
## Are there any user-facing changes?
I suggest removing some of the physical expression simplification code from the public API, which I believe reduces the maintenance burden here. These changes also helps removing code like the distinct `simplify_const_expr` and `simplify_const_expr_with_dummy`.
1. Makes all `datafusion-physical-expr::simplifier` sub-modules (`not` and `const_evaluator`) private, including their key functions. They are not used externally, and being able to change their behavior seems more valuable long term. The simplifier is also not currently an extension point as far as I can tell, so there's no value in providing atomic building blocks like them for now.
2. Removes `has_column_references` completely, its trivial to re-implement and isn't used anywhere in the codebase.
--
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] Remove recursive const check in `simplify_const_expr` [datafusion]
Posted by "alamb-ghbot (via GitHub)" <gi...@apache.org>.
alamb-ghbot commented on PR #20234:
URL: https://github.com/apache/datafusion/pull/20234#issuecomment-3954397361
🤖 `./gh_compare_branch_bench.sh` [compare_branch_bench.sh](https://github.com/alamb/datafusion-benchmarking/blob/main/scripts/compare_branch_bench.sh) Running
Linux aal-dev 6.14.0-1018-gcp #19~24.04.1-Ubuntu SMP Wed Sep 24 23:23:09 UTC 2025 x86_64 x86_64 x86_64 GNU/Linux
Comparing adamg/issue-20134 (e53b2ca5d04e1c40bce0bbf12567e8aa6c86ccee) to 17d770d6e53400db0c3d46bde330bf0cdffcce6e [diff](https://github.com/apache/datafusion/compare/17d770d6e53400db0c3d46bde330bf0cdffcce6e..e53b2ca5d04e1c40bce0bbf12567e8aa6c86ccee)
BENCH_NAME=sql_planner
BENCH_COMMAND=cargo bench --features=parquet --bench sql_planner
BENCH_FILTER=
BENCH_BRANCH_NAME=adamg_issue-20134
Results will be posted here when complete
--
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] Remove recursive const check in `simplify_const_expr` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #20234:
URL: https://github.com/apache/datafusion/pull/20234#issuecomment-3952125574
I'll plan to merge this PR in when it passes CI
--
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] Remove recursive const check in `simplify_const_expr` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #20234:
URL: https://github.com/apache/datafusion/pull/20234#issuecomment-3954396477
run benchmark sql_planner
--
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] Remove recursive const check in `simplify_const_expr` [datafusion]
Posted by "AdamGS (via GitHub)" <gi...@apache.org>.
AdamGS commented on PR #20234:
URL: https://github.com/apache/datafusion/pull/20234#issuecomment-3871173301
(if any maintainer can add the `api change` label here, that would be appreciated)
--
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] Remove recursive const check in `simplify_const_expr` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #20234:
URL: https://github.com/apache/datafusion/pull/20234#discussion_r2847349857
##########
datafusion/physical-expr/src/simplifier/mod.rs:
##########
@@ -67,10 +65,11 @@ impl<'a> PhysicalExprSimplifier<'a> {
// Apply NOT expression simplification first, then unwrap cast optimization,
// then constant expression evaluation
- let rewritten = simplify_not_expr(node, schema)?
+ #[expect(deprecated, reason = "`simplify_not_expr` is marked as deprecated until it's made private.")]
+ let rewritten = not::simplify_not_expr(node, schema)?
.transform_data(|node| unwrap_cast_in_comparison(node, schema))?
.transform_data(|node| {
- simplify_const_expr_with_dummy(node, &batch)
+ const_evaluator::simplify_const_expr_immediate(node, &batch)
Review Comment:
this is a clever change 👍
--
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] Remove recursive const check in `simplify_const_expr` [datafusion]
Posted by "AdamGS (via GitHub)" <gi...@apache.org>.
AdamGS commented on PR #20234:
URL: https://github.com/apache/datafusion/pull/20234#issuecomment-3873852723
Of course, will push that first thing tomorrow morning
--
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] Remove recursive const check in `simplify_const_expr` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #20234:
URL: https://github.com/apache/datafusion/pull/20234
--
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] Remove recursive const check in `simplify_const_expr` [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on PR #20234:
URL: https://github.com/apache/datafusion/pull/20234#issuecomment-3873694860
> I suggest removing some of the physical expression simplification code from the public API, which I believe reduces the maintenance burden here.
I do agree with you but please also consider deprecating those functions for a release or two and then remove the `pub` modifier and the `#[deprecated]` attribute. This will give any existing users of those APIs time to migrate or explain why they need these APIs.
--
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] Remove recursive const check in `simplify_const_expr` [datafusion]
Posted by "alamb-ghbot (via GitHub)" <gi...@apache.org>.
alamb-ghbot commented on PR #20234:
URL: https://github.com/apache/datafusion/pull/20234#issuecomment-3954744785
🤖: Benchmark completed
<details><summary>Details</summary>
<p>
```
group adamg_issue-20134 main
----- ----------------- ----
logical_aggregate_with_join 1.01 639.0±7.22µs ? ?/sec 1.00 635.6±8.99µs ? ?/sec
logical_plan_struct_join_agg_sort 1.03 302.0±7.82µs ? ?/sec 1.00 291.9±2.13µs ? ?/sec
logical_select_all_from_1000 1.00 10.2±0.05ms ? ?/sec 1.01 10.2±0.10ms ? ?/sec
logical_select_one_from_700 1.01 419.8±13.06µs ? ?/sec 1.00 417.0±2.28µs ? ?/sec
logical_trivial_join_high_numbered_columns 1.00 376.5±3.20µs ? ?/sec 1.00 375.3±7.79µs ? ?/sec
logical_trivial_join_low_numbered_columns 1.00 361.8±6.52µs ? ?/sec 1.00 361.7±6.96µs ? ?/sec
physical_intersection 1.00 1605.9±15.55µs ? ?/sec 1.02 1639.0±26.40µs ? ?/sec
physical_join_consider_sort 1.00 2.3±0.06ms ? ?/sec 1.04 2.4±0.06ms ? ?/sec
physical_join_distinct 1.00 351.7±2.82µs ? ?/sec 1.01 353.7±3.89µs ? ?/sec
physical_many_self_joins 1.00 12.5±0.18ms ? ?/sec 1.04 13.0±0.17ms ? ?/sec
physical_plan_clickbench_all 1.00 201.6±3.16ms ? ?/sec 1.03 207.6±5.28ms ? ?/sec
physical_plan_clickbench_q1 1.03 2.2±0.05ms ? ?/sec 1.00 2.1±0.02ms ? ?/sec
physical_plan_clickbench_q10 1.03 3.7±0.04ms ? ?/sec 1.00 3.6±0.05ms ? ?/sec
physical_plan_clickbench_q11 1.07 4.4±0.22ms ? ?/sec 1.00 4.1±0.07ms ? ?/sec
physical_plan_clickbench_q12 1.01 4.3±0.04ms ? ?/sec 1.00 4.3±0.17ms ? ?/sec
physical_plan_clickbench_q13 1.02 3.8±0.05ms ? ?/sec 1.00 3.8±0.09ms ? ?/sec
physical_plan_clickbench_q14 1.04 4.3±0.08ms ? ?/sec 1.00 4.1±0.08ms ? ?/sec
physical_plan_clickbench_q15 1.00 3.9±0.07ms ? ?/sec 1.00 3.9±0.15ms ? ?/sec
physical_plan_clickbench_q16 1.07 3.9±0.08ms ? ?/sec 1.00 3.7±0.03ms ? ?/sec
physical_plan_clickbench_q17 1.06 4.0±0.10ms ? ?/sec 1.00 3.8±0.08ms ? ?/sec
physical_plan_clickbench_q18 1.03 2.7±0.05ms ? ?/sec 1.00 2.7±0.05ms ? ?/sec
physical_plan_clickbench_q19 1.08 4.5±0.11ms ? ?/sec 1.00 4.2±0.05ms ? ?/sec
physical_plan_clickbench_q2 1.02 2.9±0.06ms ? ?/sec 1.00 2.8±0.04ms ? ?/sec
physical_plan_clickbench_q20 1.03 2.2±0.05ms ? ?/sec 1.00 2.2±0.02ms ? ?/sec
physical_plan_clickbench_q21 1.02 2.8±0.06ms ? ?/sec 1.00 2.8±0.03ms ? ?/sec
physical_plan_clickbench_q22 1.01 4.0±0.07ms ? ?/sec 1.00 4.0±0.11ms ? ?/sec
physical_plan_clickbench_q23 1.02 4.3±0.10ms ? ?/sec 1.00 4.2±0.04ms ? ?/sec
physical_plan_clickbench_q24 1.01 4.9±0.03ms ? ?/sec 1.00 4.8±0.08ms ? ?/sec
physical_plan_clickbench_q25 1.02 3.5±0.09ms ? ?/sec 1.00 3.5±0.04ms ? ?/sec
physical_plan_clickbench_q26 1.00 2.9±0.04ms ? ?/sec 1.01 2.9±0.08ms ? ?/sec
physical_plan_clickbench_q27 1.02 3.5±0.04ms ? ?/sec 1.00 3.5±0.04ms ? ?/sec
physical_plan_clickbench_q28 1.02 4.5±0.16ms ? ?/sec 1.00 4.5±0.07ms ? ?/sec
physical_plan_clickbench_q29 1.01 4.8±0.05ms ? ?/sec 1.00 4.7±0.10ms ? ?/sec
physical_plan_clickbench_q3 1.02 2.6±0.06ms ? ?/sec 1.00 2.5±0.02ms ? ?/sec
physical_plan_clickbench_q30 1.03 16.3±0.30ms ? ?/sec 1.00 15.9±0.18ms ? ?/sec
physical_plan_clickbench_q31 1.01 4.5±0.07ms ? ?/sec 1.00 4.4±0.05ms ? ?/sec
physical_plan_clickbench_q32 1.02 4.6±0.13ms ? ?/sec 1.00 4.5±0.15ms ? ?/sec
physical_plan_clickbench_q33 1.01 3.7±0.06ms ? ?/sec 1.00 3.6±0.08ms ? ?/sec
physical_plan_clickbench_q34 1.02 3.3±0.07ms ? ?/sec 1.00 3.2±0.04ms ? ?/sec
physical_plan_clickbench_q35 1.03 3.5±0.09ms ? ?/sec 1.00 3.4±0.08ms ? ?/sec
physical_plan_clickbench_q36 1.04 4.4±0.11ms ? ?/sec 1.00 4.2±0.03ms ? ?/sec
physical_plan_clickbench_q37 1.01 4.8±0.08ms ? ?/sec 1.00 4.7±0.07ms ? ?/sec
physical_plan_clickbench_q38 1.01 4.7±0.11ms ? ?/sec 1.00 4.7±0.03ms ? ?/sec
physical_plan_clickbench_q39 1.00 4.1±0.05ms ? ?/sec 1.00 4.1±0.10ms ? ?/sec
physical_plan_clickbench_q4 1.02 2.2±0.07ms ? ?/sec 1.00 2.2±0.02ms ? ?/sec
physical_plan_clickbench_q40 1.00 5.0±0.12ms ? ?/sec 1.00 4.9±0.09ms ? ?/sec
physical_plan_clickbench_q41 1.01 4.3±0.12ms ? ?/sec 1.00 4.3±0.06ms ? ?/sec
physical_plan_clickbench_q42 1.00 4.2±0.11ms ? ?/sec 1.00 4.2±0.04ms ? ?/sec
physical_plan_clickbench_q43 1.00 4.6±0.09ms ? ?/sec 1.00 4.6±0.09ms ? ?/sec
physical_plan_clickbench_q44 1.00 2.3±0.03ms ? ?/sec 1.00 2.3±0.03ms ? ?/sec
physical_plan_clickbench_q45 1.00 2.3±0.03ms ? ?/sec 1.00 2.3±0.02ms ? ?/sec
physical_plan_clickbench_q46 1.00 3.2±0.06ms ? ?/sec 1.00 3.2±0.02ms ? ?/sec
physical_plan_clickbench_q47 1.00 4.7±0.10ms ? ?/sec 1.04 4.9±0.42ms ? ?/sec
physical_plan_clickbench_q48 1.00 5.1±0.09ms ? ?/sec 1.00 5.1±0.08ms ? ?/sec
physical_plan_clickbench_q49 1.00 5.5±0.16ms ? ?/sec 1.01 5.5±0.09ms ? ?/sec
physical_plan_clickbench_q5 1.02 2.6±0.03ms ? ?/sec 1.00 2.5±0.02ms ? ?/sec
physical_plan_clickbench_q50 1.00 4.2±0.04ms ? ?/sec 1.04 4.3±0.11ms ? ?/sec
physical_plan_clickbench_q51 1.00 3.6±0.08ms ? ?/sec 1.02 3.6±0.07ms ? ?/sec
physical_plan_clickbench_q6 1.02 2.6±0.06ms ? ?/sec 1.00 2.5±0.01ms ? ?/sec
physical_plan_clickbench_q7 1.02 2.1±0.04ms ? ?/sec 1.00 2.1±0.01ms ? ?/sec
physical_plan_clickbench_q8 1.02 3.5±0.11ms ? ?/sec 1.00 3.4±0.03ms ? ?/sec
physical_plan_clickbench_q9 1.01 3.7±0.09ms ? ?/sec 1.00 3.6±0.10ms ? ?/sec
physical_plan_struct_join_agg_sort 1.00 3.5±0.17ms ? ?/sec 1.05 3.6±0.12ms ? ?/sec
physical_plan_tpcds_all 1.01 1930.2±27.02ms ? ?/sec 1.00 1909.2±22.09ms ? ?/sec
physical_plan_tpch_all 1.01 127.9±2.31ms ? ?/sec 1.00 126.6±3.40ms ? ?/sec
physical_plan_tpch_q1 1.00 3.0±0.04ms ? ?/sec 1.00 3.0±0.09ms ? ?/sec
physical_plan_tpch_q10 1.04 7.4±0.35ms ? ?/sec 1.00 7.2±0.11ms ? ?/sec
physical_plan_tpch_q11 1.01 8.6±0.20ms ? ?/sec 1.00 8.5±0.14ms ? ?/sec
physical_plan_tpch_q12 1.03 3.2±0.04ms ? ?/sec 1.00 3.1±0.03ms ? ?/sec
physical_plan_tpch_q13 1.03 3.1±0.05ms ? ?/sec 1.00 3.0±0.04ms ? ?/sec
physical_plan_tpch_q14 1.04 3.2±0.12ms ? ?/sec 1.00 3.0±0.03ms ? ?/sec
physical_plan_tpch_q16 1.03 5.3±0.25ms ? ?/sec 1.00 5.2±0.06ms ? ?/sec
physical_plan_tpch_q17 1.06 5.9±0.09ms ? ?/sec 1.00 5.6±0.05ms ? ?/sec
physical_plan_tpch_q18 1.06 6.3±0.09ms ? ?/sec 1.00 5.9±0.07ms ? ?/sec
physical_plan_tpch_q19 1.01 5.2±0.08ms ? ?/sec 1.00 5.1±0.06ms ? ?/sec
physical_plan_tpch_q2 1.02 12.6±0.52ms ? ?/sec 1.00 12.3±0.38ms ? ?/sec
physical_plan_tpch_q20 1.00 8.1±0.07ms ? ?/sec 1.00 8.0±0.14ms ? ?/sec
physical_plan_tpch_q21 1.01 10.2±0.19ms ? ?/sec 1.00 10.0±0.14ms ? ?/sec
physical_plan_tpch_q22 1.00 6.5±0.13ms ? ?/sec 1.00 6.5±0.19ms ? ?/sec
physical_plan_tpch_q3 1.01 5.7±0.23ms ? ?/sec 1.00 5.6±0.13ms ? ?/sec
physical_plan_tpch_q4 1.01 3.1±0.06ms ? ?/sec 1.00 3.0±0.10ms ? ?/sec
physical_plan_tpch_q5 1.02 6.1±0.11ms ? ?/sec 1.00 5.9±0.11ms ? ?/sec
physical_plan_tpch_q6 1.02 1640.8±76.80µs ? ?/sec 1.00 1613.7±22.11µs ? ?/sec
physical_plan_tpch_q7 1.01 7.2±0.08ms ? ?/sec 1.00 7.1±0.10ms ? ?/sec
physical_plan_tpch_q8 1.03 9.5±0.33ms ? ?/sec 1.00 9.3±0.25ms ? ?/sec
physical_plan_tpch_q9 1.01 6.6±0.05ms ? ?/sec 1.00 6.6±0.05ms ? ?/sec
physical_select_aggregates_from_200 1.00 17.3±0.33ms ? ?/sec 1.03 17.8±0.32ms ? ?/sec
physical_select_all_from_1000 1.00 22.8±0.67ms ? ?/sec 1.02 23.4±0.24ms ? ?/sec
physical_select_one_from_700 1.00 1328.1±9.08µs ? ?/sec 1.00 1326.7±20.03µs ? ?/sec
physical_sorted_union_order_by_10_int64 1.00 11.1±0.37ms ? ?/sec 1.05 11.6±0.16ms ? ?/sec
physical_sorted_union_order_by_10_uint64 1.00 29.4±0.21ms ? ?/sec 1.04 30.6±0.68ms ? ?/sec
physical_sorted_union_order_by_50_int64 1.00 194.0±3.18ms ? ?/sec 1.08 208.7±2.00ms ? ?/sec
physical_sorted_union_order_by_50_uint64 1.00 1082.1±32.93ms ? ?/sec 1.02 1102.3±37.52ms ? ?/sec
physical_theta_join_consider_sort 1.00 2.7±0.05ms ? ?/sec 1.04 2.8±0.09ms ? ?/sec
physical_unnest_to_join 1.00 3.1±0.04ms ? ?/sec 1.05 3.2±0.09ms ? ?/sec
physical_window_function_partition_by_12_on_values 1.00 1598.2±43.25µs ? ?/sec 1.01 1610.7±30.68µs ? ?/sec
physical_window_function_partition_by_30_on_values 1.00 2.9±0.02ms ? ?/sec 1.08 3.1±0.10ms ? ?/sec
physical_window_function_partition_by_4_on_values 1.00 1094.7±30.38µs ? ?/sec 1.02 1118.5±40.94µs ? ?/sec
physical_window_function_partition_by_7_on_values 1.00 1265.2±25.21µs ? ?/sec 1.03 1300.2±25.78µs ? ?/sec
physical_window_function_partition_by_8_on_values 1.00 1332.0±26.73µs ? ?/sec 1.01 1342.6±14.79µs ? ?/sec
with_param_values_many_columns 1.00 573.2±7.96µs ? ?/sec 1.01 579.4±5.71µs ? ?/sec
```
</p>
</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] Remove recursive const check in `simplify_const_expr` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #20234:
URL: https://github.com/apache/datafusion/pull/20234#issuecomment-3954369786
Thanks again @AdamGS and @martin-g
--
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] Remove recursive const check in `simplify_const_expr` [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on code in PR #20234:
URL: https://github.com/apache/datafusion/pull/20234#discussion_r2849311070
##########
datafusion/physical-expr/src/simplifier/const_evaluator.rs:
##########
@@ -77,6 +139,20 @@ pub(crate) fn simplify_const_expr_with_dummy(
}
}
+/// Create a 1-row dummy RecordBatch for evaluating constant expressions.
+///
+/// The batch is never actually accessed for data - it's just needed because
+/// the PhysicalExpr::evaluate API requires a RecordBatch. For expressions
+/// that only contain literals, the batch content is irrelevant.
+///
+/// This is the same approach used in the logical expression `ConstEvaluator`.
+pub(crate) fn create_dummy_batch() -> Result<RecordBatch> {
Review Comment:
While reviewing this I was wondering why we need a dummy batch at all -- I created a follow on PR to create it once and reuse it (should make expression simplification faster)
- https://github.com/apache/datafusion/pull/20534
--
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