Skip to content

Commit cf6c647

Browse files
committed
revert change recompute_schema for LogicalPlan::Values
1 parent a2e454b commit cf6c647

File tree

2 files changed

+28
-12
lines changed

2 files changed

+28
-12
lines changed

datafusion/core/tests/execution/logical_plan.rs

Lines changed: 18 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -39,18 +39,27 @@ use std::sync::Arc;
3939
#[tokio::test]
4040
async fn count_only_nulls() -> Result<()> {
4141
// Input: VALUES (NULL), (NULL), (NULL) AS _(col)
42-
let input = LogicalPlanBuilder::values(vec![
43-
vec![Expr::Literal(ScalarValue::Null, None)],
44-
vec![Expr::Literal(ScalarValue::Null, None)],
45-
vec![Expr::Literal(ScalarValue::Null, None)],
46-
])?
47-
.project(vec![col("column1").alias("col")])?
48-
.build()?;
49-
let input_col_ref = col("col");
42+
let input_schema = Arc::new(DFSchema::from_unqualified_fields(
43+
vec![Field::new("col", DataType::Null, true)].into(),
44+
HashMap::new(),
45+
)?);
46+
let input = Arc::new(LogicalPlan::Values(Values {
47+
schema: input_schema,
48+
values: vec![
49+
vec![Expr::Literal(ScalarValue::Null, None)],
50+
vec![Expr::Literal(ScalarValue::Null, None)],
51+
vec![Expr::Literal(ScalarValue::Null, None)],
52+
],
53+
}));
54+
let input_col_ref = Expr::Column(Column {
55+
relation: None,
56+
name: "col".to_string(),
57+
spans: Spans::new(),
58+
});
5059

5160
// Aggregation: count(col) AS count
5261
let aggregate = LogicalPlan::Aggregate(Aggregate::try_new(
53-
input.into(),
62+
input,
5463
vec![],
5564
vec![Expr::AggregateFunction(AggregateFunction {
5665
func: Arc::new(AggregateUDF::new_from_impl(Count::new())),

datafusion/expr/src/logical_plan/plan.rs

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -632,8 +632,9 @@ impl LogicalPlan {
632632
}) => Projection::try_new(expr, input).map(LogicalPlan::Projection),
633633
LogicalPlan::Dml(_) => Ok(self),
634634
LogicalPlan::Copy(_) => Ok(self),
635-
LogicalPlan::Values(Values { schema: _, values }) => {
636-
LogicalPlanBuilder::values(values)?.build()
635+
LogicalPlan::Values(Values { schema, values }) => {
636+
// todo it isn't clear why the schema is not recomputed here
637+
Ok(LogicalPlan::Values(Values { schema, values }))
637638
}
638639
LogicalPlan::Filter(Filter { predicate, input }) => {
639640
Filter::try_new(predicate, input).map(LogicalPlan::Filter)
@@ -1473,7 +1474,13 @@ impl LogicalPlan {
14731474
})?
14741475
// always recompute the schema to ensure the changed in the schema's field should be
14751476
// poplulated to the plan's parent
1476-
.map_data(|plan| plan.recompute_schema())
1477+
.map_data(|plan| plan.recompute_schema())?
1478+
.map_data(|plan| match plan {
1479+
LogicalPlan::Values(Values { values, schema: _ }) => {
1480+
LogicalPlanBuilder::values(values)?.build()
1481+
}
1482+
_ => Ok(plan),
1483+
})
14771484
})
14781485
.map(|res| res.data)
14791486
}

0 commit comments

Comments
 (0)