HIVE-29580: CBO: Ambiguous column reference not detected in some CTE/CTAS/other queries

ql/src/java/org/apache/hadoop/hive/ql/exec/ColumnInfo.java

@@ -59,6 +59,8 @@ public class ColumnInfo implements Serializable {
 
   private boolean isHiddenVirtualCol;
 
+  private boolean ambiguousName;
+
   private String typeName;
 
   private final boolean nullable;
@@ -129,9 +131,23 @@ public ColumnInfo(ColumnInfo columnInfo) {
     this.isVirtualCol = columnInfo.getIsVirtualCol();
     this.isHiddenVirtualCol = columnInfo.isHiddenVirtualCol();
     this.nullable = columnInfo.nullable;
+    this.ambiguousName = columnInfo.ambiguousName;
     this.setType(columnInfo.getType());
   }
 
+  /**
+   * True when this column's alias collided with another column's at a subquery/CTE boundary:
+   * the column stays usable positionally (star expansion, count(*)) but any by-name reference
+   * is ambiguous and must be rejected.
+   */
+  public boolean hasAmbiguousName() {
+    return ambiguousName;
+  }
+
+  public void setAmbiguousName(boolean ambiguousName) {
+    this.ambiguousName = ambiguousName;
+  }
+
   public String getTypeName() {
     return this.typeName;
   }

ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java

@@ -3616,6 +3616,12 @@ private RelNode genGBLogicalPlan(QB qb, RelNode srcRel) throws SemanticException
           // As we said before, here we use genSelectLogicalPlan to rewrite AllColRef
           srcRel = genSelectLogicalPlan(qb, srcRel, srcRel, null, null, true).getKey();
           RowResolver rr = relToHiveRR.get(srcRel);
+          // genSelectDIAST synthesizes one reference per rslvMap entry, each unique by
+          // construction, so clear the HIVE-29580 ambiguity markers on this rewrite-private
+          // projection; the subquery's own RowResolver keeps them for user-written references.
+          for (ColumnInfo colInfo : rr.getColumnInfos()) {
+            colInfo.setAmbiguousName(false);
+          }
           qbp.setSelExprForClause(destClauseName, genSelectDIAST(rr));
         }
       }
