`optimize_projections` fail due to difference schemas

[haohuaijin]

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

[alamb]

cc @mustafasrepo

[mustafasrepo]

cc @mustafasrepo

Thanks, I will look to this issue.

[mustafasrepo]

I looked at the issue. It seems that before the optimize projections rule plan is

Projection: z + Int64(1), t.y
  Projection: t.x + Int64(1) AS z, t.y
    Filter: t.y > Int64(1)
      TableScan: t

after rule it becomes

Projection: t.x + Int64(1) + Int64(1), t.y
  Filter: t.y > Int64(1)
    TableScan: t projection=[x, y]

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_same check fails after optimize_projections rule. I think we can proceed with two approaches.

1. We can relax same schema check for some rules (if it is possible to produce different schemas at the output.) As in the fn schema_check(&self) -> bool; flag of PhysicalOptimizerRule.

However, this approach may introduce silent bugs, which can be catched easily in current design.

2. We can add an API fn schema_check_relax(&self) -> bool; which would mean that
   either 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]

By the way, I have opened a mini PR to solve this issue (which implements approach 2).

[haohuaijin]

Thanks for quick fix @mustafasrepo, but I check #8438 and find the output column name change,

❯ create table t(x bigint, y bigint) as values (1,2), (1,3);
0 rows in set. Query took 0.015 seconds.

❯ select z+1, y from (select x+1 as z, y from t) where y > 1;
+---------------------------+---+
| t.x + Int64(1) + Int64(1) | y |
+---------------------------+---+
| 3                         | 2 |
| 3                         | 3 |
+---------------------------+---+
2 rows in set. Query took 0.009 seconds.

what do you think add alias to make schema consistent? I find we before use alias in merge_project to 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]

I will give it a try. We can use aliases as you suggested, to make schema names consistent.

[mustafasrepo]

Thanks @haohuaijin I changed implementation as you suggested. It is much more better. Thanks for the suggestion

[alamb]

. However, their schemas are different.

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