Re: [PR] feat: custom Rust UDFs via arrow-ffi (alternative to #4283) [experimental] [datafusion-comet]

["andygrove (via GitHub)" <gi...@apache.org>]

andygrove commented on code in PR #4459:
URL: https://github.com/apache/datafusion-comet/pull/4459#discussion_r3604117633


##########
native/proto/src/proto/expr.proto:
##########
@@ -530,3 +531,22 @@ message JvmScalarUdf {
   // Whether the result column may contain nulls.
   bool return_nullable = 4;
 }
+
+// Call to a user-supplied Rust UDF loaded from a cdylib.
+//
+// The native side resolves (library_path, name) against its loaded-library
+// cache, looks up the kernel by name, and invokes it through whichever ABI
+// flavor (C ABI / datafusion-ffi) the cdylib registered the kernel under.
+message RustUdfCall {
+  // Function name as registered through CometRustUDF.register on the JVM
+  // side; matched against names exposed by the cdylib.
+  string name = 1;
+  // Filesystem path of the cdylib.
+  string library_path = 2;

Review Comment:
   Agreed. The proto struct is intentionally open here so a future variant can carry a `RegistrationSource` oneof (SharedObject | Jar | PythonPackage) without breaking wire compat. The shared-object form is the shortest path for the Rust ecosystem; the jar / Python paths would fan out from the same registration message.



##########
native/proto/src/proto/expr.proto:
##########
@@ -530,3 +531,22 @@ message JvmScalarUdf {
   // Whether the result column may contain nulls.
   bool return_nullable = 4;
 }
+
+// Call to a user-supplied Rust UDF loaded from a cdylib.
+//
+// The native side resolves (library_path, name) against its loaded-library
+// cache, looks up the kernel by name, and invokes it through whichever ABI
+// flavor (C ABI / datafusion-ffi) the cdylib registered the kernel under.
+message RustUdfCall {
+  // Function name as registered through CometRustUDF.register on the JVM
+  // side; matched against names exposed by the cdylib.
+  string name = 1;
+  // Filesystem path of the cdylib.
+  string library_path = 2;
+  // Argument expressions, evaluated before invocation.
+  repeated Expr args = 3;
+  // Expected return type, declared at register time on the JVM side.
+  DataType return_type = 4;

Review Comment:
   Not strictly. This PR pins it because the current dispatch resolves the return type at plan time (so we can allocate the output vector once per batch) and Spark expects a fixed `DataType` for scalar UDFs. Widening to a compute-on-demand return type would let a single UDF handle polymorphic outputs, at the cost of a per-batch call across the FFI to resolve. If that becomes valuable I can follow up with an optional `return_type_fn` slot.



-- 
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-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org