jayzhan211 commented on code in PR #26879:
URL: https://github.com/apache/datafusion/pull/26879#discussion_r4430652271
##########
datafusion/physical-expr/src/equivalence/properties/mod.rs:
##########
@@ -1112,11 +1114,16 @@ impl EquivalenceProperties {
// Get dependency map for existing orderings:
oeq_class = Self::substitute_oeq_class(&self.schema, &mapping,
oeq_class);
Review Comment:
When the input is ordered by the wide column itself, `substitute_oeq_class`
still gives every source that refers to it a fresh `ColumnRanges`, so the range
is rebuilt per source again. With your benchmark's input ordered by `s` instead
of `a`, `project` takes ~14.8 ms on this branch. `projected_orderings` is the
only caller and uses the same `self.schema`, so sharing the one cache brings it
to ~0.8 ms locally (noisy machine), and the `equivalence` tests still pass.
Could you apply this and add the ordered-by-`s` variant to the benchmark?
```diff
@@ fn substitute_order_preserving_ordering(
sort_expr: &PhysicalSortExpr,
schema: &SchemaRef,
+ column_ranges: &ColumnRanges,
) -> Option<PhysicalSortExpr> {
@@
let properties =
- get_expr_properties(&r_expr, &dependencies, schema,
&ColumnRanges::default())
- .ok()?;
+ get_expr_properties(&r_expr, &dependencies, schema,
column_ranges).ok()?;
@@ fn substitute_oeq_class(
oeq_class: OrderingEquivalenceClass,
+ column_ranges: &ColumnRanges,
) -> OrderingEquivalenceClass {
@@
&original_sort_expr,
schema,
+ column_ranges,
)
@@ fn projected_orderings(
let mapping = self.normalize_mapping(mapping);
- // Get dependency map for existing orderings:
- oeq_class = Self::substitute_oeq_class(&self.schema, &mapping,
oeq_class);
- let dependency_map = self.construct_dependency_map(oeq_class,
&mapping);
// Every source is examined, ...
let column_ranges = ColumnRanges::default();
+ oeq_class =
+ Self::substitute_oeq_class(&self.schema, &mapping, oeq_class,
&column_ranges);
+ // Get dependency map for existing orderings:
+ let dependency_map = self.construct_dependency_map(oeq_class,
&mapping);
```
--
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]
