dariocurr commented on PR #23861: URL: https://github.com/apache/datafusion/pull/23861#issuecomment-5164501246
Ran an additional independent review pass (Opus model) over 131796dc8 before considering the InterleaveExec fix settled. It caught three loose ends, fixed in 4af8735a2: - `SchemaConformingStream`'s doc comment claimed `InterleaveExec::try_new` re-validates child field types the same way `UnionExec::try_new` does via `calculate_union`. It doesn't -- `InterleaveExec::compute_properties` builds fresh `EquivalenceProperties` without that check. Comment now states the actual invariant (its inputs come from an already-validated `UnionExec` via the optimizer) and notes a genuine mismatch would now surface as an explicit error rather than a silently corrupt batch. - `test_interleave_conforms_batch_schema` and `union_all_widening_cast_also_fixes_nullable` iterated collected batches without asserting the batch list was non-empty, so both could have passed vacuously. - `union_nullable_spill.rs`'s doc comment asserted `UnionExec` returns child streams "without schema coercion" -- no longer true after this fix. Updated to explain the test now guards the SpillManager-level fix (#21292) specifically, since `UnionExec` itself no longer produces mismatched-nullability batches. All existing + new tests still green, `cargo fmt`/`clippy -D warnings` clean. -- 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]
