ESQL: Validate unique plan attribute names

docs/changelog/110488.yaml

@@ -0,0 +1,6 @@
+pr: 110488
+summary: "ESQL: Validate unique plan attribute names"
+area: ES|QL
+type: bug
+issues:
+ - 110541

x-pack/plugin/esql/qa/testFixtures/src/main/resources/dissect.csv-spec

@@ -50,6 +50,18 @@ last_name:keyword | left:keyword | foo:keyword             | middle:keyword | ri
 Facello           | left         | Georgi1 Georgi2 Facello | middle         | right         | Georgi1      | Georgi2            | Facello
 ;
 
+shadowingInternal
+FROM employees
+| KEEP first_name, last_name
+| WHERE last_name == "Facello"
+| EVAL name = concat(first_name, "1 ", last_name)
+| DISSECT name "%{foo} %{foo}"
+;
+
+first_name:keyword | last_name:keyword | name:keyword    | foo:keyword
+Georgi             | Facello           | Georgi1 Facello | Facello
+;
+
 
 complexPattern
 ROW a = "1953-01-23T12:15:00Z - some text - 127.0.0.1;" 

x-pack/plugin/esql/qa/testFixtures/src/main/resources/drop.csv-spec

@@ -122,3 +122,28 @@ FROM employees | STATS COUNT(*), MIN(salary  * 10), MAX(languages)| DROP `COUNT(
 MIN(salary  * 10):i | MAX(languages):i
 253240              | 5
 ;
+
+// Not really shadowing, but let's keep the name consistent with the other command's tests
+shadowingInternal
+FROM employees
+| KEEP emp_no, first_name, last_name
+| DROP last_name, last_name
+| LIMIT 2
+;
+
+emp_no:integer | first_name:keyword
+         10001 | Georgi
+         10002 | Bezalel
+;
+
+shadowingInternalWildcard
+FROM employees
+| KEEP emp_no, first_name, last_name
+| DROP last*name, last*name, last*, last_name
+| LIMIT 2
+;
+
+emp_no:integer | first_name:keyword
+         10001 | Georgi
+         10002 | Bezalel
+;

x-pack/plugin/esql/qa/testFixtures/src/main/resources/enrich.csv-spec

@@ -107,6 +107,17 @@ ROW left = "left", airport = "Zurich Airport ZRH", city = "Zürich", middle = "m
 left:keyword | city:keyword | middle:keyword | right:keyword | airport:text | region:text | city_boundary:geo_shape
 ;
 
+shadowingInternal
+required_capability: enrich_load
+ROW city = "Zürich"
+| ENRICH city_names ON city WITH x = airport, x = region
+;
+
+city:keyword | x:text
+Zürich       | Bezirk Zürich
+;
+
+
 simple
 required_capability: enrich_load
 

x-pack/plugin/esql/qa/testFixtures/src/main/resources/eval.csv-spec

@@ -33,6 +33,16 @@ left:keyword | middle:keyword | right:keyword | x:integer | y:integer
 left         | middle         | right         | 9         | 10
 ;
 
+shadowingInternal
+ROW x = 10000
+| EVAL x = x + 1, x = x - 2
+;
+
+x:integer
+9999
+;
+
+
 withMath
 row a = 1 | eval b = 2 + 3;
 

x-pack/plugin/esql/qa/testFixtures/src/main/resources/grok.csv-spec

@@ -50,6 +50,17 @@ last_name:keyword | left:keyword | foo:keyword             | middle:keyword | ri
 Facello           | left         | Georgi1 Georgi2 Facello | middle         | right         | Georgi1      | Georgi2            | Facello
 ;
 
+shadowingInternal
+FROM employees
+| KEEP last_name
+| WHERE last_name == "Facello"
+| EVAL name = concat("1 ", last_name)
+| GROK name "%{WORD:foo} %{WORD:foo}"
+;
+
+last_name:keyword | name:keyword | foo:keyword
+Facello           | 1 Facello    | [1, Facello]
+;
 
 complexPattern
 ROW a = "1953-01-23T12:15:00Z 127.0.0.1 some.email@foo.com 42" 

x-pack/plugin/esql/qa/testFixtures/src/main/resources/keep.csv-spec

@@ -539,3 +539,28 @@ c:i
 1
 1
 ;
+
+shadowingInternal
+FROM employees
+| SORT emp_no ASC
+| KEEP last_name, last_name
+| LIMIT 2
+;
+
+last_name:keyword
+Facello
+Simmel
+;
+
+
+shadowingInternalWildcard
+FROM employees
+| SORT emp_no ASC
+| KEEP last*name, last*name, last*, last_name
+| LIMIT 2
+;
+
+last_name:keyword
+Facello
+Simmel
+;

x-pack/plugin/esql/qa/testFixtures/src/main/resources/rename.csv-spec

@@ -174,3 +174,29 @@ avg_worked_seconds:l | birth_date:date          | emp_no:i             | first_n
 341158890            | 1961-10-15T00:00:00.000Z | 10060                | Breannda             | M                    | 1.42                 | 1.4199999570846558   | 1.419921875          | 1.42                 | 1987-11-02T00:00:00.000Z | [false, false, false, true]| [Business Analyst, Data Scientist, Senior Team Lead]                                    | 2                    | 2                    | 2                    | 2                    | Billingsley          | 29175                | [-1.76, -0.85]              | [-1, 0]              | [-0.85, -1.76]             | [-1, 0]              | true                 | 29175               
 246355863            | null                     | 10042                | Magy                 | F                    | 1.44                 | 1.440000057220459    | 1.4404296875         | 1.44                 | 1993-03-21T00:00:00.000Z | null                       | [Architect, Business Analyst, Internship, Junior Developer]                             | 3                    | 3                    | 3                    | 3                    | Stamatiou            | 30404                | [-9.28, 9.42]               | [-9, 9]              | [-9.28, 9.42]              | [-9, 9]              | true                 | 30404               
 ;
+
+shadowing
+FROM employees
+| SORT emp_no ASC
+| KEEP emp_no, first_name, last_name
+| RENAME emp_no AS last_name
+| LIMIT 2
+;
+
+last_name:integer  | first_name:keyword
+             10001 | Georgi
+             10002 | Bezalel
+;
+
+shadowingInternal
+FROM employees
+| SORT emp_no ASC
+| KEEP emp_no, last_name
+| RENAME emp_no AS x, last_name AS x
+| LIMIT 2
+;
+
+x:keyword
+Facello
+Simmel
+;

x-pack/plugin/esql/qa/testFixtures/src/main/resources/row.csv-spec

@@ -36,6 +36,14 @@ a:integer
 // end::multivalue-result[]
 ;
 
+shadowingInternal
+//TODO: needs cap to skip on bwc tests
+row a = 1, a = 2;
+
+a:integer
+        2
+;
+
 unsignedLongLiteral
 ROW long_max = 9223372036854775807, ul_start = 9223372036854775808, ul_end = 18446744073709551615, double=18446744073709551616;
 
@@ -70,10 +78,11 @@ a:integer | b:integer | c:null | z:integer
 ;
 
 evalRowWithNull2
+//TODO: needs cap to skip on bwc tests
 row a = 1, null, b = 2, c = null, null | eval z = a+b;
 
-a:integer | null:null | b:integer | c:null | null:null | z:integer
-1 | null | 2 | null | null | 3
+a:integer | b:integer | c:null | null:null | z:integer
+        1 |         2 | null   | null      |         3
 ;
 
 evalRowWithNull3

x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec

@@ -1819,3 +1819,12 @@ warning:Line 3:17: java.lang.ArithmeticException: / by zero
 w_avg:double
 null
 ;
+
+shadowingInternal
+FROM employees
+| STATS x = MAX(emp_no), x = MIN(emp_no)
+;
+
+x:integer
+10001
+;

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/AnalyzerRules.java

@@ -20,8 +20,6 @@
 import java.util.function.Predicate;
 import java.util.function.Supplier;
 
-import static java.util.Collections.singletonList;
-
 public final class AnalyzerRules {
 
     public abstract static class AnalyzerRule<SubPlan extends LogicalPlan> extends Rule<SubPlan, LogicalPlan> {
@@ -138,14 +136,6 @@ public static List<Attribute> maybeResolveAgainstList(
             )
             .toList();
 
-        return singletonList(
-            ua.withUnresolvedMessage(
-                "Reference ["
-                    + ua.qualifiedName()
-                    + "] is ambiguous (to disambiguate use quotes or qualifiers); "
-                    + "matches any of "
-                    + refs
-            )
-        );
+        throw new IllegalStateException("Reference [" + ua.qualifiedName() + "] is ambiguous; " + "matches any of " + refs);
     }
 }

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/OptimizerRules.java

@@ -8,8 +8,10 @@
 package org.elasticsearch.xpack.esql.optimizer;
 
 import org.elasticsearch.xpack.esql.core.common.Failures;
+import org.elasticsearch.xpack.esql.core.expression.Attribute;
 import org.elasticsearch.xpack.esql.core.expression.AttributeSet;
 import org.elasticsearch.xpack.esql.core.expression.Expressions;
+import org.elasticsearch.xpack.esql.core.expression.NameId;
 import org.elasticsearch.xpack.esql.core.plan.QueryPlan;
 import org.elasticsearch.xpack.esql.plan.logical.Aggregate;
 import org.elasticsearch.xpack.esql.plan.logical.Enrich;
@@ -36,6 +38,9 @@
 import org.elasticsearch.xpack.esql.plan.physical.RowExec;
 import org.elasticsearch.xpack.esql.plan.physical.ShowExec;
 
+import java.util.HashSet;
+import java.util.Set;
+
 import static org.elasticsearch.xpack.esql.core.common.Failure.fail;
 
 class OptimizerRules {
@@ -49,9 +54,24 @@ void checkPlan(P p, Failures failures) {
             AttributeSet input = p.inputSet();
             AttributeSet generated = generates(p);
             AttributeSet missing = refs.subtract(input).subtract(generated);
-            if (missing.size() > 0) {
+            if (missing.isEmpty() == false) {
                 failures.add(fail(p, "Plan [{}] optimized incorrectly due to missing references {}", p.nodeString(), missing));
             }
+
+            Set<String> outputAttributeNames = new HashSet<>();
+            Set<NameId> outputAttributeIds = new HashSet<>();
+            for (Attribute outputAttr : p.output()) {
+                if (outputAttributeNames.add(outputAttr.name()) == false || outputAttributeIds.add(outputAttr.id()) == false) {
+                    failures.add(
+                        fail(
+                            p,
+                            "Plan [{}] optimized incorrectly due to duplicate output attribute {}",
+                            p.nodeString(),
+                            outputAttr.toString()
+                        )
+                    );
+                }
+            }
         }
 
         protected AttributeSet references(P p) {

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java

@@ -74,6 +74,7 @@
 import static org.elasticsearch.xpack.esql.core.parser.ParserUtils.source;
 import static org.elasticsearch.xpack.esql.core.parser.ParserUtils.typedParsing;
 import static org.elasticsearch.xpack.esql.core.parser.ParserUtils.visitList;
+import static org.elasticsearch.xpack.esql.expression.NamedExpressions.mergeOutputExpressions;
 import static org.elasticsearch.xpack.esql.plan.logical.Enrich.Mode;
 import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.stringToInt;
 
@@ -217,8 +218,9 @@ public Map<String, Object> visitCommandOptions(EsqlBaseParser.CommandOptionsCont
     }
 
     @Override
+    @SuppressWarnings("unchecked")
     public LogicalPlan visitRowCommand(EsqlBaseParser.RowCommandContext ctx) {
-        return new Row(source(ctx), visitFields(ctx.fields()));
+        return new Row(source(ctx), (List<Alias>) (List) mergeOutputExpressions(visitFields(ctx.fields()), List.of()));
     }
 
     @Override

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Rename.java

@@ -8,6 +8,7 @@
 package org.elasticsearch.xpack.esql.plan.logical;
 
 import org.elasticsearch.xpack.esql.core.expression.Alias;
+import org.elasticsearch.xpack.esql.core.expression.Attribute;
 import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
 import org.elasticsearch.xpack.esql.core.tree.Source;
 import org.elasticsearch.xpack.esql.expression.function.UnsupportedAttribute;
@@ -28,6 +29,12 @@ public List<Alias> renamings() {
         return renamings;
     }
 
+    @Override
+    public List<Attribute> output() {
+        // Rename is mapped to a Project during analysis; we do not compute the output here.
+        throw new IllegalStateException("Should never reach here.");
+    }
+
     @Override
     public boolean expressionsResolved() {
         for (var alias : renamings) {