adriangb opened a new pull request, #26565: URL: https://github.com/apache/datafusion/pull/26565
## Which issue does this PR close? - Follow-up to https://github.com/apache/datafusion/pull/24631 and https://github.com/apache/datafusion/pull/24628. No separate issue. ## Rationale for this change To register an extension type, a library must import a free function for each kind, in addition to the trait it already implements: ```rust // Before use datafusion::physical_plan::proto::{ ExtensionExprFromProto, ExtensionPlanFromProto, ProtoDecoderRegistry, register_execution_plan, register_physical_expr, }; register_execution_plan::<MyExec>(&mut registry)?; register_physical_expr::<MyExpr>(&mut registry)?; ``` ```rust // After: the traits are already imported to implement them MyExec::register(&mut registry)?; MyExpr::register(&mut registry)?; ``` ## What changes are included in this PR? - `ExtensionPlanFromProto` and `ExtensionExprFromProto` get a provided method, `fn register(registry: &mut ProtoDecoderRegistry) -> Result<()>`. It has the same behavior as the free functions: registering one type twice is a no-op, and a name collision or an empty name is an error. - `register_execution_plan` and `register_physical_expr` are removed. They were added for 56.0.0, which is not released, so there is nothing to deprecate. - All callers, the example, and the 56.0.0 upgrade guide use the new method. The method cannot be an inherent method on `ProtoDecoderRegistry`: that type is in `datafusion-proto-models`, which cannot name `ExecutionPlan` or `PhysicalExpr`. A built-in still cannot be registered. It does not implement the trait, so `FilterExec::register` and `Column::register` do not exist. The `compile_fail` doctests that check this moved to the new methods. ## What is the testing strategy for this PR? No behavior changes. The existing tests now call the method: - `proto::registry` unit tests in `datafusion-physical-plan` (5 pass). - `expr_registry` and `extensions` in `datafusion-proto` (20 pass). - The `compile_fail` doctests on `ExtensionPlanFromProto::register` and `ExtensionExprFromProto::register`, and the `extension_plan_registry` example. ## Are there any user-facing changes? Yes, to API that is not released yet: `register_execution_plan::<T>(&mut registry)` is now `T::register(&mut registry)`, and the same for `register_physical_expr`. The 56.0.0 upgrade guide is updated. 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- 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]
