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:13 UTC
(datafusion) branch gh-readonly-queue/main/pr-24411-93834e1eb72c9f8065c011f0c3d95e132fe73fe9 created (now 309dad82b8)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-24411-93834e1eb72c9f8065c011f0c3d95e132fe73fe9
in repository https://gitbox.apache.org/repos/asf/datafusion.git
at 309dad82b8 fix: preserve approximate percentile arguments with inline ORDER BY (#24411)
This branch includes the following new commits:
new 309dad82b8 fix: preserve approximate percentile arguments with inline ORDER BY (#24411)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org
(datafusion) 01/01: fix: preserve approximate percentile arguments with inline ORDER BY (#24411)
Posted by gi...@apache.org.
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