andygrove opened a new issue, #5296:
URL: https://github.com/apache/datafusion-comet/issues/5296

   Follow-up from review of #4459 
([thread](https://github.com/apache/datafusion-comet/pull/4459#discussion_r3730399898)),
 raised by @mbutrovich.
   
   `ImportedCScalarUdf::invoke_with_args` does the following on **every batch**:
   
   1. builds a fresh `CometCScalarKernelImpl` via `new_impl`,
   2. re-encodes every argument `Field` to an `FFI_ArrowSchema`,
   3. calls the kernel's `init`, which re-invokes its `return_field`,
   4. decodes the resulting `FFI_ArrowSchema` back into a `Field`,
   5. drops the impl, running its `release` callback.
   
   Argument types do not change across batches for a given 
`ScalarFunctionExpr`, and `return_type()` already did the same work once at 
planning time, so steps 2 and 4 are pure repetition.
   
   Two separable pieces of work:
   
   **Drop the return-type decode (small).** DataFusion 54's 
`ScalarFunctionArgs` carries `return_field`, so `Field::try_from(&out_schema)` 
in `invoke_with_args` is unnecessary. Careful, though: today the output array 
is imported with the type the kernel reported for *this* batch, and switching 
to the plan-time type means `from_ffi_and_data_type` receives a type that can 
differ in nested nullability from what the kernel actually produced — the same 
asymmetry the declared-vs-actual check in `planner.rs` deliberately erases. 
Needs the complex-type matrix in `CometRustUdfSuite` re-run rather than a spot 
check.
   
   **Cache the impl across batches (larger, ABI decision).** The per-batch impl 
lifecycle is currently documented in `comet-udf-sdk`, so a kernel author may 
assume a fresh impl per call. Reusing one across batches changes that contract 
and should be decided deliberately, alongside the thread-safety question in the 
Mutex issue.
   


-- 
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]

Reply via email to