hexbinoct commented on issue #22430:
URL: https://github.com/apache/datafusion/issues/22430#issuecomment-4592399944

   I started looking at this one and hit a wall worth flagging before someone 
copies the `Column` migration verbatim: `ScalarFunctionExpr` is the first 
**UDF-bearing** expression to be ported, and the current hook context doesn't 
yet cover what it needs.
   
   Its (de)serialization depends on the extension codec and task context, not 
just child expressions:
   
   - **encode:** `codec.try_encode_udf(self.fun(), &mut buf)` → `fun_definition`
   - **decode:** `codec.try_decode_udf(name, buf)` (or the 
`task_ctx().udf(name)` fallback) **plus** 
`task_ctx().session_config().options()` for the `config_options` passed to 
`ScalarFunctionExpr::new`
   
   The new `PhysicalExprEncodeCtx` / `PhysicalExprDecodeCtx` (in 
`datafusion-physical-expr-common`) currently expose only child encode/decode + 
the schema. Their docs already anticipate this: *"More specialized helpers 
(e.g. encoding UDFs/UDAFs/UDWFs through the extension codec) can be added to 
the context as expressions migrate."* So this port is really "add UDF support 
to the ctx" first.
   
   The catch is layering: the ctx traits live in `physical-expr-common`, but 
`ScalarUDF` lives in `datafusion-expr`, and `datafusion-expr` already depends 
on `datafusion-physical-expr-common` — so the ctx can't name `ScalarUDF` 
without a dependency cycle. Threading the UDF/codec through therefore needs a 
type-erased design (or some other indirection), which feels like a deliberate 
API decision rather than a copy-paste port.
   
   Before opening a PR: is there a preferred shape for a UDF hook on the ctx? 
Happy to implement whichever direction maintainers prefer. (The non-UDF 
siblings — Case, TryCast, Literal — look like clean `Column`-style ports and 
are already claimed.)


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