Copilot commented on code in PR #6676:
URL: https://github.com/apache/hive/pull/6676#discussion_r3832857230
##########
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);
Review Comment:
Clearing the marker on this RowResolver also removes it from the data that
feeds the DISTINCT/group-by result, not just from the synthetic references.
Consequently, `select x.c from (select distinct * from (select 'a' as c, 'b' as
c) t) x` can bind `x.c` to the first candidate instead of raising 10007:
`genSelectDIAST` rewrites the columns as `t.c` and `t._col1`, and the cleared
marker is not recreated at the outer boundary. Preserve/reapply the marker on
the DISTINCT output while bypassing it only for type-checking the
rewrite-generated references.
##########
ql/src/java/org/apache/hadoop/hive/ql/parse/CalcitePlanner.java:
##########
@@ -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);
Review Comment:
An inherited marker survives a positional CTE/subquery column list even
though that list gives the columns new, unique names. For example, `with bse as
(select 'a' c, 'b' c), renamed(a,b) as (select * from bse) select renamed.a
from renamed` reaches this marker at the `bse` boundary, copies it into
`renamed.a`, and is then rejected incorrectly. Since `processTableColumnNames`
already rejects duplicate names in the explicit list, clear the copied marker
when applying each positional alias; a later collision with an unlisted column
will mark it again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
