Vanzeren commented on PR #25803: URL: https://github.com/apache/datafusion/pull/25803#issuecomment-5857186268
Hi @sunchao @alamb — this is my first PR to DataFusion, so the CI workflows need a committer to kick them off. Could one of you trigger them when convenient? Context: fixes #25796. The unparser only consumed `params.order_by` for `WITHIN GROUP` aggregates, so any aggregate that spells its ordering inside the argument list (`array_agg(x ORDER BY y)`, `string_agg`, `first_value`, `last_value`) had that ordering dropped. The emitted SQL stays valid, so it runs without error and silently returns different results. 2 files, +130/-13. Already run locally: - `dev/rust_lint.sh` — all 17 steps pass - `cargo test --no-fail-fast --profile ci --workspace --lib --tests --bins --features avro,json,backtrace,extended_tests,recursive_protection,parquet_encryption` — 12203 passed, 8 ignored - `cargo test --profile=ci --test sqllogictests` — 524/524 One caveat: `memory_limit::memory_limit_validation::sort_mem_validation::sort_with_mem_limit_1` fails on this machine, with a peak RSS of ~199 MB against a ~190.7 MB ceiling. It fails identically on unmodified `main` (5a09d99), its query (`select * from generate_series(1,10000000) order by c1`) contains no aggregates, and it asserts sampled process RSS, so it looks environment-specific and unrelated to this change. Flagging it in case it shows up in CI. -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
