bubulalabu commented on issue #17379:
URL: https://github.com/apache/datafusion/issues/17379#issuecomment-3394135457

   I've looked into how well the Signature based named arguments implementation 
might work for the other UDF types.
   
   The approach using `Signature.parameter_names` seems like it will work well 
for Aggregate and Window UDFs since they already use the same `Signature` 
struct. Table Functions are a bit different though.
   
   **Aggregate UDFs** already use `Signature` (see 
[`AggregateUDFImpl::signature()` in 
udaf.rs:502](https://github.com/apache/datafusion/blob/main/datafusion/expr/src/udaf.rs#L502))
 so `.with_parameter_names()` should just work.
   
   The main thing is the SQL planner at calls `function_args_to_expr()` which 
doesn't capture argument names. We'd need to change it to use 
`function_args_to_expr_with_names()` and call `resolve_function_arguments()` 
like we did for scalar UDFs.
   
   Example usage: `SELECT sum(value => column1)`
   
   **Window UDFs** also use `Signature` (see [`WindowUDFImpl::signature()` in 
udwf.rs:329](https://github.com/apache/datafusion/blob/main/datafusion/expr/src/udwf.rs#L329))
 so the same approach should work.
   
   The SQL planner at has the same issue and would need the same fix.
   
   Example usage: `SELECT lag(column => value, offset => 1, default => 0) OVER 
(ORDER BY id)`
   
   **Table Functions** don't use `Signature` at all right now. The 
`TableFunctionImpl` trait just has `call(&self, args: &[Expr])` (see 
[catalog/table.rs:456-459](https://github.com/apache/datafusion/blob/main/datafusion/catalog/src/table.rs#L456-L459)).
   
   Interestingly the SQL planner at 
[relation/mod.rs:165-177](https://github.com/apache/datafusion/blob/main/datafusion/sql/src/relation/mod.rs#L165-L177)
 already parses named arguments but then throws away the names.
   
   Not sure what the best approach is here - maybe add `Signature` to keep 
things consistent, or maybe there's a reason Table Functions don't use 
Signatures that I'm missing.
   
   Example target usage: `SELECT * FROM read_csv(path => 'file.csv', delimiter 
=> ',')`
   
   I think extending to Aggregate and Window UDFs first would be pretty 
straightforward since they already fit the same pattern. Table Functions 
probably need some discussion about the right approach.


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