mhilton commented on code in PR #24698:
URL: https://github.com/apache/datafusion/pull/24698#discussion_r4421613310
##########
datafusion/physical-expr/src/equivalence/properties/mod.rs:
##########
@@ -528,15 +617,21 @@ impl EquivalenceProperties {
Ok(())
}
- /// Updates the ordering equivalence class within assuming that the table
- /// is re-sorted according to the argument `ordering`, and returns whether
- /// this operation resulted in any change. Note that equivalence classes
- /// (and constants) do not change as they are unaffected by a re-sort. If
- /// the given ordering is already satisfied, the function does nothing.
+ /// Updates the ordering equivalence class assuming that the table is
+ /// re-sorted according to `ordering`, and returns whether the ordering
+ /// class changed. Equivalence classes and constants are unaffected by a
+ /// re-sort. Explicit grouping assertions are cleared because a sort may
+ /// rearrange rows that compare equally, even when `ordering` was already
+ /// satisfied.
pub fn reorder(
&mut self,
ordering: impl IntoIterator<Item = PhysicalSortExpr>,
) -> Result<bool> {
+ // A sort may reorder rows that compare equally under `ordering`, so
+ // explicit grouping assertions do not necessarily survive even when
+ // the requested ordering is already satisfied. Groupings implied by
+ // the output ordering remain discoverable through `oeq_class`.
+ self.clear_groupings();
Review Comment:
It might be that the ordering maintains the grouping, if every column in the
grouping is included in the ordering, and therefore the groupings could
filtered rather than just cleared. I suspect however that this is unnecessary
as ordering is the stronger signal and will be checked first.
--
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]
