alamb commented on code in PR #9496:
URL: https://github.com/apache/arrow-datafusion/pull/9496#discussion_r1518382012
##########
datafusion/optimizer/src/analyzer/rewrite_expr.rs:
##########
@@ -118,11 +118,59 @@ impl TreeNodeRewriter for OperatorToFunctionRewriter {
args: vec![left, right],
})));
}
+
+ // TODO: change OperatorToFunction to OperatoToArrayFunction and
configure it with array_expressions feature
+ // after other array functions are udf-based
+ #[cfg(feature = "array_expressions")]
+ if let Some(expr) = rewrite_array_has_all_operator_to_func(left,
op, right) {
+ return Ok(Transformed::yes(expr));
+ }
}
Ok(Transformed::no(expr))
}
}
+#[cfg(feature = "array_expressions")]
Review Comment:
So it seems like ideally that users who want to supply their own
implementations for `array_expressions` should also supply their own Analyzer
passes.
Following that logic, it seems like the `datafusion_array_functions` crate
should supply its own analyzer pass that had array function specific rewrites.
Does that make sense @jayzhan211 and @guojidan?
If so, I will file a ticket to track the request / work.
Note that since I don't think users can actually add their own Analyzer
passes today, we can't implement this idea yet. Thus I think we can still merge
this PR.
I also think it would help to explain this rationale in comments. For example
```suggestion
// Note This rewrite is only done if the built in DataFusion
`array_expressions` feature is enabled.
// Even if users implement their own array functions, those functions are
not equal to the DataFusion
// udf based array functions, so this rewrite is not corrrect
#[cfg(feature = "array_expressions")]
```
--
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]
