Brijesh-Thakkar commented on code in PR #22705:
URL: https://github.com/apache/datafusion/pull/22705#discussion_r3390522199
##########
datafusion/ffi/src/udwf/mod.rs:
##########
@@ -358,6 +488,83 @@ impl WindowUDFImpl for ForeignWindowUDF {
}
}
+ fn documentation(&self) -> Option<&Documentation> {
+ unsafe {
+ let ptr = (self.udf.documentation)(&self.udf);
+ ptr.as_ref()
+ }
+ }
+
+ fn expressions(
+ &self,
+ expr_args: datafusion_expr::function::ExpressionArgs,
+ ) -> Vec<Arc<dyn PhysicalExpr>> {
+ unsafe {
+ let fallback = expr_args.input_exprs().to_vec();
+ let args = match FFI_ExpressionArgs::try_from(expr_args) {
+ Ok(args) => args,
+ Err(_) => return fallback,
+ };
+ (self.udf.expressions)(&self.udf, args)
+ .into_iter()
+ .map(|e| Arc::<dyn PhysicalExpr>::from(&e))
+ .collect()
+ }
+ }
+
+ fn simplify(&self) -> Option<WindowFunctionSimplification> {
+ if !self.udf.has_simplify {
+ return None;
+ }
+
+ let udf = self.udf.clone();
+ Some(Box::new(move |wf, info| {
+ let codec =
datafusion_proto::logical_plan::DefaultLogicalExtensionCodec {};
+
+ // To serialize the window function
+ let expr = Expr::WindowFunction(Box::new(wf));
+ let protobuf =
datafusion_proto::logical_plan::to_proto::serialize_expr(
+ &expr, &codec,
+ )
Review Comment:
WindowFunctionSimplification is a Box<dyn Fn(WindowFunction, &dyn
SimplifyInfo) -> Result<Expr>>. Closures cannot cross FFI boundaries directly,
so the only viable approach is: the consumer serializes the WindowFunction to
bytes, calls the fn pointer, and the producer deserializes, executes its
closure, and returns the serialized Expr. Would you prefer we skip cross-FFI
simplify in this PR and document it as a known follow-up (returning None from
ForeignWindowUDF::simplify() while keeping the has_simplify capability flag),
or is the serialization approach acceptable with the codec fixed?
--
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]