xavlee commented on code in PR #24698:
URL: https://github.com/apache/datafusion/pull/24698#discussion_r4427077771
##########
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:
Thanks for the review @mhilton. That's the intent here 🙂 grouping implied by
the resulting ordering remains discoverable when determining a ClusteringMode,
so we don't need to retain the grouping assertions here.
--
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]
