Olawoyin007 opened a new pull request, #23415: URL: https://github.com/apache/datafusion/pull/23415
## Which issue does this PR close? - Closes #22430. Part of epic #22418. ## Rationale for this change `ScalarFunctionExpr` was still serialized through the central downcast chain in `datafusion-proto`. This ports it to own its protobuf (de)serialization via the `try_to_proto` / `try_from_proto` hooks (#21929), following the helper pattern from #22596. Unlike the previously migrated expressions, `ScalarFunctionExpr` needs the extension codec (UDF payloads) and the session config on the decode side. Per the discussion on #22418, this PR extends the encode/decode contexts with those helpers in the same change: - `PhysicalExprEncodeCtx::encode_udf` - routes a scalar UDF through `PhysicalExtensionCodec::try_encode_udf`, returning the opaque `fun_definition` payload (`None` when the codec writes nothing). - `PhysicalExprDecodeCtx::decode_udf` - resolves a UDF from the payload via `try_decode_udf`, falling back to the task context's function registry by name (same lookup order as the old inline arm). - `PhysicalExprDecodeCtx::config_options` - session `ConfigOptions` for expressions that carry them. **On the type-erased UDF signatures:** as flagged in the issue discussion, `ScalarUDF` lives in `datafusion-expr`, which depends on `datafusion-physical-expr-common`, so the ctx traits cannot name it without a dependency cycle. `encode_udf` takes `&(dyn Any + Send + Sync)` and `decode_udf` returns `Arc<dyn Any + Send + Sync>`; the concrete type is always (`Arc<`)`ScalarUDF` and both sides are documented as such. The downcasts live in one place each (`ConverterEncoder` in `datafusion-proto`, `ScalarFunctionExpr::try_from_proto`) and fail with a clean internal error rather than a panic. Happy to change the shape if a different indirection is preferred. The new trait methods have default implementations (erroring for the UDF pair, default options for `config_options`) so existing `PhysicalExprEncode` / `PhysicalExprDecode` implementors, including the test stubs, keep compiling. ## What changes are included in this PR? - `datafusion/physical-expr-common/src/physical_expr.rs`: the three ctx/trait helpers above. - `datafusion/physical-expr/src/scalar_function.rs`: `try_to_proto` inside `impl PhysicalExpr for ScalarFunctionExpr` (feature-gated `proto`), `try_from_proto` as an inherent fn, plus a `proto_tests` module at the end of the file. - `datafusion/proto/src/physical_plan/to_proto.rs`: `ConverterEncoder::encode_udf`; the `ScalarFunctionExpr` downcast arm is deleted. - `datafusion/proto/src/physical_plan/from_proto.rs`: `ConverterDecoder::decode_udf` / `config_options`; the inline `ExprType::ScalarUdf` arm now dispatches to `ScalarFunctionExpr::try_from_proto`. - `datafusion/physical-expr/src/proto_test_util.rs`: `StubEncoder` gets an `encode_udf` returning `Ok(None)` (a codec that writes nothing). The wire format is unchanged: same `PhysicalScalarUdfNode` fields, `expr_id: None`, `fun_definition` only set when the codec wrote bytes, `return_field_name` from the stored return field (identical to the old `return_field(&Schema::empty())?.name()` since `ScalarFunctionExpr::return_field` returns the stored field). ## Are these changes tested? - New direct hook tests in `scalar_function.rs::proto_tests`: encode happy path (field-by-field node assertion), decode happy path, wrong `ExprType` variant, missing `return_type`, child encode/decode error propagation, and a `decode_udf`-returns-wrong-type case for the downcast guard. - `cargo test -p datafusion-proto --test proto_integration` passes - with both central arms deleted, the roundtrip coverage for scalar UDFs can only pass through the new hooks. - `cargo fmt --all` and `cargo clippy --all-targets --all-features -- -D warnings` are clean. ## Are there any user-facing changes? No changes to the wire format or public behavior. `PhysicalExprEncode` / `PhysicalExprDecode` gain defaulted methods (non-breaking); `PhysicalExprEncodeCtx` / `PhysicalExprDecodeCtx` gain new helper methods. -- 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]
