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/exec/TestColumnInfo.java

@@ -0,0 +1,39 @@
+/*
+ * 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.exec;
+
+import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class TestColumnInfo {
+
+  @Test
+  public void testAmbiguousNameDefaultsToFalse() {
+    ColumnInfo colInfo = new ColumnInfo("_col0", TypeInfoFactory.stringTypeInfo, "t", false);
+    Assert.assertFalse(colInfo.hasAmbiguousName());
+  }
+
+  @Test
+  public void testCopyConstructorPreservesAmbiguousName() {
+    ColumnInfo original = new ColumnInfo("_col0", TypeInfoFactory.stringTypeInfo, "t", false);
+    original.setAmbiguousName(true);
+    Assert.assertTrue(new ColumnInfo(original).hasAmbiguousName());
+  }
+}

ql/src/test/org/apache/hadoop/hive/ql/parse/TestSemanticAnalyzer.java

@@ -21,6 +21,7 @@
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertThrows;
 import static org.junit.Assert.assertTrue;
 import static org.junit.Assert.fail;
 import static org.mockito.ArgumentMatchers.any;
@@ -33,13 +34,16 @@
 import static org.mockito.Mockito.when;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
 
 import com.google.common.collect.Sets;
+import org.antlr.runtime.CommonToken;
 import org.apache.hadoop.hive.common.MaterializationSnapshot;
 import org.apache.hadoop.hive.common.type.Date;
 import org.apache.hadoop.hive.conf.HiveConf;
@@ -55,6 +59,7 @@
 import org.apache.hadoop.hive.ql.QueryProperties.QueryType;
 import org.apache.hadoop.hive.ql.QueryState;
 import org.apache.hadoop.hive.ql.cache.results.QueryResultsCache;
+import org.apache.hadoop.hive.ql.exec.ColumnInfo;
 import org.apache.hadoop.hive.ql.exec.FileSinkOperator;
 import org.apache.hadoop.hive.ql.exec.Operator;
 import org.apache.hadoop.hive.ql.lockmgr.DbTxnManager;
@@ -66,6 +71,7 @@
 import org.apache.hadoop.hive.ql.security.HadoopDefaultAuthenticator;
 import org.apache.hadoop.hive.ql.session.SessionState;
 import org.apache.hadoop.hive.serde2.io.DateWritableV2;
+import org.apache.hadoop.hive.serde2.typeinfo.TypeInfoFactory;
 import org.junit.AfterClass;
 import org.junit.Assert;
 import org.junit.BeforeClass;
@@ -545,4 +551,108 @@ private void testMaterializeCTEUsesDDLFactory(boolean cboEnabled) throws Excepti
           cteAnalyzer[0] instanceof CreateTableAnalyzer);
     }
   }
+
+  // ==== HIVE-29580: duplicate column aliases escaping a subquery/CTE boundary ====
+
+  private BaseSemanticAnalyzer analyzeWithCbo(String query) throws Exception {
+    HiveConf cboConf = new HiveConf(conf);
+    cboConf.setBoolVar(HiveConf.ConfVars.HIVE_CBO_ENABLED, true);
+    SessionState.start(cboConf);
+    Context ctx = new Context(cboConf);
+    ASTNode astNode = ParseUtils.parse(query, ctx);
+    QueryState queryState = new QueryState.Builder().withHiveConf(cboConf).build();
+    BaseSemanticAnalyzer analyzer = SemanticAnalyzerFactory.get(queryState, astNode);
+    analyzer.initCtx(ctx);
+    try {
+      analyzer.analyze(astNode, ctx);
+    } finally {
+      analyzer.endAnalysis(astNode);
+    }
+    return analyzer;
+  }
+
+  private void assertCboRejectsAmbiguous(String query, String expectedReference) {
+    SemanticException e = assertThrows(SemanticException.class, () -> analyzeWithCbo(query));
+    assertTrue(e.getMessage(),
+        e.getMessage().contains("Ambiguous column reference " + expectedReference));
+  }
+
+  @Test
+  public void testCboRejectsAmbiguousReferenceAcrossCteBoundary() {
+    assertCboRejectsAmbiguous(
+        "with bse as (select 'a' as c, 'b' as c), tpm as (select * from bse) select tpm.c from tpm",
+        "c in tpm");
+  }
+
+  @Test
+  public void testCboAmbiguityMarkerSurvivesWindowingProjection() {
+    assertCboRejectsAmbiguous(
+        "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",
+        "c in x");
+  }
+
+  @Test
+  public void testCboToleratesUnionDistinctWithDuplicateAliases() throws Exception {
+    // UNION DISTINCT is rewritten into SELECT DISTINCT * whose group by references are
+    // synthesized by genSelectDIAST; despite the duplicate output alias this must compile
+    assertNotNull(analyzeWithCbo("select x.key, z.value, y.value"
+        + " from table1 x join table2 y on x.key = y.key"
+        + " join (select * from table1 union select * from table2) z on x.value = z.value"
+        + " union"
+        + " select x.key, z.value, y.value"
+        + " from table1 x join table2 y on x.key = y.key"
+        + " join (select * from table1 union select * from table2) z on x.value = z.value"));
+  }
+
+  private static ColumnInfo stringCol(String internalName, String tab, String alias, boolean markedAmbiguous) {
+    ColumnInfo colInfo = new ColumnInfo(internalName, TypeInfoFactory.stringTypeInfo, tab, false);
+    colInfo.setAlias(alias);
+    colInfo.setAmbiguousName(markedAmbiguous);
+    return colInfo;
+  }
+
+  private SemanticAnalyzer newAnalyzerForDirectCalls() throws Exception {
+    SessionState.start(conf);
+    QueryState queryState = new QueryState.Builder().withHiveConf(conf).build();
+    SemanticAnalyzer analyzer = new SemanticAnalyzer(queryState);
+    analyzer.initCtx(new Context(conf));
+    // genColListRegex consults tableMask, which analyzeInternal normally initializes
+    analyzer.tableMask = new TableMask(analyzer, conf, true);
+    return analyzer;
+  }
+
+  private static ASTNode allColRef() {
+    return new ASTNode(new CommonToken(HiveParser.TOK_ALLCOLREF, "TOK_ALLCOLREF"));
+  }
+
+  @Test
+  public void testGenColListRegexPropagatesAmbiguousNameMarker() throws Exception {
+    SemanticAnalyzer analyzer = newAnalyzerForDirectCalls();
+    RowResolver input = new RowResolver();
+    input.put("t", "c", stringCol("_c0", "t", "c", true));
+    input.put("t", "d", stringCol("_c1", "t", "d", false));
+    RowResolver output = new RowResolver();
+
+    analyzer.genColListRegex(".*", "t", allColRef(), new ArrayList<>(),
+        new HashSet<>(), input, null, 0, output, new ArrayList<>(Arrays.asList("t")), false);
+
+    assertTrue(output.get("t", "c").hasAmbiguousName());
+    assertFalse(output.get("t", "d").hasAmbiguousName());
+  }
+
+  @Test
+  public void testGenColListRegexPropagatesAmbiguousNameMarkerForNamedJoin() throws Exception {
+    SemanticAnalyzer analyzer = newAnalyzerForDirectCalls();
+    RowResolver colSrcRR = new RowResolver();
+    colSrcRR.put("l", "c", stringCol("_l0", "l", "c", true));
+    colSrcRR.put("r", "c", stringCol("_r0", "r", "c", false));
+    colSrcRR.setNamedJoinInfo(new NamedJoinInfo(Arrays.asList("l", "r"), Arrays.asList("c"), JoinType.INNER));
+    RowResolver output = new RowResolver();
+
+    analyzer.genColListRegex(".*", null, allColRef(), new ArrayList<>(),
+        new HashSet<>(), colSrcRR, colSrcRR, 0, output, new ArrayList<>(Arrays.asList("l", "r")), false);
+
+    assertTrue(output.get("l", "c").hasAmbiguousName());
+  }
 }