@@ -4545,6 +4551,7 @@ && isRegex(
           ColumnInfo colInfo = outputRR.getColumnInfos().get(i);
           ColumnInfo newColInfo = new ColumnInfo(colInfo.getInternalName(),
               colInfo.getType(), colInfo.getTabAlias(), colInfo.getIsVirtualCol());
+          newColInfo.setAmbiguousName(colInfo.hasAmbiguousName());
           groupByOutputRowResolver.put(colInfo.getTabAlias(), colInfo.getAlias(), newColInfo);
           if (gbyKeyExpressions != null && gbyKeyExpressions.size() == outputRR.getColumnInfos().size()) {
             groupByOutputRowResolver.putExpression(gbyKeyExpressions.get(i), colInfo);
@@ -4887,6 +4894,15 @@ private RelNode genLogicalPlan(QB qb, boolean outerMostQB,
           } else if ("".equals(tmp[0]) || tmp[1] == null) {
             // ast expression is not a valid column name for table
             tmp[1] = colInfo.getInternalName();
+          } else if (newRR.get(alias, tmp[1]) != null) {
+            // Duplicate alias escaping the subquery boundary: tolerated for positional use
+            // (HIVE-19770), but poison the name so a later by-name reference fails (HIVE-29580).
+            // Binding the duplicate to its internal name here is deliberate, not redundant:
+            // putWithCheck would otherwise do it via its own fallback AND call keepAmbiguousInfo,
+            // whose reference-time throw in RowResolver.get would then shadow this marker with a
+            // differently formatted message. Do not "simplify" this line away.
+            newRR.get(alias, tmp[1]).setAmbiguousName(true);
+            tmp[1] = colInfo.getInternalName();
           }
           newRR.putWithCheck(alias, tmp[1], colInfo.getInternalName(), newCi);
         }

ql/src/java/org/apache/hadoop/hive/ql/parse/SemanticAnalyzer.java

@@ -4014,6 +4014,7 @@ Integer genColListRegex(String colRegex, String tabAlias, ASTNode sel,
             colList.add(Pair.of(colInfo, colSrcRR));
             oColInfo = new ColumnInfo(getColumnInternalName(pos), colInfo.getType(),
                 colInfo.getTabAlias(), colInfo.getIsVirtualCol(), colInfo.isHiddenVirtualCol());
+            oColInfo.setAmbiguousName(colInfo.hasAmbiguousName());
             inputColsProcessed.put(colInfo, oColInfo);
           }
           if (ensureUniqueCols) {
@@ -4101,6 +4102,7 @@ Integer genColListRegex(String colRegex, String tabAlias, ASTNode sel,
           colList.add(Pair.of(colInfo, input));
           oColInfo = new ColumnInfo(getColumnInternalName(pos), colInfo.getType(),
               colInfo.getTabAlias(), colInfo.getIsVirtualCol(), colInfo.isHiddenVirtualCol());
+          oColInfo.setAmbiguousName(colInfo.hasAmbiguousName());
           inputColsProcessed.put(colInfo, oColInfo);
         }
         assert nonNull(tmp);

ql/src/java/org/apache/hadoop/hive/ql/parse/rewrite/MergeRewriter.java

@@ -113,7 +113,7 @@ private void handleSource(boolean hasWhenNotMatchedClause, String sourceAlias, S
     sqlGenerator.append("FROM\n");
     sqlGenerator.append("(SELECT ");
     sqlGenerator.appendAcidSelectColumns(Operation.MERGE);
-    sqlGenerator.appendAllColsOfTargetTable();
+    sqlGenerator.appendNonPartitionColsOfTargetTable();
     addSourceColumnsForRowLineage(isRowLineageSupported, sqlGenerator, "", conf);
     sqlGenerator.append(" FROM ").appendTargetTableName().append(") ");
     sqlGenerator.appendSubQueryAlias();

ql/src/java/org/apache/hadoop/hive/ql/parse/rewrite/sql/MultiInsertSqlGenerator.java

@@ -169,6 +169,18 @@ public void appendAllColsOfTargetTable(String prefix) {
   public void appendAllColsOfTargetTable() {
     appendCols(targetTable.getAllCols(), FieldSchema::getName);
   }
+
+  /**
+   * Appends the target table's columns, omitting the partition columns when the table uses native
+   * partitioning: appendAcidSelectColumns has already emitted those, and emitting them a second
+   * time yields a projection with duplicate column names, making any by-name reference to them
+   * ambiguous. Non-native tables (e.g. Iceberg) carry partition columns as regular columns, so for
+   * those all columns are appended.
+   */
+  public void appendNonPartitionColsOfTargetTable() {
+    appendCols(targetTable.hasNonNativePartitionSupport()
+        ? targetTable.getAllCols() : targetTable.getCols(), FieldSchema::getName);
+  }
 
   public <T> void appendCols(List<T> columns, Function<T, String> stringConverter) {
     appendCols(columns, null, null, stringConverter);

ql/src/java/org/apache/hadoop/hive/ql/parse/type/JoinCondTypeCheckProcFactory.java

@@ -87,6 +87,7 @@ public Object process(Node nd, Stack<Node> stack, NodeProcessorCtx procCtx,
       if (!qualifiedAccess) {
         colInfo = getColInfo(ctx, null, tableOrCol, expr);
         // It's a column.
+        checkAmbiguousName(colInfo);
         return exprFactory.createColumnRefExpr(colInfo, ctx.getInputRRList());
       } else if (hasTableAlias(ctx, tableOrCol, expr)) {
         return null;
@@ -179,6 +180,7 @@ protected T processQualifiedColRef(TypeCheckCtx ctx, ASTNode expr,
             ErrorMsg.INVALID_COLUMN.getMsg(), expr.getChild(1)), expr);
         return null;
       }
+      checkAmbiguousName(colInfo);
       ColumnInfo newColumnInfo = new ColumnInfo(colInfo);
       newColumnInfo.setTabAlias(tableAlias);
       List<RowResolver> listRR = new ArrayList<>(jctx.getInputRRList());

ql/src/java/org/apache/hadoop/hive/ql/parse/type/TypeCheckProcFactory.java

@@ -594,6 +594,13 @@ protected IntervalExprProcessor getIntervalExprProcessor() {
     return new IntervalExprProcessor();
   }
 
+  static void checkAmbiguousName(ColumnInfo colInfo) throws SemanticException {
+    if (colInfo != null && colInfo.hasAmbiguousName()) {
+      throw new SemanticException(ErrorMsg.AMBIGUOUS_COLUMN.getMsg(
+          colInfo.getAlias() + " in " + colInfo.getTabAlias()));
+    }
+  }
+
   /**
    * Processor for table columns.
    */
@@ -659,6 +666,7 @@ public Object process(Node nd, Stack<Node> stack, NodeProcessorCtx procCtx,
             return null;
           }
           // It's a column.
+          checkAmbiguousName(colInfo);
           return exprFactory.toExpr(colInfo, usedRR, offset);
         } else {
           // It's a table alias.
@@ -693,6 +701,7 @@ public Object process(Node nd, Stack<Node> stack, NodeProcessorCtx procCtx,
           }
         } else {
           // It's a column.
+          checkAmbiguousName(colInfo);
           return exprFactory.toExpr(colInfo, usedRR, offset);
         }
       }
@@ -1299,6 +1308,7 @@ protected T processQualifiedColRef(TypeCheckCtx ctx, ASTNode expr,
             ErrorMsg.INVALID_COLUMN.getMsg(), expr.getChild(1)), expr);
         return null;
       }
+      checkAmbiguousName(colInfo);
       return exprFactory.toExpr(colInfo, usedRR, offset);
     }
 

