adriangb commented on code in PR #20337:
URL: https://github.com/apache/datafusion/pull/20337#discussion_r2872938534
##########
datafusion-examples/examples/custom_data_source/custom_datasource.rs:
##########
@@ -283,4 +284,20 @@ impl ExecutionPlan for CustomExec {
None,
)?))
}
+
+ fn apply_expressions(
+ &self,
+ f: &mut dyn FnMut(
+ &dyn datafusion::physical_plan::PhysicalExpr,
Review Comment:
should this / could this be `&Arc<dyn PhysicalExpr>`?
##########
datafusion-examples/examples/custom_data_source/custom_datasource.rs:
##########
@@ -283,4 +284,20 @@ impl ExecutionPlan for CustomExec {
None,
)?))
}
+
+ fn apply_expressions(
+ &self,
+ f: &mut dyn FnMut(
+ &dyn datafusion::physical_plan::PhysicalExpr,
+ ) -> Result<TreeNodeRecursion>,
+ ) -> Result<TreeNodeRecursion> {
+ // Visit expressions in the output ordering from equivalence properties
+ let mut tnr = TreeNodeRecursion::Continue;
+ if let Some(ordering) = self.cache.output_ordering() {
+ for sort_expr in ordering {
+ tnr = tnr.visit_sibling(|| f(sort_expr.expr.as_ref()))?;
Review Comment:
Worth putting in as a code comment? I always struggle to follow the logic of
these tree traversals. It makes sense once you get it but yeah they're hard to
grok - having a comment that tries to explain in words what is going on may be
helpful.
##########
docs/source/library-user-guide/upgrading/53.0.0.md:
##########
@@ -28,6 +28,73 @@
[#19692]: https://github.com/apache/datafusion/issues/19692
+### `ExecutionPlan::apply_expressions` is now a required method
Review Comment:
@LiaCastaneda the 53.0.0 branch has been cut, so I don't think this will
make it in since it's decidedly a new feature. Is that okay with you? If so
let's move this section to 54.0.0. I think it's always good to merge a new API
right after a release, it gives us time to make non breaking changes if we find
issues 2 weeks in.
--
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]
