alamb opened a new pull request, #27140: URL: https://github.com/apache/datafusion/pull/27140
## Which issue does this PR close? - Closes #26762 ## Rationale for this change A window ordered by `(total DESC, k)` fails to plan with a `SanityCheckPlan` error when `k` is constant within each partition (for example `lower(g)` under `WHERE g = 'a'`), its input is sorted by `[k, total]` (from `SUM(...) OVER (PARTITION BY k)`), and `target_partitions = 1`. The query planned fine in 54.1.0 and fails since 55.0.0. Since #23528, a normalized ordering keeps a constant whose value differs between partitions (`AcrossPartitions::Heterogeneous`), so such a constant can lead a normal ordering, as `k` does in `[k, total]`. `SortKeyWalk` assumed that every ordering's head is at index 0 until the first key is pinned, so it treated `k` as the head and never saw `total`. The requirement `[total DESC, k]` was therefore judged unsatisfied by the sanity check, while the window sort removal (which goes through `add_constants`, which drops all constants from the normal orderings) judged it satisfied, and no sort was added. ## What changes are included in this PR? `SortKeyWalk::new` computes each ordering's initial head past leading fixed expressions, the same way the walk already moves heads after each pin. When no normal ordering starts with a fixed expression, nothing changes and no allocation happens. Normalization, `output_ordering()` and the sort pruning in the physical optimizer are untouched, so the behavior from #23528 and #26455 is preserved. (I also tried dropping per-partition constants from normalized orderings again. That fixes the repro too but makes `output_ordering()` lose the constant, which turns an order-preserving repartition into a blocking sort over an unbounded input in `range_join_key_alignment_ignores_per_partition_constants`.) ## What is the testing strategy for this PR? - Unit test `leading_per_partition_constant_is_not_a_head` in `walk.rs`. It fails on `main` with the assertion this fix targets. - The query from the issue and its physical plan (no sort added) in `window.slt`. I ran the `datafusion-physical-expr` unit tests, the `physical_optimizer` tests in `core_integration`, and the `order`, `union`, `window*`, `topk`, `limit`, `aggregate*` and `joins*` sqllogictest files locally. ## Are there any user-facing changes? Affected queries plan and run again. No API changes. This is a regression in 55.0.0 and later, so it is a candidate for a backport to `branch-56`. -- 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]
