Skip to content

Commit 2bba020

Browse files
committed
physical-plan: coerce UNION/INTERLEAVE schema mismatches at plan time
Follow-up to #23861 (issue #15394). Moves the schema re-stamping for nullability-mismatched UNION ALL / INTERLEAVE inputs out of `execute()` and into plan construction, via a new `CoerceSchemaExec` node inserted by `UnionExec::try_new`/`InterleaveExec::try_new` whenever a child's own schema disagrees with the computed union schema. The node is now visible in `EXPLAIN` output, is transparent for statistics/pushdown/proto purposes, and adds no measurable overhead versus inline re-stamping.
1 parent 426b351 commit 2bba020

4 files changed

Lines changed: 323 additions & 56 deletions

File tree

datafusion/core/tests/dataframe/mod.rs

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7075,7 +7075,8 @@ async fn test_copy_to_preserves_order() -> Result<()> {
70757075
DataSinkExec: sink=CsvSink(file_groups=[])
70767076
SortExec: expr=[column1@0 DESC], preserve_partitioning=[false]
70777077
DataSourceExec: partitions=1, partition_sizes=[1]
7078-
DataSourceExec: partitions=1, partition_sizes=[1]
7078+
CoerceSchemaExec
7079+
DataSourceExec: partitions=1, partition_sizes=[1]
70797080
"
70807081
);
70817082
Ok(())

datafusion/physical-plan/src/union.rs

Lines changed: 279 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -67,20 +67,14 @@ use tokio::macros::support::thread_rng_n;
6767
/// Wraps a child stream so that every batch it yields is re-stamped with
6868
/// `schema` instead of the child's own schema.
6969
///
70-
/// This is used by both [`UnionExec`] and [`InterleaveExec`] when a child's
71-
/// output schema disagrees with the operator's declared output schema --
72-
/// in practice this only happens for nullability (the declared schema is
73-
/// nullable wherever *any* input's field is, but casts are only inserted
74-
/// between inputs when the *type* differs, not when only nullability
75-
/// does). For [`UnionExec`], [`UnionExec::try_new`] guarantees this: it
76-
/// calls `calculate_union`, which rejects any input whose field data types
77-
/// don't match the computed union schema. [`InterleaveExec::try_new`] does
78-
/// not repeat that check -- its inputs are only ever produced by the
79-
/// optimizer rewriting an already-validated `UnionExec`, whose children's
80-
/// types are therefore already known to agree -- but if this wrapper ever
81-
/// did see a genuine data type mismatch (e.g. from a hand-built
82-
/// `InterleaveExec`), `RecordBatch::try_new_with_options` below reports it
83-
/// as an error rather than silently yielding a corrupt batch.
70+
/// Used by [`CoerceSchemaExec`], which [`UnionExec::try_new`] and
71+
/// [`InterleaveExec::try_new`] insert above any child whose output schema
72+
/// disagrees with the operator's declared output schema -- in practice this
73+
/// only happens for nullability (the declared schema is nullable wherever
74+
/// *any* input's field is, but casts are only inserted between inputs when
75+
/// the *type* differs, not when only nullability does). If this wrapper
76+
/// ever did see a genuine data type mismatch, `RecordBatch::try_new_with_options`
77+
/// below reports it as an error rather than silently yielding a corrupt batch.
8478
struct SchemaConformingStream {
8579
schema: SchemaRef,
8680
inner: SendableRecordBatchStream,
@@ -141,6 +135,179 @@ fn conform_stream_schema(
141135
}
142136
}
143137

