patrickswedish commented on PR #24394:
URL: https://github.com/apache/datafusion/pull/24394#issuecomment-5360533395
### Update Summary
Thank you @alamb and @kosiew for the thorough review and guidance!
Following the feedback, we have restructured the fix around the **producer
boundary** rather than wrapping individual downstream consumer operators:
1. **Producer Normalization in `MemoryStream`
(`datafusion/physical-plan/src/memory.rs`)**:
- When `MemTable` accepts batches whose nested schemas are stricter than
the table's declared schema (via `Schema::contains`), `MemoryStream::poll_next`
now normalizes emitted batches using
`datafusion_common::nested_struct::adapt_batch_to_schema` whenever the batch
schema differs from the stream's advertised schema and
`self.schema.contains(batch.schema())`.
- Ensures every `RecordBatch` emitted by `MemoryStream` conforms to
`stream.schema()` before downstream operators consume it.
2. **Reverted `AggregateExec` Wrapper**:
- Removed `AdaptedInputRecordBatchStream` and restored
`datafusion/physical-plan/src/aggregates/` to clean upstream state.
3. **Narrow Schema-Conformance Support for Unions
(`datafusion/common/src/nested_struct.rs`)**:
- Retained recursive child adaptation and exact type-ID set matching for
Arrow `UnionArray` (sparse and dense) in `adapt_batch_to_schema` without
expanding general DataFusion SQL CAST behavior.
4. **Test Suite Refactoring**:
- Streamlined unit tests in `nested_struct.rs` using helper builders to
remove repeated boilerplate while asserting exact values (`10`, `"b"`, `30`),
type IDs, and dense offsets.
- Added invariant unit tests in `physical-plan/src/memory.rs` validating
emitted batch schema conformance and projection handling.
- Cleaned up `nested_nullability.rs` to focus on end-to-end bug report
#24069 reproducer regressions (standard, distinct, and spilling).
5. **Validation Battery**:
- `cargo fmt --check` passed across all workspace packages.
- `cargo clippy --workspace ... -- -D warnings` passed with 0 warnings.
- `cargo test -p datafusion-common --lib` passed (562 passed; 0 failed).
- `cargo test -p datafusion-physical-plan --lib` passed (1770 passed; 0
failed).
- `cargo test -p datafusion --test core_integration --
nested_nullability` passed (5 passed; 0 failed).
- Rebased cleanly onto the latest `upstream/main` as 1 atomic commit.
--
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]
