geoHeil opened a new pull request, #27104: URL: https://github.com/apache/datafusion/pull/27104
## Which issue does this PR close? - Closes #26998. ## Rationale for this change `datafusion-proto` is built without `avro` by default. In that build, decoding a physical `AvroScan` node panics instead of returning an error. Two logical `ListingScan` arms do the same: the Avro format without `avro`, and the Parquet format without `parquet`. A process that decodes plans it did not encode, such as a worker receiving plan stages from peers, can be stopped by a few bytes of input. Under `panic = "abort"` the whole process stops. ## What changes are included in this PR? The three `panic!` calls now use `not_impl_err!`, which the physical `ParquetScan` arm already does in the same situation: - the physical `AvroScan` arm in `physical_plan/mod.rs` - the logical `ListingScan` Avro and Parquet arms in `logical_plan/mod.rs` ## What is the testing strategy for this PR? Two tests, each with one arm per feature setting: - `avro_scan_decode_returns_error` (`tests/cases/plans/sources.rs`) decodes an empty `AvroScan` node. With `avro` it expects the existing missing-`base_conf` error. Without `avro` it expects the not-implemented error. - `listing_scan_avro_format_decode` (`tests/cases/roundtrip_logical_plan.rs`) switches an encoded `ListingScan` to the Avro format and decodes it. With `avro` it decodes. Without `avro` it expects the not-implemented error. The workspace tests here run with `avro` on, so this repo's CI runs only the `avro` arms. A fork-only run executes both tests with default features (no `avro`), once on this branch and once with the two source files restored to `main`. Without the fix both tests panic. With the fix both pass: https://github.com/geoHeil/datafusion/actions/runs/41818796008 Full CI on this branch in the fork: https://github.com/geoHeil/datafusion/actions/runs/41816985641 Unknowns: - The Parquet-without-`parquet` logical arm has no test. Running it needs the proto tests built without `parquet`, which this repo does not do. CI's `no-default` `cargo check` of `datafusion-proto` only shows that it compiles. ## Are there any user-facing changes? No API change. These decodes now return `Err(DataFusionError::NotImplemented)` 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]
