timsaucer commented on PR #105:
URL: https://github.com/apache/datafusion-java/pull/105#issuecomment-4691359700
**Fold-in from #104 review — avro feature consistency**
`native-common/src/errors.rs` gates the `DataFusionError::AvroError(_) =>
IoException` arm behind native-common's **own** `avro` feature (`avro =
["datafusion/avro"]`). That feature is independent of whether the unified
`datafusion` in a given build graph actually has `avro` on.
In this PR, `spark/bridge/Cargo.toml` pulls:
```toml
datafusion = { workspace = true } # no avro
datafusion-jni-common = { path = "../../native-common" } # no avro feature
```
That's **correct today** — both are avro-off, so `AvroError` can't be
produced and the missing arm doesn't matter. Each cdylib also statically links
its own native-common copy, so `native`'s avro-on graph doesn't leak in.
The footgun: the moment the bridge's `datafusion` gains `avro` (directly, or
transitively via some dep) **without** also enabling `datafusion-jni-common`'s
`avro` feature, an Avro read error will fall through native-common's `_ =>
DataFusionException` catch-all instead of mapping to `IoException` — a silent
misclassification, no compile error.
To fold in here (or before this lands):
- Keep the invariant explicit: a consumer must enable
`datafusion-jni-common/avro` **iff** its `datafusion` has `avro`.
- Consider a compile-time guard in native-common (e.g. `compile_error!` when
`datafusion/avro` is detected on but native-common's `avro` is off) so the two
can't drift silently.
Ref: #104 (1/6) review.
--
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]
