Repository navigation
docs: Clarify EquivalenceClass, ConstExpr, and OrderingEquivalenceClass docs - #27238
Conversation
…s docs Co-Authored-By: Claude Fable 5.1 <[email protected]>
|
|
||
| /// Represents whether a constant expression's value is uniform or varies across | ||
| /// partitions. Has two variants: | ||
| /// - `Heterogeneous`: The constant expression may have different values for |
There was a problem hiding this comment.
I think it is more common to document the field names at their definition site rather than in the module level so I moved them there
| /// different partitions. | ||
| /// - `Uniform(Option<ScalarValue>)`: The constant expression has the same value | ||
| /// across all partitions, or is `None` if the value is unknown. | ||
| /// How a constant expression's value varies across partitions. |
There was a problem hiding this comment.
I also tried to make this more concise so it is easier to undersand
| /// earlier in the plan, `A` would become a constant in subsequent operations. | ||
| /// | ||
| /// # Fields | ||
| /// An expression known to be constant during the execution of a query. |
There was a problem hiding this comment.
similarly I made this more concise and removed the redundant field level documentation
| /// `EquivalenceGroup` supports fast lookups of expressions and their | ||
| /// equivalence classes. See [`EquivalenceProperties`] for more details. | ||
| /// | ||
| /// [`EquivalenceProperties`]: crate::equivalence::EquivalenceProperties |
There was a problem hiding this comment.
I am trying to route all docs back to EquivalenceProperties where we can centralize as much as possible
| /// [`GroupingEquivalenceClass`] is used when groups of equal values are | ||
| /// contiguous, but not in order. | ||
| /// | ||
| /// An `OrderingEquivalenceClass` is a set of such equivalent orderings, which |
There was a problem hiding this comment.
this describes how the data is represented and invariants (which is an implementation detail) -- moved to the field definition
| /// `[k, w]` once for each of the four directions of `w`. | ||
| /// | ||
| /// ### Future Improvements | ||
| /// This is how a whole-partition window aggregate records that its value |
There was a problem hiding this comment.
I found the existing documentation quite hard to understand about whole-partition window aggregates, etc. more coming soon
Co-authored-by: xavlee <[email protected]>
github-actions
Bot
removed
the
documentation
jayzhan211
left a comment
jayzhan211
left a comment
There was a problem hiding this comment.
@alamb thanks for writing this down. The clippy failure blocks the merge queue; it's a one-line fix, details inline.
Co-authored-by: Jay Zhan <[email protected]>
Co-authored-by: Jay Zhan <[email protected]>
|
I think it is ready to merge! |
|
Thank you @jayzhan211 -- I am glad we are all working to make this code easier to undersrtand and maintain |
Which issue does this PR close?
Rationale for this change
While reviewing #27191 I was researching how
EquivalencePropertiestracks orderings, equal expressions and constants, and wanted to write down what I learned so others do not have to repeat it.Several of the struct docs in the
equivalencemodule were out of date or hard to follow. For example, theConstExprdocs still describedacross_partitionsas a boolean flag.What changes are included in this PR?
Improve docs
What is the testing strategy for this PR?
CI
Are there any user-facing changes?
No.