138+
/// Coerces a single child's declared output schema to `schema`, re-stamping
139+
/// every batch it produces to match. [`UnionExec::try_new`] and
140+
/// [`InterleaveExec::try_new`] insert this above any child whose own schema
141+
/// disagrees with the computed union schema, so that the coercion is visible
142+
/// in the plan tree (e.g. in `EXPLAIN`) instead of happening invisibly inside
143+
/// the union operator's own `execute()`.
144+
///
145+
/// A genuine data type mismatch (as opposed to a nullability-only one) is
146+
/// rejected eagerly, at construction time, by `EquivalenceProperties::
147+
/// with_new_schema` below -- unlike the old purely-runtime approach, a
148+
/// hand-built union/interleave with mismatched child types now fails in
149+
/// `try_new` rather than in `execute()`.
150+
///
151+
/// This node is a strict 1:1, order-preserving passthrough of `input` (it
152+
/// only ever changes a batch's declared schema, never its rows), so every
153+
/// `ExecutionPlan` method below that isn't about the schema itself just
154+
/// delegates straight to `input`.
155+
///
156+
/// See <https://github.com/apache/datafusion/issues/15394>.
157+
#[derive(Debug)]
158+
struct CoerceSchemaExec {
159+
input: Arc<dyn ExecutionPlan>,
160+
cache: Arc<PlanProperties>,
161+
metrics: ExecutionPlanMetricsSet,
162+
}
163+
164+
impl CoerceSchemaExec {
165+
/// Wraps `input` in a [`CoerceSchemaExec`] targeting `schema` if its own
166+
/// schema disagrees with `schema`, otherwise returns it unchanged.
167+
fn wrap_if_needed(
168+
input: Arc<dyn ExecutionPlan>,
169+
schema: &SchemaRef,
170+
) -> Result<Arc<dyn ExecutionPlan>> {
171+
if &input.schema() == schema {
172+
Ok(input)
173+
} else {
174+
Ok(Arc::new(Self::new(input, schema)?))
175+
}
176+
}
177+
178+
fn new(input: Arc<dyn ExecutionPlan>, schema: &SchemaRef) -> Result<Self> {
179+
let eq_properties = input
180+
.equivalence_properties()
181+
.clone()
182+
.with_new_schema(Arc::clone(schema))?;
183+
let output_partitioning = input.output_partitioning().clone();
184+
let cache = PlanProperties::new(
185+
eq_properties,
186+
output_partitioning,
187+
emission_type_from_children(std::iter::once(&input)),
188+
boundedness_from_children(std::iter::once(&input)),
189+
);
190+
Ok(Self {
191+
input,
192+
cache: Arc::new(cache),
193+
metrics: ExecutionPlanMetricsSet::new(),
194+
})
195+
}
196+
}
197+
198+
impl DisplayAs for CoerceSchemaExec {
199+
fn fmt_as(
200+
&self,
201+
t: DisplayFormatType,
202+
f: &mut std::fmt::Formatter,
203+
) -> std::fmt::Result {
204+
match t {
205+
DisplayFormatType::Default | DisplayFormatType::Verbose => {
206+
write!(f, "CoerceSchemaExec")
207+
}
208+
DisplayFormatType::TreeRender => Ok(()),
209+
}
210+
}
211+
}
212+
213+
impl ExecutionPlan for CoerceSchemaExec {
214+
fn name(&self) -> &'static str {
215+
"CoerceSchemaExec"
216+
}
217+
218+
fn properties(&self) -> &Arc<PlanProperties> {
219+
&self.cache
220+
}
221+
222+
fn children(&self) -> Vec<&Arc<dyn ExecutionPlan>> {
223+
vec![&self.input]
224+
}
225+
226+
fn maintains_input_order(&self) -> Vec<bool> {
227+
vec![true]
228+
}
229+
230+
// A 1:1 passthrough never combines partitions, so re-deriving the cache
231+
// (rather than collapsing back to the raw child via `wrap_if_needed`) is
232+
// always safe here -- it just keeps this node from vanishing and
233+
// changing arity out from under a caller mid-rewrite.
234+
fn with_new_children(
235+
self: Arc<Self>,
236+
mut children: Vec<Arc<dyn ExecutionPlan>>,
237+
) -> Result<Arc<dyn ExecutionPlan>> {
238+
assert_or_internal_err!(
239+
children.len() == 1,
240+
"CoerceSchemaExec expects exactly one child"
241+
);
242+
Ok(Arc::new(Self::new(children.remove(0), &self.schema())?))
243+
}
244+
245+
fn execute(
246+
&self,
247+
partition: usize,
248+
context: Arc<TaskContext>,
249+
) -> Result<SendableRecordBatchStream> {
250+
let baseline_metrics = BaselineMetrics::new(&self.metrics, partition);
251+
let stream = self.input.execute(partition, context)?;
252+
let stream = conform_stream_schema(self.schema(), stream);
253+
Ok(Box::pin(ObservedStream::new(
254+
stream,
255+
baseline_metrics,
256+
None,
257+
)))
258+
}
259+
260+
fn metrics(&self) -> Option<MetricsSet> {
261+
Some(self.metrics.clone_inner())
262+
}
263+
264+
fn benefits_from_input_partitioning(&self) -> Vec<bool> {
265+
vec![false]
266+
}
267+
268+
fn supports_limit_pushdown(&self) -> bool {
269+
true
270+
}
271+
272+
fn cardinality_effect(&self) -> CardinalityEffect {
273+
CardinalityEffect::Equal
274+
}
275+
276+
fn child_stats_requests(&self, partition: Option<usize>) -> Vec<ChildStats> {
277+
vec![ChildStats::At(partition)]
278+
}
279+
280+
fn statistics_from_inputs(
281+
&self,
282+
input_stats: &[Arc<Statistics>],
283+
_args: &StatisticsArgs,
284+
) -> Result<Arc<Statistics>> {
285+
Ok(Arc::clone(&input_stats[0]))
286+
}
287+
288+
fn gather_filters_for_pushdown(
289+
&self,
290+
_phase: FilterPushdownPhase,
291+
parent_filters: Vec<Arc<dyn PhysicalExpr>>,
292+
_config: &ConfigOptions,
293+
) -> Result<FilterDescription> {
294+
FilterDescription::from_children(parent_filters, &self.children())
295+
}
296+
297+
#[cfg(feature = "proto")]
298+
fn try_to_proto(
299+
&self,
300+
ctx: &crate::proto::ExecutionPlanEncodeCtx<'_>,
301+
) -> Result<Option<datafusion_proto_models::protobuf::PhysicalPlanNode>> {
302+
// No dedicated protobuf variant: this node is fully determined by its
303+
// child and the parent union/interleave's declared schema, so it's
304+
// serialized as if it weren't there. `UnionExec`/`InterleaveExec::
305+
// try_from_proto` both go through `try_new`, which re-inserts the
306+
// wrapper via `wrap_if_needed` on decode.
307+
Ok(Some(ctx.encode_child(&self.input)?))
308+
}
309+
}
310+
144311
/// `UnionExec`: `UNION ALL` execution plan.
145312
///
146313
/// `UnionExec` combines multiple inputs with the same schema by
@@ -208,6 +375,10 @@ impl UnionExec {
208375
// The schema of the inputs and the union schema is consistent when:
209376
// - They have the same number of fields, and
210377
// - Their fields have same types at the same indices.
378+
let inputs = inputs
379+
.into_iter()
380+
.map(|input| CoerceSchemaExec::wrap_if_needed(input, &schema))
381+
.collect::<Result<Vec<_>>>()?;
211382
let cache = Self::compute_properties(&inputs, schema)?;
212383
Ok(Arc::new(UnionExec {
213384
inputs,
@@ -372,7 +543,6 @@ impl ExecutionPlan for UnionExec {
372543
if partition < input.output_partitioning().partition_count() {
373544
let stream = input.execute(partition, context)?;
374545
debug!("Found a Union partition to execute");
375-
let stream = conform_stream_schema(self.schema(), stream);
376546
return Ok(Box::pin(ObservedStream::new(
377547
stream,
378548
baseline_metrics,
@@ -642,7 +812,12 @@ impl InterleaveExec {
642812
can_interleave(inputs.iter()),
643813
"Not all InterleaveExec children have a consistent hash or range partitioning"
644814
);
645-
let cache = Self::compute_properties(&inputs)?;
815+
let schema = union_schema(&inputs)?;
816+
let inputs = inputs
817+
.into_iter()
818+
.map(|input| CoerceSchemaExec::wrap_if_needed(input, &schema))
819+
.collect::<Result<Vec<_>>>()?;
820+
let cache = Self::compute_properties(&inputs, schema)?;
646821
Ok(InterleaveExec {
647822
inputs,
648823
metrics: ExecutionPlanMetricsSet::new(),
@@ -656,8 +831,10 @@ impl InterleaveExec {
656831
}
657832

658833
/// This function creates the cache object that stores the plan properties such as schema, equivalence properties, ordering, partitioning, etc.
659-
fn compute_properties(inputs: &[Arc<dyn ExecutionPlan>]) -> Result<PlanProperties> {
660-
let schema = union_schema(inputs)?;
834+
fn compute_properties(
835+
inputs: &[Arc<dyn ExecutionPlan>],
836+
schema: SchemaRef,
837+
) -> Result<PlanProperties> {
661838
let eq_properties = EquivalenceProperties::new(schema);
662839
// Get output partitioning:
663840
let output_partitioning = inputs[0].output_partitioning().clone();
@@ -748,7 +925,7 @@ impl ExecutionPlan for InterleaveExec {
748925
for input in self.inputs.iter() {
749926
if partition < input.output_partitioning().partition_count() {
750927
let stream = input.execute(partition, Arc::clone(&context))?;
751-
input_stream_vec.push(conform_stream_schema(self.schema(), stream));
928+
input_stream_vec.push(stream);
752929
} else {
753930
// Do not find a partition to execute
754931
break;
@@ -1262,6 +1439,89 @@ mod tests {
12621439
Ok(())
12631440
}
12641441

1442+
#[test]
1443+
fn test_union_partition_statistics_with_mismatched_nullability() -> Result<()> {
1444+
// Regression test for the `CoerceSchemaExec` wrapper `UnionExec::try_new`
1445+
// inserts above the non-nullable leg here: before it forwarded
1446+
// `child_stats_requests`/`statistics_from_inputs` to its child, it
1447+
// reported `Statistics::new_unknown`, poisoning the merge into all-`Absent`
1448+
// even though both legs have exact statistics.
1449+
let (_, left, right, expected) = stats_merge_inputs();
1450+
1451+
let non_nullable_schema =
1452+
Schema::new(vec![Field::new("a", DataType::UInt32, false)]);
1453+
let nullable_schema = Schema::new(vec![Field::new("a", DataType::UInt32, true)]);
1454+
1455+
let left: Arc<dyn ExecutionPlan> =
1456+
Arc::new(StatisticsExec::new(left, non_nullable_schema));
1457+
let right: Arc<dyn ExecutionPlan> =
1458+
Arc::new(StatisticsExec::new(right, nullable_schema));
1459+
1460+
let union = UnionExec::try_new(vec![left, right])?;
1461+
let stats =
1462+
StatisticsContext::new().compute(union.as_ref(), &StatisticsArgs::new())?;
1463+
1464+
assert_eq!(stats.as_ref(), &expected);
1465+
Ok(())
1466+
}
1467+
1468+
#[tokio::test]
1469+
async fn test_coerce_schema_exec_execution_plan_methods() -> Result<()> {
1470+
// Most sqllogictest coverage only observes `CoerceSchemaExec` through
1471+
// `EXPLAIN`; exercise its `ExecutionPlan` methods directly here.
1472+
let schema_not_null =
1473+
Arc::new(Schema::new(vec![Field::new("a", DataType::Int32, false)]));
1474+
let batch_not_null = RecordBatch::try_new(
1475+
Arc::clone(&schema_not_null),
1476+
vec![Arc::new(arrow::array::Int32Array::from(vec![1, 2]))],
1477+
)?;
1478+
let input: Arc<dyn ExecutionPlan> = TestMemoryExec::try_new_exec(
1479+
&[vec![batch_not_null]],
1480+
Arc::clone(&schema_not_null),
1481+
None,
1482+
)?;
1483+
1484+
// Matching schema: `wrap_if_needed` is a no-op.
1485+
let unwrapped =
1486+
CoerceSchemaExec::wrap_if_needed(Arc::clone(&input), &schema_not_null)?;
1487+
assert!(Arc::ptr_eq(&unwrapped, &input));
1488+
1489+
// Mismatched nullability: the input gets wrapped.
1490+
let nullable_schema =
1491+
Arc::new(Schema::new(vec![Field::new("a", DataType::Int32, true)]));
1492+
let wrapped =
1493+
CoerceSchemaExec::wrap_if_needed(Arc::clone(&input), &nullable_schema)?;
1494+
assert_eq!(wrapped.name(), "CoerceSchemaExec");
1495+
assert_eq!(&wrapped.schema(), &nullable_schema);
1496+
assert_eq!(wrapped.maintains_input_order(), vec![true]);
1497+
assert_eq!(wrapped.benefits_from_input_partitioning(), vec![false]);
1498+
assert!(wrapped.supports_limit_pushdown());
1499+
assert!(matches!(
1500+
wrapped.cardinality_effect(),
1501+
CardinalityEffect::Equal
1502+
));
1503+
assert!(wrapped.metrics().is_some());
1504+
1505+
wrapped.gather_filters_for_pushdown(
1506+
FilterPushdownPhase::Pre,
1507+
vec![],
1508+
&ConfigOptions::new(),
1509+
)?;
1510+
1511+
// Re-deriving via `with_new_children` keeps targeting the same schema.
1512+
let new_child: Arc<dyn ExecutionPlan> =
1513+
TestMemoryExec::try_new_exec(&[vec![]], Arc::clone(&schema_not_null), None)?;
1514+
let rewrapped = Arc::clone(&wrapped).with_new_children(vec![new_child])?;
1515+
assert_eq!(&rewrapped.schema(), &nullable_schema);
1516+
1517+
let task_ctx = Arc::new(TaskContext::default());
1518+
let batches = collect(wrapped, task_ctx).await?;
1519+
assert_eq!(batches.len(), 1);
1520+
assert_eq!(batches[0].schema(), nullable_schema);
1521+
1522+
Ok(())
1523+
}
1524+
12651525
#[test]
12661526
fn test_interleave_partition_statistics_uses_shared_statistics_merge() -> Result<()> {
12671527
let (schema, left, right, expected) = stats_merge_inputs();

datafusion/sqllogictest/test_files/array_agg.slt

Lines changed: 13 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -534,16 +534,19 @@ physical_plan
534534
03)----RepartitionExec: partitioning=Hash([id@0], 4), input_partitions=5
535535
04)------AggregateExec: mode=Partial, gby=[id@0 as id], aggr=[array_agg(DISTINCT a.foo), sum(DISTINCT Int64(1))], ordering_mode=Sorted
536536
05)--------UnionExec
537-
06)----------ProjectionExec: expr=[1 as id, 2 as foo]
538-
07)------------PlaceholderRowExec
539-
08)----------ProjectionExec: expr=[1 as id, NULL as foo]
540-
09)------------PlaceholderRowExec
541-
10)----------ProjectionExec: expr=[1 as id, NULL as foo]
542-
11)------------PlaceholderRowExec
543-
12)----------ProjectionExec: expr=[1 as id, 3 as foo]
544-
13)------------PlaceholderRowExec
545-
14)----------ProjectionExec: expr=[1 as id, 2 as foo]
546-
15)------------PlaceholderRowExec
537+
06)----------CoerceSchemaExec
538+
07)------------ProjectionExec: expr=[1 as id, 2 as foo]
539+
08)--------------PlaceholderRowExec
540+
09)----------ProjectionExec: expr=[1 as id, NULL as foo]
541+
10)------------PlaceholderRowExec
542+
11)----------ProjectionExec: expr=[1 as id, NULL as foo]
543+
12)------------PlaceholderRowExec
544+
13)----------CoerceSchemaExec
545+
14)------------ProjectionExec: expr=[1 as id, 3 as foo]
546+
15)--------------PlaceholderRowExec
547+
16)----------CoerceSchemaExec
548+
17)------------ProjectionExec: expr=[1 as id, 2 as foo]
549+
18)--------------PlaceholderRowExec
547550

548551
#######
549552
# Unsupported syntax

0 commit comments

Comments
 (0)