adriangb commented on code in PR #17323:
URL: https://github.com/apache/datafusion/pull/17323#discussion_r2299788603


##########
datafusion/core/tests/physical_optimizer/filter_pushdown/mod.rs:
##########
@@ -595,7 +595,7 @@ fn test_no_pushdown_through_aggregates() {
         Ok:
           - FilterExec: b@1 = bar
           -   CoalesceBatchesExec: target_batch_size=100
-          -     AggregateExec: mode=Final, gby=[a@0 as a, b@1 as b], 
aggr=[cnt], ordering_mode=PartiallySorted([0])
+          -     AggregateExec: mode=Final, gby=[a@0 as a, b@1 as b], aggr=[cnt]

Review Comment:
   Not sure about this, need to think about it



##########
datafusion/proto/tests/cases/roundtrip_physical_plan.rs:
##########
@@ -1000,7 +1000,7 @@ fn roundtrip_parquet_exec_with_custom_predicate_expr() -> 
Result<()> {
             self: Arc<Self>,
             _children: Vec<Arc<dyn PhysicalExpr>>,
         ) -> Result<Arc<dyn PhysicalExpr>> {
-            todo!()
+            Ok(self)

Review Comment:
   Now gets called by `reassign_predicate_columns`



##########
datafusion/physical-expr/src/equivalence/properties/mod.rs:
##########
@@ -326,6 +326,22 @@ impl EquivalenceProperties {
         self.add_orderings(std::iter::once(ordering));
     }
 
+    fn update_oeq_cache(&mut self) -> Result<()> {
+        // Renormalize orderings if the equivalence group changes:
+        let normal_cls = mem::take(&mut self.oeq_cache.normal_cls);
+        let normal_orderings = normal_cls
+            .into_iter()
+            .map(|o| self.eq_group.normalize_sort_exprs(o));
+        self.oeq_cache.normal_cls = 
OrderingEquivalenceClass::new(normal_orderings);
+        self.oeq_cache.update_map();
+        // Discover any new orderings based on the new equivalence classes:
+        let leading_exprs: Vec<_> = 
self.oeq_cache.leading_map.keys().cloned().collect();
+        for expr in leading_exprs {
+            self.discover_new_orderings(expr)?;
+        }
+        Ok(())
+    }

Review Comment:
   This ~ same code was in 2 different places, this is a drive by deduplication



-- 
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

Reply via email to