Revert "Fix/support duplicate column names apache#6543 (apache#21126)"

This reverts commit aadae6b.

Author: mbutrovich

datafusion/sql/src/select.rs

@@ -23,9 +23,8 @@ use crate::planner::{ContextProvider, PlannerContext, SqlToRel};
 use crate::query::to_order_by_exprs_with_select;
 use crate::utils::{
     CheckColumnsMustReferenceAggregatePurpose, CheckColumnsSatisfyExprsPurpose,
-    check_columns_satisfy_exprs, deduplicate_select_expr_names, extract_aliases,
-    rebase_expr, resolve_aliases_to_exprs, resolve_columns, resolve_positions_to_exprs,
-    rewrite_recursive_unnests_bottom_up,
+    check_columns_satisfy_exprs, extract_aliases, rebase_expr, resolve_aliases_to_exprs,
+    resolve_columns, resolve_positions_to_exprs, rewrite_recursive_unnests_bottom_up,
 };
 
 use datafusion_common::error::DataFusionErrorBuilder;
@@ -110,10 +109,6 @@ impl<S: ContextProvider> SqlToRel<'_, S> {
             planner_context,
         )?;
 
-        // Auto-suffix duplicate expression names (e.g. cov, cov → cov, cov:1)
-        // before projection so that the unique-name constraint is satisfied.
-        let select_exprs = deduplicate_select_expr_names(select_exprs);
-
         // Having and group by clause may reference aliases defined in select projection
         let projected_plan = self.project(base_plan.clone(), select_exprs)?;
         let select_exprs = projected_plan.expressions();

datafusion/sql/src/utils.rs

@@ -33,7 +33,6 @@ use datafusion_expr::builder::get_struct_unnested_columns;
 use datafusion_expr::expr::{
     Alias, GroupingSet, Unnest, WindowFunction, WindowFunctionParams,
 };
-use datafusion_expr::select_expr::SelectExpr;
 use datafusion_expr::utils::{expr_as_column_expr, find_column_exprs};
 use datafusion_expr::{
     ColumnUnnestList, Expr, ExprSchemable, LogicalPlan, col, expr_vec_fmt,
@@ -634,38 +633,6 @@ fn push_projection_dedupl(projection: &mut Vec<Expr>, expr: Expr) {
         projection.push(expr);
     }
 }
-
-/// Auto-suffix duplicate SELECT expression names with `:{count}`.
-///
-/// The first occurrence keeps its original name so that ORDER BY / HAVING
-/// references resolve correctly. Wildcards are left untouched because they
-/// are expanded later in `project_with_validation`.
-///
-/// Duplicates are detected by the schema name of each expression, which
-/// identifies logically identical expressions before column normalization.
-pub(crate) fn deduplicate_select_expr_names(exprs: Vec<SelectExpr>) -> Vec<SelectExpr> {
-    let mut seen: HashMap<String, usize> = HashMap::new();
-    exprs
-        .into_iter()
-        .map(|select_expr| match select_expr {
-            SelectExpr::Expression(expr) => {
-                let name = expr.schema_name().to_string();
-                let count = seen.entry(name.clone()).or_insert(0);
-                let result = if *count > 0 {
-                    let (_qualifier, field_name) = expr.qualified_name();
-                    SelectExpr::Expression(expr.alias(format!("{field_name}:{count}")))
-                } else {
-                    SelectExpr::Expression(expr)
-                };
-                *count += 1;
-                result
-            }
-            // Leave wildcards alone — they are expanded later
-            other => other,
-        })
-        .collect()
-}
-
 /// The context is we want to rewrite unnest() into InnerProjection->Unnest->OuterProjection
 /// Given an expression which contains unnest expr as one of its children,
 /// Try transform depends on unnest type

datafusion/sql/tests/sql_integration.rs

