jayzhan211 commented on code in PR #27191:
URL: https://github.com/apache/datafusion/pull/27191#discussion_r4504554781
##########
datafusion/physical-optimizer/src/ensure_requirements/enforce_sorting/sort_pushdown.rs:
##########
@@ -1318,10 +1318,28 @@ fn handle_projection_pushdown(
// (expressed in the projection's output schema) must be remapped into the
// child schema before being pushed down. Forwarding `score@1` unchanged
// could refer to a different input column and sort on the wrong key.
- Ok(
+ let Some(requirements) =
remap_requirement_through_projection(projection_exec, parent_required)
- .map(|requirements| vec![Some(requirements)]),
- )
+ else {
+ return Ok(None);
+ };
+ // If the input already has this ordering, no sort will be added below the
+ // projection. The parent still needs the projection to prove it, which it
+ // may not be able to do: the proof can depend on a column that it drops,
+ // such as a leading per-partition constant. Keep the sort above the
+ // projection in that case.
+ if projection_exec
Review Comment:
Yes, for this shape. `implied_by` now tries every key, not only the first
one past the constants, so `[k, a, c]` with `a = CAST(c AS DATE)` also stores
`[k, c]`, and the projection keeps it after dropping `a`. Your query plans with
no sort again. At 4 partitions it plans just the `SortPreservingMergeExec`,
where `main` fails `SanityCheckPlan`. The TopK form has no extra sort either,
and window.slt now checks its plan.
The guard is still needed when a projection drops a leading per-partition
constant, as in the #26762 query without `k` in SELECT (window.slt:7900): the
input satisfies `[total]`, but no ordering survives the projection. There the
guard keeps the sort above the projection, which is where `main` has it. I
tried having the projection drop the constant from its orderings and removing
the guard, but then 2 generated queries fail `SanityCheckPlan`, so the guard
stays.
Across the 7,440 generated runs, the PR planned more sorts than `main` in 17
runs, and now in 4. Those 4 are a `GROUP BY k, c` over a TopK on `(a, c)` at 4
partitions: knowing `[c]`, the aggregate streams on `c` and adds a sort on `c`
after the hash repartition. `main` picks the same streaming aggregate when the
ordering is known before the equality. I updated the worst case in the
description.
--
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]
