Repository navigation
optimize_projections fail due to difference schemas #8432
Description
Describe the bug
I run the follow sql in main 3ddd5eb , get the follow error
❯ create table t(x bigint, y bigint) as values (1,2), (1,3);
0 rows in set. Query took 0.011 seconds.
❯ select z+1, y from (select x+1 as z, y from t) where y > 1;
Optimizer rule 'optimize_projections' failed
caused by
optimize_projections
caused by
Internal error: Failed due to a difference in schemas, original schema: DFSchema { fields: [DFField { qualifier: None, field: Field { name: "z + Int64(1)", data_type: Int64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} } }, DFField { qualifier: Some(Bare { table: "t" }), field: Field { name: "y", data_type: Int64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} } }], metadata: {}, functional_dependencies: FunctionalDependencies { deps: [] } }, new schema: DFSchema { fields: [DFField { qualifier: None, field: Field { name: "t.x + Int64(1) + Int64(1)", data_type: Int64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} } }, DFField { qualifier: Some(Bare { table: "t" }), field: Field { name: "y", data_type: Int64, nullable: true, dict_id: 0, dict_is_ordered: false, metadata: {} } }], metadata: {}, functional_dependencies: FunctionalDependencies { deps: [] } }.
This was likely caused by a bug in DataFusion's code and we would welcome that you file an bug report in our issue tracker
but it work in datafusion-cli v33
DataFusion CLI v33.0.0
❯ create table t(x bigint, y bigint) as values (1,2), (1,3);
0 rows in set. Query took 0.003 seconds.
❯ select z+1, y from (select x+1 as z, y from t) where y > 1;
+--------------+---+
| z + Int64(1) | y |
+--------------+---+
| 3 | 2 |
| 3 | 3 |
+--------------+---+
2 rows in set. Query took 0.006 seconds.
To Reproduce
No response
Expected behavior
No response
Additional context
looks like it is related to #8340
Activity
alamb commentedon Dec 6, 2023
cc @mustafasrepo
mustafasrepo commentedon Dec 6, 2023
Thanks, I will look to this issue.
mustafasrepo commentedon Dec 6, 2023
I looked at the issue. It seems that before the optimize projections rule plan is
after rule it becomes
I think both of the plans are valid. Also merging projections in the first plan makes sense. However, their schemas are different. Hence,
assert_schema_is_the_samecheck fails afteroptimize_projectionsrule. I think we can proceed with two approaches.fn schema_check(&self) -> bool;flag ofPhysicalOptimizerRule.However, this approach may introduce silent bugs, which can be catched easily in current design.
fn schema_check_relax(&self) -> bool;which would mean thateither we expect to have exactly same schema at the output (with names and types) or we expect to have same schema in relax mode (types are equal). In this way, we can catch some obvious bugs early. Also I cannot think any case, where we will see different types at the output. Hence doing this check for all cases makes sense to me. What do you think?
mustafasrepo commentedon Dec 6, 2023
By the way, I have opened a mini PR to solve this issue (which implements approach 2).
haohuaijin commentedon Dec 6, 2023
Thanks for quick fix @mustafasrepo, but I check #8438 and find the output column name change,
what do you think add
aliasto make schema consistent? I find we before usealiasinmerge_projectto make schema consistent. But I am afraid it maybe make the incorrect schema correct and whether it is fit in current framework.https://github.com/apache/arrow-datafusion/blob/8f48053fc5f6fc3de27b69cd6f229558d8fc8990/datafusion/optimizer/src/merge_projection.rs#L80-L85
mustafasrepo commentedon Dec 6, 2023
I will give it a try. We can use aliases as you suggested, to make schema names consistent.
mustafasrepo commentedon Dec 6, 2023
Thanks @haohuaijin I changed implementation as you suggested. It is much more better. Thanks for the suggestion
alamb commentedon Dec 6, 2023
FWIW I think we can always preserve the invariant that the schemas are the same via the alias suggestion @haohuaijin suggests
Thank you for #8438