geoHeil opened a new pull request, #27105: URL: https://github.com/apache/datafusion/pull/27105
## Which issue does this PR close? - Closes #26999. ## Rationale for this change Decoding a physical plan panics on two optional proto fields that a peer can leave unset: - an `Unnest` node whose `options.recursions` entry has no `input_column` or `output_column`: `called Option::unwrap() on a None value` - an `Explain` node whose `stringified_plans` entry has no `plan_type`, or no `plan_type_enum` inside it: `Cannot create protobuf::StringifiedPlan from ...` The same decoders already return an error for a missing `options` or `schema`. A process that decodes plans from peers is stopped by such a node, and under `panic = "abort"` the whole process stops. ## What changes are included in this PR? - `UnnestExec::try_from_proto`: each recursion's columns are checked with `ok_or_else`. A missing one returns an internal error naming the field. - `ExplainExec::try_from_proto`: the private `stringified_plan_from_proto` now returns `Result`, and a missing `plan_type` returns an internal error naming the field. Not included: the logical decoders have the same panics, in the `From<&protobuf::UnnestOptions>` and `From<&protobuf::StringifiedPlan>` impls in `datafusion-proto-models`. Fixing those means changing infallible public `From` impls to `TryFrom`, an API change, so I left them for a follow-up. ## What is the testing strategy for this PR? Two tests in `datafusion/proto/tests/cases/plans/misc.rs`. Each encodes a valid plan, clears the field in the proto, decodes it, and checks that the error names the field: - `decode_explain_exec_node_without_plan_type` covers both an unset `plan_type` and an empty `PlanType`. - `decode_unnest_exec_node_without_recursion_columns` covers both `input_column` and `output_column`. A fork-only run executes both tests once on this branch and once with `unnest.rs` and `explain.rs` restored to `main`. Without the fix both panic. With the fix both pass: https://github.com/geoHeil/datafusion/actions/runs/41818851454 Full CI on this branch in the fork: https://github.com/geoHeil/datafusion/actions/runs/41817460416 ## Are there any user-facing changes? No API change. These decodes now return `Err(DataFusionError::Internal)` where they used to panic. AI disclosure: the change, the tests and this description were drafted with an AI coding assistant (Claude Code). 🤖 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]
