This is an automated email from the ASF dual-hosted git repository. github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-24411-93834e1eb72c9f8065c011f0c3d95e132fe73fe9 in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 309dad82b862200dbbc0e25b3791564309057e57 Author: Huaijin <[email protected]> AuthorDate: Thu Aug 20 19:18:58 2026 +0000 fix: preserve approximate percentile arguments with inline ORDER BY (#24411) ## Which issue does this PR close? - Closes #24410. ## Rationale for this change Inline ORDER BY handling incorrectly removes percentile or weight arguments from approximate percentile aggregates. ## What changes are included in this PR? Only remove the first aggregate argument when it matches the inline ordering expression. Add regression coverage for `approx_percentile_cont` and `approx_percentile_cont_with_weight`. ## Are these changes tested? Yes. The aggregate.slt regression tests, ./dev/rust_lint.sh, and cargo clippy --all-targets --all-features -- -D warnings all pass. ## Are there any user-facing changes? Valid approximate percentile queries using inline ORDER BY now plan and execute successfully. --- datafusion/sql/src/expr/function.rs | 33 ++++++++++-------------- datafusion/sqllogictest/test_files/aggregate.slt | 20 ++++++++++++++ 2 files changed, 33 insertions(+), 20 deletions(-) diff --git a/datafusion/sql/src/expr/function.rs b/datafusion/sql/src/expr/function.rs index 4ad022134a..f1ab0db470 100644 --- a/datafusion/sql/src/expr/function.rs +++ b/datafusion/sql/src/expr/function.rs @@ -728,32 +728,16 @@ impl<S: ContextProvider> SqlToRel<'_, S> { // accept a WITHIN GROUP clause. let supports_within_group = fm.supports_within_group_clause(); - // Built-in ordered-set aggregates must also support WITHIN GROUP - let is_builtin_ordered_set = matches!( - name.as_str(), - "percentile_cont" - | "quantile_cont" - | "approx_percentile_cont" - | "approx_percentile_cont_with_weight" - ); - - let supports_within_group = - supports_within_group || is_builtin_ordered_set; - let mut within_group = within_group; let mut order_by = order_by; - if supports_within_group + let is_inline_order_by = supports_within_group && within_group.is_empty() - && !order_by.is_empty() - { + && !order_by.is_empty(); + + if is_inline_order_by { // Inline ORDER BY syntax: // quantile_cont(value, percentile ORDER BY value) - if args.len() >= 2 { - args.remove(0); - arg_names.remove(0); - } - within_group = order_by; order_by = vec![]; } @@ -781,6 +765,15 @@ impl<S: ContextProvider> SqlToRel<'_, S> { ); } + // Remove the ordered value only when it is explicitly repeated. + // This ensures these are equivalent: + // approx_percentile_cont(c3, 0.95, 200 ORDER BY c3) + // approx_percentile_cont(0.95, 200 ORDER BY c3) + if is_inline_order_by && args.first() == Some(&sorts[0].expr) { + args.remove(0); + arg_names.remove(0); + } + // Prepend ordered value expression to args let value_expr = sorts[0].expr.clone(); arg_names = std::iter::once(None).chain(arg_names).collect(); diff --git a/datafusion/sqllogictest/test_files/aggregate.slt b/datafusion/sqllogictest/test_files/aggregate.slt index 70f080aed5..974487dac2 100644 --- a/datafusion/sqllogictest/test_files/aggregate.slt +++ b/datafusion/sqllogictest/test_files/aggregate.slt @@ -2733,6 +2733,26 @@ c 122 d 123.15 e 110.266666666667 +# inline ORDER BY preserves percentile and centroids arguments +query TR +SELECT c1, approx_percentile_cont(0.95, 200 ORDER BY c3) AS c3_p95 FROM aggregate_test_100 GROUP BY c1 ORDER BY c1 +---- +a 73.55 +b 68 +c 122.5 +d 124.2 +e 115.6 + +# inline ORDER BY preserves the weight argument +query TR +SELECT c1, approx_percentile_cont_with_weight(1, 0.95 ORDER BY c3) AS c3_p95 FROM aggregate_test_100 GROUP BY c1 ORDER BY c1 +---- +a 73.55 +b 68 +c 122.5 +d 124.2 +e 115.6 + # csv_query_sum_crossjoin query TTI SELECT a.c1, b.c1, SUM(a.c2) FROM aggregate_test_100 as a CROSS JOIN aggregate_test_100 as b GROUP BY a.c1, b.c1 ORDER BY a.c1, b.c1 --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
