From aa9db21ca85e99c7ebe40e3b1e06f2d49f97a743 Mon Sep 17 00:00:00 2001 From: Huaijin Date: Sun, 16 Aug 2026 17:28:12 +0800 Subject: [PATCH 1/2] fix: preserve approximate percentile arguments with inline ORDER BY --- datafusion/sql/src/expr/function.rs | 18 +++++++++-------- .../sqllogictest/test_files/aggregate.slt | 20 +++++++++++++++++++ 2 files changed, 30 insertions(+), 8 deletions(-) diff --git a/datafusion/sql/src/expr/function.rs b/datafusion/sql/src/expr/function.rs index e6bee31fbf106..abbb3b277dca4 100644 --- a/datafusion/sql/src/expr/function.rs +++ b/datafusion/sql/src/expr/function.rs @@ -750,17 +750,13 @@ impl SqlToRel<'_, S> { 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![]; } @@ -788,6 +784,12 @@ impl SqlToRel<'_, S> { ); } + // Remove the ordered value only when it is explicitly repeated. + 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 460d4cd2ffda3..ed5d91f0b9cd6 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 From 32f7528924bfedfbc2997cd6e0434013fb40907d Mon Sep 17 00:00:00 2001 From: Huaijin Date: Sun, 16 Aug 2026 22:15:54 +0800 Subject: [PATCH 2/2] update --- datafusion/sql/src/expr/function.rs | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/datafusion/sql/src/expr/function.rs b/datafusion/sql/src/expr/function.rs index abbb3b277dca4..b211f7904cbaf 100644 --- a/datafusion/sql/src/expr/function.rs +++ b/datafusion/sql/src/expr/function.rs @@ -735,18 +735,6 @@ impl 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; @@ -785,6 +773,9 @@ impl 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);