Browse Source

ESQL: Validate unique plan attribute names (#110488)

* Enforce an invariant in our dependency checker so that logical plans never have duplicate output attribute names or ids.
* Fix ROW to not produce columns with duplicate names.
* Fix ResolveUnionTypes to not create multiple synthetic field attributes for the same union type.
* Add tests for commands using the same column name more than once.
* Update docs w.r.t. how commands behave if they are used with duplicate column names.
Alexander Spies 1 year ago
parent
commit
da5392134f
29 changed files with 516 additions and 28 deletions
  1. 6 0
      docs/changelog/110488.yaml
  2. 2 0
      docs/reference/esql/processing-commands/dissect.asciidoc
  3. 6 1
      docs/reference/esql/processing-commands/enrich.asciidoc
  4. 3 1
      docs/reference/esql/processing-commands/eval.asciidoc
  5. 15 0
      docs/reference/esql/processing-commands/grok.asciidoc
  6. 3 1
      docs/reference/esql/processing-commands/keep.asciidoc
  7. 1 0
      docs/reference/esql/processing-commands/lookup.asciidoc
  8. 3 1
      docs/reference/esql/processing-commands/rename.asciidoc
  9. 3 0
      docs/reference/esql/processing-commands/stats.asciidoc
  10. 1 0
      docs/reference/esql/source-commands/row.asciidoc
  11. 3 2
      x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvTestsDataLoader.java
  12. 4 0
      x-pack/plugin/esql/qa/testFixtures/src/main/resources/addresses.csv
  13. 25 0
      x-pack/plugin/esql/qa/testFixtures/src/main/resources/dissect.csv-spec
  14. 17 0
      x-pack/plugin/esql/qa/testFixtures/src/main/resources/docs.csv-spec
  15. 50 0
      x-pack/plugin/esql/qa/testFixtures/src/main/resources/drop.csv-spec
  16. 68 0
      x-pack/plugin/esql/qa/testFixtures/src/main/resources/enrich.csv-spec
  17. 23 0
      x-pack/plugin/esql/qa/testFixtures/src/main/resources/eval.csv-spec
  18. 25 0
      x-pack/plugin/esql/qa/testFixtures/src/main/resources/grok.csv-spec
  19. 60 0
      x-pack/plugin/esql/qa/testFixtures/src/main/resources/keep.csv-spec
  20. 44 0
      x-pack/plugin/esql/qa/testFixtures/src/main/resources/mapping-addresses.json
  21. 39 0
      x-pack/plugin/esql/qa/testFixtures/src/main/resources/rename.csv-spec
  22. 21 2
      x-pack/plugin/esql/qa/testFixtures/src/main/resources/row.csv-spec
  23. 33 0
      x-pack/plugin/esql/qa/testFixtures/src/main/resources/stats.csv-spec
  24. 9 3
      x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java
  25. 20 4
      x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java
  26. 1 11
      x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/AnalyzerRules.java
  27. 21 1
      x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/optimizer/OptimizerRules.java
  28. 3 1
      x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java
  29. 7 0
      x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/Rename.java

+ 6 - 0
docs/changelog/110488.yaml

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

+ 2 - 0
docs/reference/esql/processing-commands/dissect.asciidoc

@@ -20,6 +20,8 @@ multiple values, `DISSECT` will process each value.
 
 
 `pattern`::
 `pattern`::
 A <<esql-dissect-patterns,dissect pattern>>.
 A <<esql-dissect-patterns,dissect pattern>>.
+If a field name conflicts with an existing column, the existing column is dropped.
+If a field name is used more than once, only the rightmost duplicate creates a column.
 
 
 `<separator>`::
 `<separator>`::
 A string used as the separator between appended values, when using the <<esql-append-modifier,append modifier>>.
 A string used as the separator between appended values, when using the <<esql-append-modifier,append modifier>>.

+ 6 - 1
docs/reference/esql/processing-commands/enrich.asciidoc

@@ -31,11 +31,16 @@ name as the `match_field` defined in the <<esql-enrich-policy,enrich policy>>.
 The enrich fields from the enrich index that are added to the result as new
 The enrich fields from the enrich index that are added to the result as new
 columns. If a column with the same name as the enrich field already exists, the
 columns. If a column with the same name as the enrich field already exists, the
 existing column will be replaced by the new column. If not specified, each of
 existing column will be replaced by the new column. If not specified, each of
-the enrich fields defined in the policy is added
+the enrich fields defined in the policy is added.
+A column with the same name as the enrich field will be dropped unless the
+enrich field is renamed.
 
 
 `new_nameX`::
 `new_nameX`::
 Enables you to change the name of the column that's added for each of the enrich
 Enables you to change the name of the column that's added for each of the enrich
 fields. Defaults to the enrich field name.
 fields. Defaults to the enrich field name.
+If a column has the same name as the new name, it will be discarded.
+If a name (new or original) occurs more than once, only the rightmost duplicate
+creates a new column.
 
 
 *Description*
 *Description*
 
 

+ 3 - 1
docs/reference/esql/processing-commands/eval.asciidoc

@@ -16,10 +16,12 @@ EVAL [column1 =] value1[, ..., [columnN =] valueN]
 
 
 `columnX`::
 `columnX`::
 The column name.
 The column name.
+If a column with the same name already exists, the existing column is dropped.
+If a column name is used more than once, only the rightmost duplicate creates a column.
 
 
 `valueX`::
 `valueX`::
 The value for the column. Can be a literal, an expression, or a
 The value for the column. Can be a literal, an expression, or a
-<<esql-functions,function>>.
+<<esql-functions,function>>. Can use columns defined left of this one.
 
 
 *Description*
 *Description*
 
 

+ 15 - 0
docs/reference/esql/processing-commands/grok.asciidoc

@@ -20,6 +20,9 @@ multiple values, `GROK` will process each value.
 
 
 `pattern`::
 `pattern`::
 A grok pattern.
 A grok pattern.
+If a field name conflicts with an existing column, the existing column is discarded.
+If a field name is used more than once, a multi-valued column will be created with one value
+per each occurrence of the field name.
 
 
 *Description*
 *Description*
 
 
@@ -67,4 +70,16 @@ include::{esql-specs}/docs.csv-spec[tag=grokWithToDatetime]
 |===
 |===
 include::{esql-specs}/docs.csv-spec[tag=grokWithToDatetime-result]
 include::{esql-specs}/docs.csv-spec[tag=grokWithToDatetime-result]
 |===
 |===
+
+If a field name is used more than once, `GROK` creates a multi-valued
+column:
+
+[source.merge.styled,esql]
+----
+include::{esql-specs}/docs.csv-spec[tag=grokWithDuplicateFieldNames]
+----
+[%header.monospaced.styled,format=dsv,separator=|]
+|===
+include::{esql-specs}/docs.csv-spec[tag=grokWithDuplicateFieldNames-result]
+|===
 // end::examples[]
 // end::examples[]

+ 3 - 1
docs/reference/esql/processing-commands/keep.asciidoc

@@ -16,6 +16,8 @@ KEEP columns
 
 
 `columns`::
 `columns`::
 A comma-separated list of columns to keep. Supports wildcards.
 A comma-separated list of columns to keep. Supports wildcards.
+See below for the behavior in case an existing column matches multiple
+given wildcards or column names.
 
 
 *Description*
 *Description*
 
 
@@ -29,7 +31,7 @@ Fields are added in the order they appear. If one field matches multiple express
 2. Partial wildcard expressions (for example: `fieldNam*`)
 2. Partial wildcard expressions (for example: `fieldNam*`)
 3. Wildcard only (`*`)
 3. Wildcard only (`*`)
 
 
-If a field matches two expressions with the same precedence, the right-most expression wins.
+If a field matches two expressions with the same precedence, the rightmost expression wins.
 
 
 Refer to the examples for illustrations of these precedence rules.
 Refer to the examples for illustrations of these precedence rules.
 
 

+ 1 - 0
docs/reference/esql/processing-commands/lookup.asciidoc

@@ -18,6 +18,7 @@ LOOKUP table ON match_field1[, match_field2, ...]
 
 
 `table`::
 `table`::
 The name of the `table` provided in the request to match.
 The name of the `table` provided in the request to match.
+If the table's column names conflict with existing columns, the existing columns will be dropped.
 
 
 `match_field`::
 `match_field`::
 The fields in the input to match against the table.
 The fields in the input to match against the table.

+ 3 - 1
docs/reference/esql/processing-commands/rename.asciidoc

@@ -17,7 +17,9 @@ RENAME old_name1 AS new_name1[, ..., old_nameN AS new_nameN]
 The name of a column you want to rename.
 The name of a column you want to rename.
 
 
 `new_nameX`::
 `new_nameX`::
-The new name of the column.
+The new name of the column. If it conflicts with an existing column name,
+the existing column is dropped. If multiple columns are renamed to the same
+name, all but the rightmost column with the same new name are dropped.
 
 
 *Description*
 *Description*
 
 

+ 3 - 0
docs/reference/esql/processing-commands/stats.asciidoc

@@ -18,12 +18,15 @@ STATS [column1 =] expression1[, ..., [columnN =] expressionN]
 `columnX`::
 `columnX`::
 The name by which the aggregated value is returned. If omitted, the name is
 The name by which the aggregated value is returned. If omitted, the name is
 equal to the corresponding expression (`expressionX`).
 equal to the corresponding expression (`expressionX`).
+If multiple columns have the same name, all but the rightmost column with this
+name will be ignored.
 
 
 `expressionX`::
 `expressionX`::
 An expression that computes an aggregated value.
 An expression that computes an aggregated value.
 
 
 `grouping_expressionX`::
 `grouping_expressionX`::
 An expression that outputs the values to group by.
 An expression that outputs the values to group by.
+If its name coincides with one of the computed columns, that column will be ignored.
 
 
 NOTE: Individual `null` values are skipped when computing aggregations.
 NOTE: Individual `null` values are skipped when computing aggregations.
 
 

+ 1 - 0
docs/reference/esql/source-commands/row.asciidoc

@@ -16,6 +16,7 @@ ROW column1 = value1[, ..., columnN = valueN]
 
 
 `columnX`::
 `columnX`::
 The column name.
 The column name.
+In case of duplicate column names, only the rightmost duplicate creates a column.
 
 
 `valueX`::
 `valueX`::
 The value for the column. Can be a literal, an expression, or a
 The value for the column. Can be a literal, an expression, or a

+ 3 - 2
x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/CsvTestsDataLoader.java

@@ -96,8 +96,8 @@ public class CsvTestsDataLoader {
         "cartesian_multipolygons.csv"
         "cartesian_multipolygons.csv"
     );
     );
     private static final TestsDataset DISTANCES = new TestsDataset("distances", "mapping-distances.json", "distances.csv");
     private static final TestsDataset DISTANCES = new TestsDataset("distances", "mapping-distances.json", "distances.csv");