ql/src/test/org/apache/hadoop/hive/ql/parse/type/TestAmbiguousColumnName.java

@@ -0,0 +1,58 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.hadoop.hive.ql.parse.type;
+
+import org.apache.hadoop.hive.ql.exec.ColumnInfo;
+import org.apache.hadoop.hive.ql.parse.SemanticException;
+import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestAmbiguousColumnName {
+
+  private static ColumnInfo colInfo() {
+    ColumnInfo colInfo = new ColumnInfo("_col0", TypeInfoFactory.stringTypeInfo, "t", false);
+    colInfo.setAlias("c");
+    return colInfo;
+  }
+
+  @Test
+  public void testCopyConstructorPreservesAmbiguousName() {
+    ColumnInfo original = colInfo();
+    original.setAmbiguousName(true);
+    Assert.assertTrue(new ColumnInfo(original).hasAmbiguousName());
+  }
+
+  @Test
+  public void testCheckAmbiguousNameThrows() {
+    ColumnInfo marked = colInfo();
+    marked.setAmbiguousName(true);
+    try {
+      TypeCheckProcFactory.checkAmbiguousName(marked);
+      Assert.fail("expected SemanticException");
+    } catch (SemanticException e) {
+      Assert.assertTrue(e.getMessage(), e.getMessage().contains("Ambiguous column reference c in t"));
+    }
+  }
+
+  @Test
+  public void testCheckAmbiguousNameNoThrow() throws SemanticException {
+    TypeCheckProcFactory.checkAmbiguousName(colInfo());
+  }
+}

ql/src/test/queries/clientnegative/ambiguous_col_2_noncbo.q

@@ -0,0 +1,3 @@
+set hive.cbo.enable=false;
+create table t1nc (c1 int);
+explain select t.c1 from (select t11.c1, t12.c1 from t1nc as t11 inner join t1nc as t12 on t11.c1 = t12.c1) as t;

ql/src/test/queries/clientnegative/ambiguous_col_aggregate_noncbo.q

@@ -0,0 +1,3 @@
+--! qt:dataset:src
+set hive.cbo.enable=false;
+select count(*) from (select key, key from src) subq;

ql/src/test/queries/clientnegative/ambiguous_col_ctas.q

@@ -0,0 +1,9 @@
+-- A CTAS whose SELECT contains a cross-alias ambiguous reference. Without the ambiguity check
+-- this silently persists one arbitrarily-chosen candidate into a table ('FIRST', discarding
+-- 'SECOND') and reports nothing, so every downstream reader treats the arbitrary choice as fact.
+-- Keep this test if the check ever gains an exemption for statements Hive generates for itself
+-- (rewritten MERGE/UPDATE/DELETE, materialised CTEs): a USER CTAS must never be exempted.
+create table ctas_ambiguous_ref as
+  with bse as (select 'FIRST' as c, 'SECOND' as c),
+       tpm as (select * from bse)
+  select tpm.c from tpm;

ql/src/test/queries/clientnegative/ambiguous_col_cte.q

@@ -0,0 +1,3 @@
+with bse as (select 'a' as c, 'b' as c),
+     tpm as (select * from bse)
+select tpm.c from tpm;

ql/src/test/queries/clientnegative/ambiguous_col_cte_noncbo.q

@@ -0,0 +1,4 @@
+set hive.cbo.enable=false;
+with bse as (select 'a' as c, 'b' as c),
+     tpm as (select * from bse)
+select tpm.c from tpm;

