Skip to content

Commit adf2a8b

Browse files
jackwener2010YOUY01
authored andcommitted
* revert: from_plan keep same schema Project in apache#6595 * revert: from_plan keep same schema Agg/Window in apache#6820 * revert type coercion * add comment
1 parent cbb6649 commit adf2a8b

File tree

3 files changed

+38
-25
lines changed

3 files changed

+38
-25
lines changed

datafusion/common/src/dfschema.rs

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -384,12 +384,8 @@ impl DFSchema {
384384
let self_fields = self.fields().iter();
385385
let other_fields = other.fields().iter();
386386
self_fields.zip(other_fields).all(|(f1, f2)| {
387-
// TODO: resolve field when exist alias
388-
// f1.qualifier() == f2.qualifier()
389-
// && f1.name() == f2.name()
390-
// column(t1.a) field is "t1"."a"
391-
// column(x) as t1.a field is ""."t1.a"
392-
f1.qualified_name() == f2.qualified_name()
387+
f1.qualifier() == f2.qualifier()
388+
&& f1.name() == f2.name()
393389
&& Self::datatype_is_semantically_equal(f1.data_type(), f2.data_type())
394390
})
395391
}

datafusion/expr/src/utils.rs

Lines changed: 27 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -724,16 +724,22 @@ where
724724
/// // create new plan using rewritten_exprs in same position
725725
/// let new_plan = from_plan(&plan, rewritten_exprs, new_inputs);
726726
/// ```
727+
///
728+
/// Notice: sometimes [from_plan] will use schema of original plan, it don't change schema!
729+
/// Such as `Projection/Aggregate/Window`
727730
pub fn from_plan(
728731
plan: &LogicalPlan,
729732
expr: &[Expr],
730733
inputs: &[LogicalPlan],
731734
) -> Result<LogicalPlan> {
732735
match plan {
733-
LogicalPlan::Projection(_) => Ok(LogicalPlan::Projection(Projection::try_new(
734-
expr.to_vec(),
735-
Arc::new(inputs[0].clone()),
736-
)?)),
736+
LogicalPlan::Projection(Projection { schema, .. }) => {
737+
Ok(LogicalPlan::Projection(Projection::try_new_with_schema(
738+
expr.to_vec(),
739+
Arc::new(inputs[0].clone()),
740+
schema.clone(),
741+
)?))
742+
}
737743
LogicalPlan::Dml(DmlStatement {
738744
table_name,
739745
table_schema,
@@ -818,19 +824,23 @@ pub fn from_plan(
818824
input: Arc::new(inputs[0].clone()),
819825
})),
820826
},
821-
LogicalPlan::Window(Window { window_expr, .. }) => {
822-
Ok(LogicalPlan::Window(Window::try_new(
823-
expr[0..window_expr.len()].to_vec(),
824-
Arc::new(inputs[0].clone()),
825-
)?))
826-
}
827-
LogicalPlan::Aggregate(Aggregate { group_expr, .. }) => {
828-
Ok(LogicalPlan::Aggregate(Aggregate::try_new(
829-
Arc::new(inputs[0].clone()),
830-
expr[0..group_expr.len()].to_vec(),
831-
expr[group_expr.len()..].to_vec(),
832-
)?))
833-
}
827+
LogicalPlan::Window(Window {
828+
window_expr,
829+
schema,
830+
..
831+
}) => Ok(LogicalPlan::Window(Window {
832+
input: Arc::new(inputs[0].clone()),
833+
window_expr: expr[0..window_expr.len()].to_vec(),
834+
schema: schema.clone(),
835+
})),
836+
LogicalPlan::Aggregate(Aggregate {
837+
group_expr, schema, ..
838+
}) => Ok(LogicalPlan::Aggregate(Aggregate::try_new_with_schema(
839+
Arc::new(inputs[0].clone()),
840+
expr[0..group_expr.len()].to_vec(),
841+
expr[group_expr.len()..].to_vec(),
842+
schema.clone(),
843+
)?)),
834844
LogicalPlan::Sort(SortPlan { fetch, .. }) => Ok(LogicalPlan::Sort(SortPlan {
835845
expr: expr.to_vec(),
836846
input: Arc::new(inputs[0].clone()),

datafusion/optimizer/src/analyzer/type_coercion.rs

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ use datafusion_expr::utils::from_plan;
4343
use datafusion_expr::{
4444
is_false, is_not_false, is_not_true, is_not_unknown, is_true, is_unknown,
4545
type_coercion, AggregateFunction, BuiltinScalarFunction, Expr, LogicalPlan, Operator,
46-
WindowFrame, WindowFrameBound, WindowFrameUnits,
46+
Projection, WindowFrame, WindowFrameBound, WindowFrameUnits,
4747
};
4848
use datafusion_expr::{ExprSchemable, Signature};
4949

@@ -109,7 +109,14 @@ fn analyze_internal(
109109
})
110110
.collect::<Result<Vec<_>>>()?;
111111

112-
from_plan(plan, &new_expr, &new_inputs)
112+
// TODO: from_plan can't change the schema, so we need to do this here
113+
match &plan {
114+
LogicalPlan::Projection(_) => Ok(LogicalPlan::Projection(Projection::try_new(
115+
new_expr,
116+
Arc::new(new_inputs[0].clone()),
117+
)?)),
118+
_ => from_plan(plan, &new_expr, &new_inputs),
119+
}
113120
}
114121

115122
pub(crate) struct TypeCoercionRewriter {

0 commit comments

Comments
 (0)