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

   ## Problem
   
   `CometNativeUDF.register` asks the caller for the full signature:
   
   ```scala
   CometNativeUDF.register(spark, "add_one_c", libPath, Seq(LongType), LongType)
   ```
   
   but the library already knows most of it. A kernel's `return_field` computes 
the output type from the argument types, and Comet calls it at planning time to 
check the declaration, so the declared `returnType` is verified against the 
library rather than being the source of truth. Raised by @wForget on #4459: can 
`inputTypes` / `returnType` / `deterministic` come from the library instead?
   
   ## Where each one stands
   
   **`returnType`** is the strongest candidate, and it is already redundant in 
the sense that a wrong value is rejected. The obstacle is ordering, not 
information: Spark's analyzer needs a concrete `DataType` when `register` 
installs the catalog stub, and at that moment the argument types of the 
eventual call sites are not known. Deriving it would mean calling the library 
on the driver with the `inputTypes` the caller supplied, which removes one of 
the two declarations but not both.
   
   **`inputTypes`** cannot be derived as things stand. The ABI has no way to 
enumerate a kernel's accepted signatures: `return_field` is a predicate over 
argument types, not a description of them, and a kernel like `echo_c` accepts 
every type. Getting this from the library would need a new ABI entry point 
returning declared signatures, which is an ABI break and worth its own design.
   
   **`deterministic`** is per-registration today and always has to be `true` 
(#5249). It is genuinely a property of the kernel rather than the registration, 
so if anything it belongs on the library side, but that is blocked behind 
honoring it at all.
   
   ## Possible shape
   
   An overload that takes only the arguments and derives the return type:
   
   ```scala
   CometNativeUDF.register(spark, "add_one_c", libPath, Seq(LongType))
   ```
   
   calling the library on the driver to resolve the return type, and failing 
with the kernel's own message if the kernel rejects those argument types. The 
existing overload stays for callers who want the declaration checked.
   
   That leaves the plan-time check in place. It is still worth keeping even 
when the type was derived, because the derivation happens on the driver and the 
check runs on the executor against the library actually loaded there, which 
need not be the same file.
   
   ## Related
   
   - #4459, where this was raised
   - #5249, honoring `deterministic`
   - #4173, the equivalent validation gap on the JVM `CometUDF` path
   


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