You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/08/20 19:19:14 UTC
(datafusion) 01/01: fix: preserve approximate percentile arguments with inline ORDER BY (#24411)
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 <ha...@gmail.com>
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: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org