-
     private static final TestsDataset K8S = new TestsDataset("k8s", "k8s-mappings.json", "k8s.csv", "k8s-settings.json", true);
     private static final TestsDataset K8S = new TestsDataset("k8s", "k8s-mappings.json", "k8s.csv", "k8s-settings.json", true);
+    private static final TestsDataset ADDRESSES = new TestsDataset("addresses", "mapping-addresses.json", "addresses.csv", null, true);
 
 
     public static final Map<String, TestsDataset> CSV_DATASET_MAP = Map.ofEntries(
     public static final Map<String, TestsDataset> CSV_DATASET_MAP = Map.ofEntries(
         Map.entry(EMPLOYEES.indexName, EMPLOYEES),
         Map.entry(EMPLOYEES.indexName, EMPLOYEES),
@@ -121,7 +121,8 @@ public class CsvTestsDataLoader {
         Map.entry(AIRPORT_CITY_BOUNDARIES.indexName, AIRPORT_CITY_BOUNDARIES),
         Map.entry(AIRPORT_CITY_BOUNDARIES.indexName, AIRPORT_CITY_BOUNDARIES),
         Map.entry(CARTESIAN_MULTIPOLYGONS.indexName, CARTESIAN_MULTIPOLYGONS),
         Map.entry(CARTESIAN_MULTIPOLYGONS.indexName, CARTESIAN_MULTIPOLYGONS),
         Map.entry(K8S.indexName, K8S),
         Map.entry(K8S.indexName, K8S),
-        Map.entry(DISTANCES.indexName, DISTANCES)
+        Map.entry(DISTANCES.indexName, DISTANCES),
+        Map.entry(ADDRESSES.indexName, ADDRESSES)
     );
     );
 
 
     private static final EnrichConfig LANGUAGES_ENRICH = new EnrichConfig("languages_policy", "enrich-policy-languages.json");
     private static final EnrichConfig LANGUAGES_ENRICH = new EnrichConfig("languages_policy", "enrich-policy-languages.json");

+ 4 - 0
x-pack/plugin/esql/qa/testFixtures/src/main/resources/addresses.csv

@@ -0,0 +1,4 @@
+street:keyword,number:keyword,zip_code:keyword,city.name:keyword,city.country.name:keyword,city.country.continent.name:keyword,city.country.continent.planet.name:keyword,city.country.continent.planet.galaxy:keyword
+Keizersgracht,281,1016 ED,Amsterdam,Netherlands,Europe,Earth,Milky Way
+Kearny St,88,CA 94108,San Francisco,United States of America,North America,Earth,Milky Way
+Marunouchi,2-7-2,100-7014,Tokyo,Japan,Asia,Earth,Milky Way

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

@@ -26,6 +26,19 @@ first_name:keyword | left:keyword | full_name:keyword | right:keyword | last_nam
 Georgi             | left         |    Georgi Facello | right         | Facello
 Georgi             | left         |    Georgi Facello | right         | Facello
 ;
 ;
 
 
+shadowingSubfields
+FROM addresses
+| KEEP city.country.continent.planet.name, city.country.name, city.name
+| DISSECT city.name "%{city.country.continent.planet.name} %{?}"
+| SORT city.name
+;
+
+city.country.name:keyword | city.name:keyword | city.country.continent.planet.name:keyword
+Netherlands               | Amsterdam         | null
+United States of America  | San Francisco     | San
+Japan                     | Tokyo             | null
+;
+
 shadowingSelf
 shadowingSelf
 FROM employees
 FROM employees
 | KEEP first_name, last_name
 | KEEP first_name, last_name
@@ -50,6 +63,18 @@ last_name:keyword | left:keyword | foo:keyword             | middle:keyword | ri
 Facello           | left         | Georgi1 Georgi2 Facello | middle         | right         | Georgi1      | Georgi2            | Facello
 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
 complexPattern
 ROW a = "1953-01-23T12:15:00Z - some text - 127.0.0.1;" 
 ROW a = "1953-01-23T12:15:00Z - some text - 127.0.0.1;" 

+ 17 - 0
x-pack/plugin/esql/qa/testFixtures/src/main/resources/docs.csv-spec

@@ -436,6 +436,23 @@ ROW a = "1.2.3.4 [2023-01-23T12:15:00.000Z] Connected"
 // end::grokWithEscape-result[]
 // end::grokWithEscape-result[]
 ;
 ;
 
 
+grokWithDuplicateFieldNames
+// tag::grokWithDuplicateFieldNames[]
+FROM addresses
+| KEEP city.name, zip_code
+| GROK zip_code "%{WORD:zip_parts} %{WORD:zip_parts}"
+// end::grokWithDuplicateFieldNames[]
+| SORT city.name
+;
+
+// tag::grokWithDuplicateFieldNames-result[]
+city.name:keyword | zip_code:keyword | zip_parts:keyword
+Amsterdam         | 1016 ED          | ["1016", "ED"]
+San Francisco     | CA 94108         | ["CA", "94108"]
+Tokyo             | 100-7014         | null
+// end::grokWithDuplicateFieldNames-result[]
+;
+
 basicDissect
 basicDissect
 // tag::basicDissect[]
 // tag::basicDissect[]
 ROW a = "2023-01-23T12:15:00.000Z - some text - 127.0.0.1" 
 ROW a = "2023-01-23T12:15:00.000Z - some text - 127.0.0.1" 

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

@@ -122,3 +122,53 @@ FROM employees | STATS COUNT(*), MIN(salary  * 10), MAX(languages)| DROP `COUNT(
 MIN(salary  * 10):i | MAX(languages):i
 MIN(salary  * 10):i | MAX(languages):i
 253240              | 5
 253240              | 5
 ;
 ;
+
+// Not really shadowing, but let's keep the name consistent with the other command's tests
+shadowingInternal
+FROM employees
+| SORT emp_no ASC
+| 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
+| SORT emp_no ASC
+| 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
+;
+
+subfields
+FROM addresses
+| DROP city.country.continent.planet.name, city.country.continent.name, city.country.name, number, street, zip_code, city.country.continent.planet.name
+| SORT city.name
+;
+
+city.country.continent.planet.galaxy:keyword | city.name:keyword
+Milky Way                                    | Amsterdam
+Milky Way                                    | San Francisco
+Milky Way                                    | Tokyo
+;
+
+subfieldsWildcard
+FROM addresses
+| DROP *.name, number, street, zip_code, *ame
+;
+
+city.country.continent.planet.galaxy:keyword
+Milky Way
+Milky Way
+Milky Way
+;

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

@@ -69,6 +69,34 @@ ROW left = "left", foo = "foo", client_ip = "172.21.0.5", env = "env", right = "
 left:keyword | client_ip:keyword | env:keyword | right:keyword | foo:keyword
 left:keyword | client_ip:keyword | env:keyword | right:keyword | foo:keyword
 ;
 ;
 
 
+shadowingSubfields
+required_capability: enrich_load
+FROM addresses
+| KEEP city.country.continent.planet.name, city.country.name, city.name
+| EVAL city.name = REPLACE(city.name, "San Francisco", "South San Francisco")
+| ENRICH city_names ON city.name WITH city.country.continent.planet.name = airport
+| SORT city.name
+;
+
+city.country.name:keyword | city.name:keyword   | city.country.continent.planet.name:text
+Netherlands               | Amsterdam           | null
+United States of America  | South San Francisco | San Francisco Int'l
+Japan                     | Tokyo               | null
+;
+
+shadowingSubfieldsLimit0
+required_capability: enrich_load
+FROM addresses
+| KEEP city.country.continent.planet.name, city.country.name, city.name
+| EVAL city.name = REPLACE(city.name, "San Francisco", "South San Francisco")
+| ENRICH city_names ON city.name WITH city.country.continent.planet.name = airport
+| SORT city.name
+| LIMIT 0
+;
+
+city.country.name:keyword | city.name:keyword | city.country.continent.planet.name:text
+;
+
 shadowingSelf
 shadowingSelf
 required_capability: enrich_load
 required_capability: enrich_load
 ROW left = "left", client_ip = "172.21.0.5", env = "env", right = "right"
 ROW left = "left", client_ip = "172.21.0.5", env = "env", right = "right"
@@ -107,6 +135,46 @@ 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
 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
+;
+
+shadowingInternalImplicit
+required_capability: enrich_load
+ROW city = "Zürich"
+| ENRICH city_names ON city WITH airport = region
+;
+
+city:keyword | airport:text
+Zürich       | Bezirk Zürich
+;
+
+shadowingInternalImplicit2
+required_capability: enrich_load
+ROW city = "Zürich"
+| ENRICH city_names ON city WITH airport, airport = region
+;
+
+city:keyword | airport:text
+Zürich       | Bezirk Zürich
+;
+
+shadowingInternalImplicit3
+required_capability: enrich_load
+ROW city = "Zürich"
+| ENRICH city_names ON city WITH airport = region, airport
+;
+
+city:keyword | airport:text
+Zürich       | Zurich Int'l
+;
+
 simple
 simple
 required_capability: enrich_load
 required_capability: enrich_load
 
 

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

@@ -15,6 +15,19 @@ left:keyword | right:keyword | x:integer
 left         | right         | 1
 left         | right         | 1
 ;
 ;
 
 
+shadowingSubfields
+FROM addresses
+| KEEP city.country.continent.planet.name, city.country.name, city.name
+| EVAL city.country.continent.planet.name = to_upper(city.country.continent.planet.name)
+| SORT city.name
+;
+
+city.country.name:keyword | city.name:keyword | city.country.continent.planet.name:keyword 
+Netherlands               | Amsterdam         | EARTH
+United States of America  | San Francisco     | EARTH
+Japan                     | Tokyo             | EARTH
+;
+
 shadowingSelf
 shadowingSelf
 ROW left = "left", x = 10000 , right = "right"
 ROW left = "left", x = 10000 , right = "right"
 | EVAL x = x + 1
 | EVAL x = x + 1
@@ -33,6 +46,16 @@ left:keyword | middle:keyword | right:keyword | x:integer | y:integer
 left         | middle         | right         | 9         | 10
 left         | middle         | right         | 9         | 10
 ;
 ;
 
 
+shadowingInternal
+ROW x = 10000
+| EVAL x = x + 1, x = x - 2
+;
+
+x:integer
+9999
+;
+
+
 withMath
 withMath
 row a = 1 | eval b = 2 + 3;
 row a = 1 | eval b = 2 + 3;
 
 

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

@@ -26,6 +26,19 @@ first_name:keyword | left:keyword | full_name:keyword | right:keyword | last_nam
 Georgi             | left         |    Georgi Facello | right         | Facello
 Georgi             | left         |    Georgi Facello | right         | Facello
 ;
 ;
 
 
+shadowingSubfields
+FROM addresses
+| KEEP city.country.continent.planet.name, city.country.name, city.name
+| GROK city.name "%{WORD:city.country.continent.planet.name} %{WORD}"
+| SORT city.name
+;
+
+city.country.name:keyword | city.name:keyword | city.country.continent.planet.name:keyword
+Netherlands               | Amsterdam         | null
+United States of America  | San Francisco     | San
+Japan                     | Tokyo             | null
+;
+
 shadowingSelf
 shadowingSelf
 FROM employees
 FROM employees
 | KEEP first_name, last_name
 | KEEP first_name, last_name
@@ -50,6 +63,18 @@ last_name:keyword | left:keyword | foo:keyword             | middle:keyword | ri
 Facello           | left         | Georgi1 Georgi2 Facello | middle         | right         | Georgi1      | Georgi2            | Facello
 Facello           | left         | Georgi1 Georgi2 Facello | middle         | right         | Georgi1      | Georgi2            | Facello
 ;
 ;
 
 
+shadowingInternal
+FROM addresses
+| KEEP city.name, zip_code
+| GROK zip_code "%{WORD:zip_parts} %{WORD:zip_parts}"
+| SORT city.name
+;
+
+city.name:keyword | zip_code:keyword | zip_parts:keyword
+Amsterdam         | 1016 ED          | ["1016", "ED"]
+San Francisco     | CA 94108         | ["CA", "94108"]
+Tokyo             | 100-7014         | null
+;
 
 
 complexPattern
 complexPattern
 ROW a = "1953-01-23T12:15:00Z 127.0.0.1 some.email@foo.com 42" 
 ROW a = "1953-01-23T12:15:00Z 127.0.0.1 some.email@foo.com 42" 

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

@@ -539,3 +539,63 @@ c:i
 1
 1
 1
 1
 ;
 ;
+
+shadowingInternal
+FROM employees
+| SORT emp_no ASC
+| KEEP last_name, emp_no, last_name
+| LIMIT 2
+;
+
+emp_no:integer | last_name:keyword
+         10001 | Facello
+         10002 | Simmel
+;
+
+shadowingInternalWildcard
+FROM employees
+| SORT emp_no ASC
+| KEEP last*name, emp_no, last*name, first_name, last*, gender, last*
+| LIMIT 2
+;
+
+emp_no:integer | first_name:keyword | gender:keyword | last_name:keyword
+         10001 | Georgi             | M              | Facello
+         10002 | Bezalel            | F              | Simmel
+;
+
+shadowingInternalWildcardAndExplicit
+FROM employees
+| SORT emp_no ASC
+| KEEP last*name, emp_no, last_name, first_name, last*, languages, last_name, gender, last*name
+| LIMIT 2
+;
+
+emp_no:integer | first_name:keyword | languages:integer | last_name:keyword | gender:keyword
+         10001 | Georgi             | 2                 | Facello           | M
+         10002 | Bezalel            | 5                 | Simmel            | F
+;
+
+shadowingSubfields
+FROM addresses
+| KEEP city.country.continent.planet.name, city.country.continent.name, city.country.name, city.name, city.country.continent.planet.name
+| SORT city.name
+;
+
+city.country.continent.name:keyword | city.country.name:keyword | city.name:keyword | city.country.continent.planet.name:keyword
+Europe                              | Netherlands               | Amsterdam         | Earth
+North America                       | United States of America  | San Francisco     | Earth
+Asia                                | Japan                     | Tokyo             | Earth
+;
+
+shadowingSubfieldsWildcard
+FROM addresses
+| KEEP *name, city.country.continent.planet.name
+| SORT city.name
+;
+
+city.country.continent.name:keyword | city.country.name:keyword | city.name:keyword | city.country.continent.planet.name:keyword
+Europe                              | Netherlands               | Amsterdam         | Earth
+North America                       | United States of America  | San Francisco     | Earth
+Asia                                | Japan                     | Tokyo             | Earth
+;

+ 44 - 0
x-pack/plugin/esql/qa/testFixtures/src/main/resources/mapping-addresses.json

@@ -0,0 +1,44 @@
+{
+    "properties" : {
+        "street" : {
+            "type": "keyword"
+        },
+        "number" : {
+            "type": "keyword"
+        },
+        "zip_code": {
+            "type": "keyword"
+        },
+        "city" : {
+            "properties": {
+                "name": {
+                    "type": "keyword"
+                },
+                "country": {
+                    "properties": {
+                        "name": {
+                            "type": "keyword"
+                        },
+                        "continent": {
+                            "properties": {
+                                "name": {
+                                    "type": "keyword"
+                                },
+                                "planet": {
+                                    "properties": {
+                                        "name": {
+                                            "type": "keyword"
+                                        },
+                                        "galaxy": {
+                                            "type": "keyword"
+                                        }
+                                    }
+                                }
+                            }
+                        }
+                    }
+                }
+            }
+        }
+    }
+}

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

@@ -174,3 +174,42 @@ 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               
 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               
 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
+;
+
+shadowingSubfields
+FROM addresses
+| KEEP city.country.continent.planet.name, city.country.continent.name, city.country.name, city.name
+| RENAME city.name AS city.country.continent.planet.name, city.country.name AS city.country.continent.name
+| SORT city.country.continent.planet.name
+;
+
+city.country.continent.name:keyword | city.country.continent.planet.name:keyword
+Netherlands                         | Amsterdam
+United States of America            | San Francisco
+Japan                               | Tokyo
+;
+
+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
+;

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

@@ -36,6 +36,24 @@ a:integer
 // end::multivalue-result[]
 // end::multivalue-result[]
 ;
 ;
 
 
+shadowingInternal
+required_capability: unique_names
+ROW a = 1, a = 2;
+
+a:integer
+        2
+;
+
+shadowingInternalSubfields
+required_capability: unique_names
+// Fun fact: "Sissi" is an actual exoplanet name, after the character from the movie with the same name. A.k.a. HAT-P-14 b.
+ROW city.country.continent.planet.name = "Earth", city.country.continent.name = "Netherlands", city.country.continent.planet.name = "Sissi"
+;
+
+city.country.continent.name:keyword | city.country.continent.planet.name:keyword
+Netherlands                         | Sissi
+;
+
 unsignedLongLiteral
 unsignedLongLiteral
 ROW long_max = 9223372036854775807, ul_start = 9223372036854775808, ul_end = 18446744073709551615, double=18446744073709551616;
 ROW long_max = 9223372036854775807, ul_start = 9223372036854775808, ul_end = 18446744073709551615, double=18446744073709551616;
 
 
@@ -70,10 +88,11 @@ a:integer | b:integer | c:null | z:integer
 ;
 ;
 
 
 evalRowWithNull2
 evalRowWithNull2
+required_capability: unique_names
 row a = 1, null, b = 2, c = null, null | eval z = a+b;
 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
 evalRowWithNull3

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

@@ -1886,6 +1886,39 @@ w_avg:double
 null
 null
 ;
 ;
 
 
+shadowingInternal
+FROM employees
+| STATS x = MAX(emp_no), x = MIN(emp_no)
+;
+
+x:integer
+10001
+;
+
+shadowingInternalWithGroup
+FROM employees
+| STATS x = MAX(emp_no), x = MIN(emp_no) BY x = gender
+| SORT x ASC
+;
+
+x:keyword
+F
+M
+null
+;
+
+shadowingTheGroup
+FROM employees
+| STATS gender = MAX(emp_no), gender = MIN(emp_no) BY gender
+| SORT gender ASC
+;
+
+gender:keyword
+F
+M
+null
+;
+
 docsStatsMvGroup
 docsStatsMvGroup
 // tag::mv-group[]
 // tag::mv-group[]
 ROW i=1, a=["a", "b"] | STATS MIN(i) BY a | SORT a ASC
 ROW i=1, a=["a", "b"] | STATS MIN(i) BY a | SORT a ASC

+ 9 - 3
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java

@@ -152,14 +152,20 @@ public class EsqlCapabilities {
         FIX_COUNT_DISTINCT_SOURCE_ERROR,
         FIX_COUNT_DISTINCT_SOURCE_ERROR,
 
 
         /**
         /**
-        * Use RangeQuery for BinaryComparison on DateTime fields.
-        * */
+         * Use RangeQuery for BinaryComparison on DateTime fields.
+         */
         RANGEQUERY_FOR_DATETIME,
         RANGEQUERY_FOR_DATETIME,
 
 
         /**
         /**
          * Add tests for #105383, STATS BY constant.
          * Add tests for #105383, STATS BY constant.
          */
          */
-        STATS_BY_CONSTANT;
+        STATS_BY_CONSTANT,
+
+        /**
+         * Fix for non-unique attribute names in ROW and logical plans.
+         * https://github.com/elastic/elasticsearch/issues/110541
+         */
+        UNIQUE_NAMES;
 
 
         private final boolean snapshotOnly;
         private final boolean snapshotOnly;
 
 

+ 20 - 4
x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java

@@ -1068,13 +1068,29 @@ public class Analyzer extends ParameterizedRuleExecutor<LogicalPlan, AnalyzerCon
      * Any fields which could not be resolved by conversion functions will be converted to UnresolvedAttribute instances in a later rule
      * Any fields which could not be resolved by conversion functions will be converted to UnresolvedAttribute instances in a later rule
      * (See UnresolveUnionTypes below).
      * (See UnresolveUnionTypes below).
      */
      */
-    private static class ResolveUnionTypes extends BaseAnalyzerRule {
+    private static class ResolveUnionTypes extends Rule<LogicalPlan, LogicalPlan> {
 
 
         record TypeResolutionKey(String fieldName, DataType fieldType) {}
         record TypeResolutionKey(String fieldName, DataType fieldType) {}
 
 
+        private List<FieldAttribute> unionFieldAttributes;
+
         @Override
         @Override
-        protected LogicalPlan doRule(LogicalPlan plan) {
-            List<FieldAttribute> unionFieldAttributes = new ArrayList<>();
+        public LogicalPlan apply(LogicalPlan plan) {
+            unionFieldAttributes = new ArrayList<>();
+            // Collect field attributes from previous runs
+            plan.forEachUp(EsRelation.class, rel -> {
+                for (Attribute attr : rel.output()) {
+                    if (attr instanceof FieldAttribute fa && fa.field() instanceof MultiTypeEsField) {
+                        unionFieldAttributes.add(fa);
+                    }
+                }
+            });
+
+            return plan.transformUp(LogicalPlan.class, p -> p.resolved() || p.childrenResolved() == false ? p : doRule(p));
+        }
+
+        private LogicalPlan doRule(LogicalPlan plan) {
+            int alreadyAddedUnionFieldAttributes = unionFieldAttributes.size();
             // See if the eval function has an unresolved MultiTypeEsField field
             // See if the eval function has an unresolved MultiTypeEsField field
             // Replace the entire convert function with a new FieldAttribute (containing type conversion knowledge)
             // Replace the entire convert function with a new FieldAttribute (containing type conversion knowledge)
             plan = plan.transformExpressionsOnly(
             plan = plan.transformExpressionsOnly(
@@ -1082,7 +1098,7 @@ public class Analyzer extends ParameterizedRuleExecutor<LogicalPlan, AnalyzerCon
                 convert -> resolveConvertFunction(convert, unionFieldAttributes)
                 convert -> resolveConvertFunction(convert, unionFieldAttributes)
             );
             );
             // If no union fields were generated, return the plan as is
             // If no union fields were generated, return the plan as is
-            if (unionFieldAttributes.isEmpty()) {
+            if (unionFieldAttributes.size() == alreadyAddedUnionFieldAttributes) {
                 return plan;
                 return plan;
             }
             }
 
 

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

@@ -20,8 +20,6 @@ import java.util.Objects;
 import java.util.function.Predicate;
 import java.util.function.Predicate;
 import java.util.function.Supplier;
 import java.util.function.Supplier;
 
 
-import static java.util.Collections.singletonList;
-
 public final class AnalyzerRules {
 public final class AnalyzerRules {
 
 
     public abstract static class AnalyzerRule<SubPlan extends LogicalPlan> extends Rule<SubPlan, LogicalPlan> {
     public abstract static class AnalyzerRule<SubPlan extends LogicalPlan> extends Rule<SubPlan, LogicalPlan> {
@@ -138,14 +136,6 @@ public final class AnalyzerRules {
             )
             )
             .toList();
             .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);
     }
     }
 }
 }

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

@@ -8,8 +8,10 @@
 package org.elasticsearch.xpack.esql.optimizer;
 package org.elasticsearch.xpack.esql.optimizer;
 
 
 import org.elasticsearch.xpack.esql.common.Failures;
 import org.elasticsearch.xpack.esql.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.AttributeSet;
 import org.elasticsearch.xpack.esql.core.expression.Expressions;
 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.core.plan.QueryPlan;
 import org.elasticsearch.xpack.esql.plan.logical.Aggregate;
 import org.elasticsearch.xpack.esql.plan.logical.Aggregate;
 import org.elasticsearch.xpack.esql.plan.logical.Enrich;
 import org.elasticsearch.xpack.esql.plan.logical.Enrich;
@@ -36,6 +38,9 @@ import org.elasticsearch.xpack.esql.plan.physical.RegexExtractExec;
 import org.elasticsearch.xpack.esql.plan.physical.RowExec;
 import org.elasticsearch.xpack.esql.plan.physical.RowExec;
 import org.elasticsearch.xpack.esql.plan.physical.ShowExec;
 import org.elasticsearch.xpack.esql.plan.physical.ShowExec;
 
 
+import java.util.HashSet;
+import java.util.Set;
+
 import static org.elasticsearch.xpack.esql.common.Failure.fail;
 import static org.elasticsearch.xpack.esql.common.Failure.fail;
 
 
 class OptimizerRules {
 class OptimizerRules {
@@ -49,9 +54,24 @@ class OptimizerRules {
             AttributeSet input = p.inputSet();
             AttributeSet input = p.inputSet();
             AttributeSet generated = generates(p);
             AttributeSet generated = generates(p);
             AttributeSet missing = refs.subtract(input).subtract(generated);
             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));
                 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) {
         protected AttributeSet references(P p) {

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

@@ -73,6 +73,7 @@ import static org.elasticsearch.common.logging.HeaderWarning.addWarning;
 import static org.elasticsearch.xpack.esql.core.parser.ParserUtils.source;
 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.typedParsing;
 import static org.elasticsearch.xpack.esql.core.parser.ParserUtils.visitList;
 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.plan.logical.Enrich.Mode;
 import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.stringToInt;
 import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.stringToInt;
 
 
@@ -234,8 +235,9 @@ public class LogicalPlanBuilder extends ExpressionBuilder {
     }
     }
 
 
     @Override
     @Override
+    @SuppressWarnings("unchecked")
     public LogicalPlan visitRowCommand(EsqlBaseParser.RowCommandContext ctx) {
     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
     @Override

+ 7 - 0
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;
 package org.elasticsearch.xpack.esql.plan.logical;
 
 
 import org.elasticsearch.xpack.esql.core.expression.Alias;
 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.NodeInfo;
 import org.elasticsearch.xpack.esql.core.tree.Source;
 import org.elasticsearch.xpack.esql.core.tree.Source;
 import org.elasticsearch.xpack.esql.expression.function.UnsupportedAttribute;
 import org.elasticsearch.xpack.esql.expression.function.UnsupportedAttribute;
@@ -28,6 +29,12 @@ public class Rename extends UnaryPlan {
         return 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
     @Override
     public boolean expressionsResolved() {
     public boolean expressionsResolved() {
         for (var alias : renamings) {
         for (var alias : renamings) {