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

   Follow-up from review of #4459 
([thread](https://github.com/apache/datafusion-comet/pull/4459#discussion_r2601292184)).
   
   `ImportedCScalarUdf::return_type` already calls the kernel through the C 
ABI, decodes an `FFI_ArrowSchema` into a full `Field`, and then throws 
everything but the `DataType` away. The planner separately hardcodes the output 
field as nullable:
   
   ```rust
   let return_field = Arc::new(Field::new(&call.name, kernel_return_type, 
true));
   ```
   
   So a kernel that reports a non-nullable output, or attaches field metadata, 
has both discarded.
   
   Implementing `ScalarUDFImpl::return_field` instead would carry the kernel's 
own `Field` through to the plan, and would let the planner stop fabricating one.
   
   Worth checking before doing it:
   
   - Spark UDF results are nullable in Spark's own schema, so promising 
DataFusion a non-nullable field needs to not trip a schema comparison at the 
Comet boundary or in the shuffle writer.
   - The declared-vs-actual return type check in `planner.rs` deliberately 
erases nested nullability, because Spark carries `containsNull` in the type 
while the delivered array normalizes children to nullable. Whatever this does 
with top-level nullability should be consistent with that.
   - A test for a kernel returning a non-nullable field, since nothing 
exercises that today.


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