adriangb opened a new pull request, #26353: URL: https://github.com/apache/datafusion/pull/26353
## Which issue does this PR close? - Follow-up to https://github.com/apache/datafusion/pull/24631. No separate issue. ## Rationale for this change A session carries one `ProtoDecoderRegistry`, as a `SessionConfig` extension. Today the only way to attach it is `SessionConfig::with_extension`, which **replaces** the extension. If two libraries each register extension decoders, the second one silently discards the decoders of the first one. The failure shows only later, at decode time, as "No decoder is registered". We found this when we moved datafusion-distributed to the registry API. Distributed registers its plans in the worker `SessionConfig`, then gives the config to the user's session builder. A user who attaches their own registry there removes the distributed decoders. ```rust // Before: library B discards library A's decoders config = config.with_extension(Arc::new(registry_a)); config = config.with_extension(Arc::new(registry_b)); ``` ## What changes are included in this PR? A new generic method, `SessionConfig::update_extension`. It changes an extension in place, and creates it with `Default` if it is not there. If another config shares the extension, it is cloned first (`Arc::make_mut`), so that config does not change. ```rust // After: each library adds to the same registry, in any order config.update_extension(|registry: &mut ProtoDecoderRegistry| { register_execution_plan::<MyExec>(registry) })?; ``` The method is generic because `datafusion-execution` does not depend on `datafusion-proto-models`, where the registry is defined. It is useful for any extension that more than one part of an application writes to. The docs for `ProtoDecoderRegistry` and `register_execution_plan`, and the unreleased 56.0.0 upgrade guide, now recommend `update_extension` and say not to use `with_extension` for the registry. ## What is the testing strategy for this PR? - `proto::registry::tests::libraries_add_to_one_session_registry`: two "libraries" register different plans through `update_extension`. Both names stay registered. A config cloned between the two calls keeps only the first name. - A doc test on `update_extension`. ## Are there any user-facing changes? Yes, one new public method on `SessionConfig`. It is additive and does not break anything. No behavior changes for existing callers of `with_extension`. 🤖 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]
