comphead commented on code in PR #21896:
URL: https://github.com/apache/datafusion/pull/21896#discussion_r3156927369
##########
datafusion/physical-expr/src/higher_order_function.rs:
##########
@@ -73,15 +88,13 @@ pub struct HigherOrderFunctionExpr {
/// LiteralExpression(2)
/// ```
args: Vec<Arc<dyn PhysicalExpr>>,
- /// Positions in `args` where lambdas were top level arguments during
try_new_with_schema
- /// but may have been wrapped by tree node rewrites via with_new_children,
like:
- ///
- /// ```ignore
- /// expr.transform(|expr|
Ok(Transformed::yes(Arc::new(DebugExpr::new(expr)))))
- /// ```
- ///
- /// For example, for `array_transform([2, 3], v -> v != 2)`, this will be
`vec![1]`
- lambda_positions: Vec<usize>,
+ /// Per-arg classification, parallel to `args`. Length always equals
+ /// `args.len()`. Lambda variants carry the resolved inner [`LambdaExpr`]
+ /// so `evaluate` doesn't walk through wrapper nodes.
+ slots: Vec<ArgSlot>,
+ /// Cached value of [`HigherOrderUDF::clear_null_values`]. Avoids a virtual
+ /// call per non-lambda arg per batch.
+ clear_null_values: bool,
Review Comment:
let me think. For current implementation it is not worthy, because trait
just return true and LLVM can optimize this virtual call to just using a
constant. however in future this method can be overridden and depending on
implementation can potentially cause troubles. This micro optimization
probably is too premature, you are right
--
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]
