You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "harrydevforlife (via GitHub)" <gi...@apache.org> on 2026/09/27 11:28:22 UTC
[I] SQL unparser drops ORDER BY inside ordered aggregates (last_value, first_value, array_agg, string_agg) [datafusion]
harrydevforlife opened a new issue, #25796:
URL: https://github.com/apache/datafusion/issues/25796
### Describe the bug
The SQL unparser drops the `ORDER BY` inside ordered aggregate functions (`last_value`, `first_value`, `array_agg`, `string_agg`, …). The unparsed SQL is still valid, so it runs without an error, but it computes a different result. Any tool that sends unparsed SQL to another engine returns wrong answers without any warning. `datafusion-federation` is one example.
In `datafusion/sql/src/unparser/expr.rs`, the `Expr::AggregateFunction` branch only uses `order_by` for `WITHIN GROUP` aggregates. For every other aggregate it builds `FunctionArgumentList { clauses: vec![], .. }`, so the ordering never reaches the AST. This is still the case on `main` (checked at 5a09d99) and in 55.1.0.
This is the same kind of loss as #25462 (`IGNORE NULLS` / `RESPECT NULLS`, fix in #25475), in the same code.
### To Reproduce
```rust
use datafusion::{error::Result, prelude::*, sql::unparser::plan_to_sql};
#[tokio::main]
async fn main() -> Result<()> {
let ctx = SessionContext::new();
ctx.sql("CREATE TABLE t (a INT, b INT, g INT) AS VALUES (1, 2, 1), (3, 1, 1)").await?;
for sql in [
"SELECT g, last_value(a ORDER BY b) FROM t GROUP BY g",
"SELECT g, first_value(a ORDER BY b DESC) FROM t GROUP BY g",
"SELECT g, array_agg(a ORDER BY b) FROM t GROUP BY g",
"SELECT g, string_agg(CAST(a AS VARCHAR), ',' ORDER BY b) FROM t GROUP BY g",
] {
let plan = ctx.sql(sql).await?.into_unoptimized_plan();
println!("{}", plan_to_sql(&plan)?);
}
Ok(())
}
```
Output (DataFusion 55.1.0):
```
SELECT t.g, last_value(t.a) FROM t GROUP BY t.g
SELECT t.g, first_value(t.a) FROM t GROUP BY t.g
SELECT t.g, array_agg(t.a) FROM t GROUP BY t.g
SELECT t.g, string_agg(CAST(t.a AS VARCHAR), ',') FROM t GROUP BY t.g
```
Running the unparsed SQL gives a different result from the original for all four queries.
### Expected behavior
The ordering is kept, for example `SELECT t.g, last_value(t.a ORDER BY t.b ASC NULLS LAST) FROM t GROUP BY t.g`, so that parsing, planning and unparsing a query gives back an equivalent query.
### Additional context
A possible fix: when `order_by` is non-empty and the function doesn't support `WITHIN GROUP`, emit it as an argument clause:
```rust
clauses: if order_by.is_empty() || agg.func.supports_within_group_clause() {
vec![]
} else {
vec![ast::FunctionArgumentClause::OrderBy(
order_by.iter().map(|s| self.sort_to_sql(s)).collect::<Result<Vec<_>>>()?,
)]
},
```
It could be tested with a round-trip case in `datafusion/sql/tests/cases/plan_to_sql.rs` for each of the four functions above. Found while federating `last_value(v ORDER BY t)` to remote engines with `datafusion-federation` 0.5.7.
--
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.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: [I] SQL unparser drops ORDER BY inside ordered aggregates (last_value, first_value, array_agg, string_agg) [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway closed issue #25796: SQL unparser drops ORDER BY inside ordered aggregates (last_value, first_value, array_agg, string_agg)
URL: https://github.com/apache/datafusion/issues/25796
--
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: [I] SQL unparser drops ORDER BY inside ordered aggregates (last_value, first_value, array_agg, string_agg) [datafusion]
Posted by "Vanzeren (via GitHub)" <gi...@apache.org>.
Vanzeren commented on issue #25796:
URL: https://github.com/apache/datafusion/issues/25796#issuecomment-5855852569
take
--
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