ql/src/test/queries/clientnegative/ambiguous_col_cte_unreferenced_noncbo.q

@@ -0,0 +1,3 @@
+set hive.cbo.enable=false;
+with c1 as (select 'a' as c, 'b' as c, 'x' as d)
+select d from c1;

ql/src/test/queries/clientnegative/ambiguous_col_distinct_window.q

@@ -0,0 +1 @@
+select x.c from (select distinct *, rank() over (order by d) r from (select 'a' as c, 'b' as c, 'x' as d) t) x;

ql/src/test/queries/clientnegative/ambiguous_col_exists_having_noncbo.q

@@ -0,0 +1,3 @@
+--! qt:dataset:src
+set hive.cbo.enable=false;
+select value, count(1) from src group by value having exists (select 'x' as c, 'y' as c from src b where b.value = src.value);

ql/src/test/queries/clientnegative/ambiguous_col_exists_noncbo.q

@@ -0,0 +1,3 @@
+--! qt:dataset:src
+set hive.cbo.enable=false;
+select key from src a where exists (select 'x' as c, 'y' as c from src b where b.key = a.key);

ql/src/test/queries/clientnegative/ambiguous_col_join_cond.q

@@ -0,0 +1,7 @@
+-- the duplicate alias is referenced only in the join condition, via a qualified name.
+-- Unparse translation is disabled below on purpose: when it is on, the ON clause is also
+-- walked by the generic type check, which would make this test pass even if the ambiguity
+-- check in JoinCondTypeCheckProcFactory were removed.
+set hive.materializedview.rewriting.sql=false;
+set hive.materializedview.rewriting.sql.subquery=false;
+select t.d from (select 'a' as c, 'b' as c, 'x' as d) t join (select 'a' as e) u on t.c = u.e;

ql/src/test/queries/clientnegative/ambiguous_col_join_cond_unqual.q

@@ -0,0 +1,5 @@
+-- same as ambiguous_col_join_cond.q but the reference is unqualified, which is resolved by a
+-- different processor override. See that file for why unparse translation is disabled here.
+set hive.materializedview.rewriting.sql=false;
+set hive.materializedview.rewriting.sql.subquery=false;
+select t.d from (select 'a' as c, 'b' as c, 'x' as d) t join (select 'a' as e) u on c = u.e;

ql/src/test/queries/clientnegative/ambiguous_col_join_using.q

@@ -0,0 +1 @@
+select t.c from (select * from (select 'a' as c, 'b' as c) s join (select 'a' as c) u using (c)) t;

ql/src/test/queries/clientnegative/ambiguous_col_join_wildcard_noncbo.q

@@ -0,0 +1,4 @@
+set hive.cbo.enable=false;
+create table wj3 (k int, v int);
+create table wj4 (k int, w int);
+select t.v from (select a.*, b.* from wj3 a join wj4 b on a.k = b.k) t;

ql/src/test/queries/clientnegative/ambiguous_col_lateral_view_alias.q

@@ -0,0 +1,7 @@
+-- The lateral view reuses the table alias, so the exploded column collides with the base table
+-- column under the same table alias. This is caught by the reference-time check in
+-- RowResolver.get (ambiguousColumns), not by the duplicate-alias marker: the message form
+-- "Ambiguous column reference: t.c" identifies that path. Keep this test: it is the only
+-- coverage of that check, which would otherwise look like dead code and get removed.
+create table lv_dup_alias (c int, arr array<int>);
+select t.c from lv_dup_alias t lateral view explode(t.arr) t as c;

ql/src/test/queries/clientnegative/ambiguous_col_nested_star_noncbo.q

@@ -0,0 +1,2 @@
+set hive.cbo.enable=false;
+select * from (select * from (select 'a' as c, 'b' as c) a) b;

ql/src/test/queries/clientnegative/ambiguous_col_noncbo.q

@@ -0,0 +1,3 @@
+--! qt:dataset:src
+set hive.cbo.enable=false;
+FROM (SELECT key, concat(value) AS key FROM src) a SELECT a.key;

ql/src/test/queries/clientnegative/ambiguous_col_union.q

@@ -0,0 +1 @@
+select t.c from (select 'a' as c, 'b' as c union all select 'x', 'y') t;

ql/src/test/queries/clientnegative/ambiguous_col_union_noncbo.q

@@ -0,0 +1,2 @@
+set hive.cbo.enable=false;
+select t.c from (select 'a' as c, 'b' as c union all select 'x', 'y') t;

