You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "adriangb (via GitHub)" <gi...@apache.org> on 2026/05/21 14:17:29 UTC

[I] Port `ScalarFunctionExpr` to use `try_to_proto` / `try_from_proto` [datafusion]

adriangb opened a new issue, #22430:
URL: https://github.com/apache/datafusion/issues/22430

   Sub-issue of #22418. Port `ScalarFunctionExpr` to use the `try_to_proto` / `try_from_proto` hooks added in #21929.
   
   Replace the `ScalarFunctionExpr` arm in `datafusion/proto/src/physical_plan/{to_proto,from_proto}.rs` with `try_to_proto` / `try_from_proto` impls next to `ScalarFunctionExpr`. See the `Column` / `BinaryExpr` migrations in #21929 for reference.


-- 
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: github-unsubscribe@datafusion.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [I] Port `ScalarFunctionExpr` to use `try_to_proto` / `try_from_proto` [datafusion]

Posted by "omer-cengel (via GitHub)" <gi...@apache.org>.
omer-cengel commented on issue #22430:
URL: https://github.com/apache/datafusion/issues/22430#issuecomment-4578154630

   Hi @adriangb !
   
   If it’s OK with you, I’d like to work on this 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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [I] Port `ScalarFunctionExpr` to use `try_to_proto` / `try_from_proto` [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb closed issue #22430: Port `ScalarFunctionExpr` to use `try_to_proto` / `try_from_proto`
URL: https://github.com/apache/datafusion/issues/22430


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [I] Port `ScalarFunctionExpr` to use `try_to_proto` / `try_from_proto` [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on issue #22430:
URL: https://github.com/apache/datafusion/issues/22430#issuecomment-4938780930

   After discussion / experimentation in the linked PRs we've come to the conclusion that this isn't worth doing. ScalarUDF will stay as a "special case" for now. If we need to support it more generically so that e.g. expressions wrapping a ScalarUDF can declare custom serialization we should probably go with https://github.com/apache/datafusion/pull/23421. Thanks all!


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [I] Port `ScalarFunctionExpr` to use `try_to_proto` / `try_from_proto` [datafusion]

Posted by "Olawoyin007 (via GitHub)" <gi...@apache.org>.
Olawoyin007 commented on issue #22430:
URL: https://github.com/apache/datafusion/issues/22430#issuecomment-4922990584

   I'd like to pick this up - it looks like the earlier discussion stalled. I'll follow the helper pattern from #22596 with the Column migration in #21929 as the reference, and account for the decode-side UDF lookup wrinkle discussed above. Planning to have a PR up in the next few days.


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [I] Port `ScalarFunctionExpr` to use `try_to_proto` / `try_from_proto` [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on issue #22430:
URL: https://github.com/apache/datafusion/issues/22430#issuecomment-4567240664

   Please base your PR on the helpers documented in #22418 (added in #22596). If you've already written this against the older inline pattern, the migration is small — see the second commit of #22596 for examples.
   


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [I] Port `ScalarFunctionExpr` to use `try_to_proto` / `try_from_proto` [datafusion]

Posted by "hexbinoct (via GitHub)" <gi...@apache.org>.
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org

Re: [I] Port `ScalarFunctionExpr` to use `try_to_proto` / `try_from_proto` [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on issue #22430:
URL: https://github.com/apache/datafusion/issues/22430#issuecomment-4578178938

   There was some discussion already in https://github.com/apache/datafusion/issues/22418#issuecomment-4564017491, lets make sure we are not duplicating work.
   
   @ernestprovo23 do you plan on working on this?


-- 
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: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org