adriangb opened a new issue, #24197:
URL: https://github.com/apache/datafusion/issues/24197
> Filed with AI assistance (Claude Code); verified against `main`.
### Describe the bug
Decoding a physical plan that contains a `ParquetScan` or `AvroScan` node
**panics** when `datafusion-proto` was built without the corresponding feature,
instead of returning a `DataFusionError`:
```rust
PhysicalPlanType::ParquetScan(_) => {
#[cfg(feature = "parquet")]
{
ParquetSource::try_from_proto(self.node(), &decode_ctx)
}
#[cfg(not(feature = "parquet"))]
panic!(
"Unable to process a Parquet PhysicalPlan when `parquet` feature is
not enabled"
)
}
```
The bytes being decoded here come from a peer — that is the entire point of
`datafusion-proto`. A distributed executor built without the `parquet` feature
will abort the process when a scheduler sends it a Parquet scan, rather than
failing that one plan and staying up.
This is pre-existing behaviour that was carried over verbatim by the
proto-hook migration (#24169 for Parquet, #24190 for Avro), so it is not a
regression — just something worth cleaning up now that both arms have the same
shape.
### Expected behavior
Return an error. `not_impl_err!` is already the convention elsewhere in the
same file for feature-gated paths, e.g.:
```rust
not_impl_err!("ParquetSink requires the `parquet` feature")
```
Both scan arms should follow it:
```rust
#[cfg(not(feature = "parquet"))]
not_impl_err!("Unable to process a Parquet PhysicalPlan when the `parquet`
feature is not enabled")
```
### Additional context
Sites to fix in `datafusion/proto/src/physical_plan/mod.rs`:
* the `PhysicalPlanType::ParquetScan` decode arm
* the `PhysicalPlanType::AvroScan` decode arm
* the same `panic!` inside the deprecated `try_into_avro_scan_physical_plan`
compatibility wrapper
While there: the message reads "a Avro PhysicalPlan", which should be "an
Avro PhysicalPlan".
Good first issue — the change is mechanical, but note that the arms need to
keep type-checking under every combination of the `parquet` and `avro`
features, so please verify with `cargo check -p datafusion-proto` (no
features), `--features parquet`, `--features avro`, and `--all-features`.
Noticed while reviewing #24190. Part of the epic in #23494.
--
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]
