alamb commented on code in PR #27191:
URL: https://github.com/apache/datafusion/pull/27191#discussion_r4506205677
##########
datafusion/physical-expr/src/equivalence/properties/mod.rs:
##########
@@ -568,13 +583,32 @@ impl EquivalenceProperties {
let mut new_orderings = vec![];
for idx in ordering_idxs {
let ordering = &self.oeq_cache.normal_cls[*idx];
- new_orderings
- .extend(self.implied_ordering(&ordering[0],
ordering[1..].iter())?);
+ new_orderings.extend(self.implied_by(ordering)?);
}
- if !new_orderings.is_empty() {
- self.add_orderings(new_orderings);
+ self.add_orderings(new_orderings)
+ }
+
+ /// Returns the orderings that the normal ordering `ordering` implies. If a
+ /// key is a function of the keys after it, as `a` is in `[k, a, c]` with
+ /// `a = CAST(c AS DATE)`, rows tied on the keys before it are sorted by
the
+ /// keys after it, so the ordering without that key holds: `[k, c]`. The
+ /// constants that a normal ordering can keep (those that differ between
+ /// partitions) are left out, because within a partition they have one
+ /// value.
+ fn implied_by(&self, ordering: &LexOrdering) ->
Result<Vec<Vec<PhysicalSortExpr>>> {
+ let keys: Vec<_> = ordering
+ .iter()
+ .filter(|sort_expr|
self.eq_group.is_expr_constant(&sort_expr.expr).is_none())
+ .collect();
+ let mut implied = vec![];
+ for (idx, key) in keys.iter().enumerate() {
+ if let Some(rest) =
+ self.implied_ordering(key, keys[idx + 1..].iter().copied())?
Review Comment:
it seems like the fact that `implied_ordering` returns a Result here
requires many signature changes (and the upgrade guide entry) which
significantly complicates the PR.
Since the only error that is actually possible is if the type can't be found
from the schema, which seems like a safe thing to ignore (`.ok()`, despite my
previous comment to the contrary 🤦 )
--
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]
