Repository navigation
docs: Explain SortKeyWalk with worked examples - #27266
Conversation
Co-Authored-By: Claude Fable 5.1 <[email protected]>
github-actions
Bot
removed
the
documentation
| /// rows: they are sorted by `x`; among rows with equal `x`, sorted by `y`; | ||
| /// among rows with equal `x` and `y`, sorted by `z`. | ||
| /// | ||
| /// ## Pinning and Fixed Expressions |
There was a problem hiding this comment.
Once I went through this logic, I found the algorithm quite clever and the refactoring well done. Thank you for working on this @jayzhan211
|
I am sorry to ask you for another review @jayzhan211 but I think this is related to your attempt to try and clean up SortKeyWalk, etc cc @ozankabak and @berkaysynnada @buraksenn who may also have worked on this code and be available for a review |
| /// For each ordering (the normal orderings of `props`, then `implied`), | ||
| /// the index of its head. Empty until the first pin, so that a walk with | ||
| /// nothing pinned allocates nothing; until then every head is at index 0. | ||
| /// The position of the head of each ordering. Empty until the first pin. |
There was a problem hiding this comment.
I found the "heads" notion quite confusing at first, but I think this captures it
jayzhan211
left a comment
jayzhan211
left a comment
There was a problem hiding this comment.
@alamb thanks for writing this up. I ran each worked example through ordering_satisfy and all of them match the code. The inline suggestions are optional doc fixes.
Optional cleanup in walk.rs around line 542: splitting the paragraph left a short line (/// Pinning kleavesx``), so the paragraph could be rewrapped.
Co-authored-by: Jay Zhan <[email protected]>
Co-authored-by: Jay Zhan <[email protected]>
Co-Authored-By: Claude Fable 5.2 <[email protected]>
github-actions
Bot
removed
the
documentation
Which issue does this PR close?
SortKeyWalk#27234Rationale for this change
While reviewing #27191 I was very confused about how
SortKeyWalk(added in #26853) decides whether known orderings satisfy a sort requirement, and wanted to write down what I learned so others do not have to repeat it.Specifically the existing docs stated the rules but did not define the terms clearly, and had no example a reader could trace step by step.
What changes are included in this PR?
Rustdoc only, in
datafusion/physical-expr/src/equivalence/properties/walk.rs:What is the testing strategy for this PR?
CI
Are there any user-facing changes?
No.