alamb commented on code in PR #14037:
URL: https://github.com/apache/datafusion/pull/14037#discussion_r1905777670
##########
datafusion/physical-expr/src/equivalence/ordering.rs:
##########
@@ -53,24 +53,33 @@ impl OrderingEquivalenceClass {
self.orderings.clear();
}
- /// Creates new ordering equivalence class from the given orderings.
+ /// Creates new ordering equivalence class from the given orderings
+ ///
+ /// Any redundant entries are removed, as described on
[`Self::remove_redundant_entries`].
pub fn new(orderings: Vec<LexOrdering>) -> Self {
let mut result = Self { orderings };
result.remove_redundant_entries();
result
}
+ /// Converts this OrderingEquivalenceClass to a vector of orderings.
+ pub fn into_inner(self) -> Vec<LexOrdering> {
+ self.orderings
+ }
+
/// Checks whether `ordering` is a member of this equivalence class.
pub fn contains(&self, ordering: &LexOrdering) -> bool {
self.orderings.contains(ordering)
}
/// Adds `ordering` to this equivalence class.
#[allow(dead_code)]
+ #[deprecated(
Review Comment:
note this method is only used in tests (and thus needed "allow(dead_code)").
It is the same as `add_new_ordering` so I made that explicit and deprecated the
method and updated the tests.
##########
datafusion/physical-expr/src/equivalence/properties.rs:
##########
@@ -2009,16 +2009,8 @@ fn calculate_union_binary(
// Next, calculate valid orderings for the union by searching for prefixes
// in both sides.
let mut orderings = UnionEquivalentOrderingBuilder::new();
- orderings.add_satisfied_orderings(
- lhs.normalized_oeq_class().orderings,
- lhs.constants(),
- &rhs,
- );
- orderings.add_satisfied_orderings(
- rhs.normalized_oeq_class().orderings,
- rhs.constants(),
- &lhs,
- );
+ orderings.add_satisfied_orderings(lhs.normalized_oeq_class(),
lhs.constants(), &rhs);
Review Comment:
most of the test changes I think actually make the code easier to read --
for example `OrderingEquivalenceClass` already implements `IntoIterator` so
there is no need to explicitly name the inner field
--
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: github-unsubscr...@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscr...@datafusion.apache.org
For additional commands, e-mail: github-h...@datafusion.apache.org