@@ -789,13 +789,11 @@ fn select_column_does_not_exist() {
 #[test]
 fn select_repeated_column() {
     let sql = "SELECT age, age FROM person";
-    let plan = logical_plan(sql).unwrap();
+    let err = logical_plan(sql).expect_err("query should have failed");
+
     assert_snapshot!(
-        plan,
-        @r"
-    Projection: person.age, person.age AS age:1
-      TableScan: person
-    "
+        err.strip_backtrace(),
+        @r#"Error during planning: Projections require unique expression names but the expression "person.age" at position 0 and "person.age" at position 1 have the same name. Consider aliasing ("AS") one of them."#
     );
 }
 
@@ -1533,14 +1531,11 @@ fn select_simple_aggregate_column_does_not_exist() {
 #[test]
 fn select_simple_aggregate_repeated_aggregate() {
     let sql = "SELECT MIN(age), MIN(age) FROM person";
-    let plan = logical_plan(sql).unwrap();
+    let err = logical_plan(sql).expect_err("query should have failed");
+
     assert_snapshot!(
-        plan,
-        @r"
-    Projection: min(person.age), min(person.age) AS min(person.age):1
-      Aggregate: groupBy=[[]], aggr=[[min(person.age)]]
-        TableScan: person
-    "
+        err.strip_backtrace(),
+        @r#"Error during planning: Projections require unique expression names but the expression "min(person.age)" at position 0 and "min(person.age)" at position 1 have the same name. Consider aliasing ("AS") one of them."#
     );
 }
 
@@ -1589,14 +1584,11 @@ fn select_from_typed_string_values() {
 #[test]
 fn select_simple_aggregate_repeated_aggregate_with_repeated_aliases() {
     let sql = "SELECT MIN(age) AS a, MIN(age) AS a FROM person";
-    let plan = logical_plan(sql).unwrap();
+    let err = logical_plan(sql).expect_err("query should have failed");
+
     assert_snapshot!(
-        plan,
-        @r"
-    Projection: min(person.age) AS a, min(person.age) AS a AS a:1
-      Aggregate: groupBy=[[]], aggr=[[min(person.age)]]
-        TableScan: person
-    "
+        err.strip_backtrace(),
+        @r#"Error during planning: Projections require unique expression names but the expression "min(person.age) AS a" at position 0 and "min(person.age) AS a" at position 1 have the same name. Consider aliasing ("AS") one of them."#
     );
 }
 
@@ -1633,14 +1625,11 @@ fn select_simple_aggregate_with_groupby_with_aliases() {
 #[test]
 fn select_simple_aggregate_with_groupby_with_aliases_repeated() {
     let sql = "SELECT state AS a, MIN(age) AS a FROM person GROUP BY state";
-    let plan = logical_plan(sql).unwrap();
+    let err = logical_plan(sql).expect_err("query should have failed");
+
     assert_snapshot!(
-        plan,
-        @r"
-    Projection: person.state AS a, min(person.age) AS a AS a:1
-      Aggregate: groupBy=[[person.state]], aggr=[[min(person.age)]]
-        TableScan: person
-    "
+        err.strip_backtrace(),
+        @r#"Error during planning: Projections require unique expression names but the expression "person.state AS a" at position 0 and "min(person.age) AS a" at position 1 have the same name. Consider aliasing ("AS") one of them."#
     );
 }
 
@@ -1761,14 +1750,11 @@ fn select_simple_aggregate_with_groupby_can_use_alias() {
 #[test]
 fn select_simple_aggregate_with_groupby_aggregate_repeated() {
     let sql = "SELECT state, MIN(age), MIN(age) FROM person GROUP BY state";
-    let plan = logical_plan(sql).unwrap();
+    let err = logical_plan(sql).expect_err("query should have failed");
+
     assert_snapshot!(
-        plan,
-        @r"
-    Projection: person.state, min(person.age), min(person.age) AS min(person.age):1
-      Aggregate: groupBy=[[person.state]], aggr=[[min(person.age)]]
-        TableScan: person
-    "
+        err.strip_backtrace(),
+        @r#"Error during planning: Projections require unique expression names but the expression "min(person.age)" at position 1 and "min(person.age)" at position 2 have the same name. Consider aliasing ("AS") one of them."#
     );
 }
 

datafusion/sqllogictest/test_files/aggregate.slt

@@ -7941,16 +7941,21 @@ select count(), count() * count() from t;
 ----
 2 4
 
-# Duplicate aggregate expressions are now auto-suffixed
-query II
+# DataFusion error: Error during planning: Projections require unique expression names but the expression "count\(Int64\(1\)\)" at position 0 and "count\(Int64\(1\)\)" at position 1 have the same name\. Consider aliasing \("AS"\) one of them\.
+query error
 select count(1), count(1) from t;
-----
-2 2
 
-query II
+# DataFusion error: Error during planning: Projections require unique expression names but the expression "count\(Int64\(1\)\)" at position 0 and "count\(Int64\(1\)\)" at position 1 have the same name\. Consider aliasing \("AS"\) one of them\.
+query error
+explain select count(1), count(1) from t;
+
+# DataFusion error: Error during planning: Projections require unique expression names but the expression "count\(Int64\(1\) AS \)" at position 0 and "count\(Int64\(1\) AS \)" at position 1 have the same name\. Consider aliasing \("AS"\) one of them\.
+query error
 select count(), count() from t;
-----
-2 2
+
+# DataFusion error: Error during planning: Projections require unique expression names but the expression "count\(Int64\(1\) AS \)" at position 0 and "count\(Int64\(1\) AS \)" at position 1 have the same name\. Consider aliasing \("AS"\) one of them\.
+query error
+explain select count(), count() from t;
 
 query II
 select count(1), count(2) from t;

datafusion/sqllogictest/test_files/duplicate_column_alias.slt

@@ -547,16 +547,8 @@ select unnest(column1) from (select * from (values([1,2,3]), ([4,5,6])) limit 1
 5
 6
 
-query II
+query error DataFusion error: Error during planning: Projections require unique expression names but the expression "UNNEST\(unnest_table.column1\)" at position 0 and "UNNEST\(unnest_table.column1\)" at position 1 have the same name. Consider aliasing \("AS"\) one of them.
 select unnest(column1), unnest(column1) from unnest_table;
-----
-1 1
-2 2
-3 3
-4 4
-5 5
-6 6
-12 12
 
 query II
 select unnest(column1), unnest(column1) u1 from unnest_table;