ql/src/test/queries/clientnegative/ambiguous_col_unqualified_ref.q

@@ -0,0 +1,8 @@
+-- The inner CTE references the duplicated alias by an UNQUALIFIED name, which is resolved by
+-- TypeCheckProcFactory.ColumnExprProcessor (the "It's a column" branch) rather than by
+-- processQualifiedColRef. Every other ambiguity test uses a qualified reference, so this is the
+-- only coverage of that check site. The error names the definition-site alias (bse), matching
+-- what the non-CBO path reports for the same query.
+with bse as (select 'a' as c, 'b' as c),
+     tpm as (select c from bse)
+select tpm.c from tpm;

ql/src/test/queries/clientnegative/ambiguous_col_unreferenced_noncbo.q

@@ -0,0 +1,2 @@
+set hive.cbo.enable=false;
+select t.d from (select 'a' as c, 'b' as c, 'x' as d) t;

ql/src/test/queries/clientnegative/ambiguous_colref_in_gby_noncbo.q

@@ -0,0 +1,3 @@
+set hive.cbo.enable=false;
+create table t1gnc (a int);
+select s.a from (select a, a from t1gnc) s group by s.a;

ql/src/test/queries/clientpositive/ambiguous_col_tolerated.q

@@ -0,0 +1,14 @@
+select 'a' as c, 'b' as c;
+select 'a' as c, 'b' as c order by c;
+select * from (select 'a' as c, 'b' as c) t;
+create table dup_alias_ins (x string, y string);
+insert into dup_alias_ins select 'a' as c, 'b' as c;
+select x, y from dup_alias_ins;
+
+set hive.cbo.enable=false;
+
+select 'a' as c, 'b' as c;
+select 'a' as c, 'b' as c order by c;
+select * from (select 'a' as c, 'b' as c) t;
+insert into dup_alias_ins select 'c' as c, 'd' as c;
+select x, y from dup_alias_ins order by x;

ql/src/test/queries/clientpositive/ambiguous_col_union_distinct_tolerated.q

@@ -0,0 +1,10 @@
+-- SORT_QUERY_RESULTS
+-- HIVE-29580: UNION DISTINCT is rewritten into SELECT DISTINCT * over an internal alias, and
+-- the rewrite synthesizes by-name group-by references from the RowResolver it enumerated.
+-- Duplicate output aliases in the branches must not trip the ambiguity check there: the
+-- references are unique by construction. Both statements must compile, deduplicate across all
+-- columns, and keep both duplicate columns' values intact.
+
+select 'a' as c, 'b' as c union select 'a', 'b' union select 'a', 'x';
+
+select distinct * from (select 'a' as c, 'b' as c union all select 'a', 'b' union all select 'a', 'x') t;

ql/src/test/queries/clientpositive/ambiguous_col_unreferenced_tolerated.q

@@ -0,0 +1,13 @@
+--! qt:dataset:src
+select t.d from (select 'a' as c, 'b' as c, 'x' as d) t;
+with c1 as (select 'a' as c, 'b' as c, 'x' as d)
+select d from c1;
+select count(1) from (select 'a' as c, 'b' as c) t;
+select count(*) from (select key, key from src) subq;
+create table wjt1 (k int, v int);
+create table wjt2 (k int, w int);
+select t.v from (select a.*, b.* from wjt1 a join wjt2 b on a.k = b.k) t;
+select * from (select * from (select 'a' as c, 'b' as c) a) b;
+select count(*) from src a where exists (select 'x' as c, 'y' as c from src b where b.key = a.key);
+select count(*) from src where exists (select 'a' as c, 'b' as c from src);
+select count(*) from (select value from src group by value having exists (select 'x' as c, 'y' as c from src b where b.value = src.value)) t;

ql/src/test/queries/clientpositive/cross_prod_3.q

@@ -11,4 +11,6 @@ insert overwrite table X_n2 select distinct * from src order by key limit 10;
 create table Y_n0 as
 select * from src order by key limit 1;
 
-explain select * from Y_n0, (select * from X_n2 as A join X_n2 as B on A.key=B.key) as C where Y_n0.key=C.key;
+-- HIVE-29580: the derived table's columns are aliased explicitly because "select *" over the
+-- self-join projects two columns named key (and value), making the C.key reference ambiguous.
+explain select * from Y_n0, (select A.key, A.value, B.key as key2, B.value as value2 from X_n2 as A join X_n2 as B on A.key=B.key) as C where Y_n0.key=C.key;