Update logic and tests to conform to the new grammar

Author: pawankartik-elastic

x-pack/plugin/esql/src/internalClusterTest/java/org/elasticsearch/xpack/esql/action/EsqlActionIT.java

@@ -1071,8 +1071,6 @@ public void testDataStreamPatterns() throws Exception {
         testCases.put("test_ds_patterns*::data,test_ds_patterns*::failures,-test_ds_patterns_2*::data", 19L);
         testCases.put("test_ds_patterns*::data,test_ds_patterns*::failures,-test_ds_patterns_2*::failures", 21L);
 
-        testCases.put("\"test_ds_patterns_1,test_ds_patterns_2\"::failures", 8L);
-
         runDataStreamTest(testCases, new String[] { "test_ds_patterns_1", "test_ds_patterns_2", "test_ds_patterns_3" }, (key, value) -> {
             try (var results = run("from " + key + " | stats count(@timestamp)")) {
                 assertEquals(key, 1, getValuesList(results).size());
@@ -1097,7 +1095,7 @@ public void testDataStreamInvalidPatterns() throws Exception {
         // Only one selector separator is allowed per expression
         testCases.put("::::data", "mismatched input '::' expecting {QUOTED_STRING, UNQUOTED_SOURCE}");
         // Suffix case is not supported because there is no component named with the empty string
-        testCases.put("index::", "missing {QUOTED_STRING, UNQUOTED_SOURCE} at '|'");
+        testCases.put("index::", "missing UNQUOTED_SOURCE at '|'");
 
         runDataStreamTest(testCases, new String[] { "test_ds_patterns_1" }, (key, value) -> {
             logger.info(key);

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

@@ -58,10 +58,8 @@ protected static String quoteIdString(String unquotedString) {
     public String visitClusterString(EsqlBaseParser.ClusterStringContext ctx) {
         if (ctx == null) {
             return null;
-        } else if (ctx.UNQUOTED_SOURCE() != null) {
-            return ctx.UNQUOTED_SOURCE().getText();
         } else {
-            return unquote(ctx.QUOTED_STRING().getText());
+            return ctx.UNQUOTED_SOURCE().getText();
         }
     }
 
@@ -78,18 +76,16 @@ public String visitIndexString(IndexStringContext ctx) {
     public String visitSelectorString(EsqlBaseParser.SelectorStringContext ctx) {
         if (ctx == null) {
             return null;
-        } else if (ctx.UNQUOTED_SOURCE() != null) {
-            return ctx.UNQUOTED_SOURCE().getText();
         } else {
-            return unquote(ctx.QUOTED_STRING().getText());
+            return ctx.UNQUOTED_SOURCE().getText();
         }
     }
 
     public String visitIndexPattern(List<EsqlBaseParser.IndexPatternContext> ctx) {
         List<String> patterns = new ArrayList<>(ctx.size());
         Holder<Boolean> hasSeenStar = new Holder<>(false);
         ctx.forEach(c -> {
-            String indexPattern = visitIndexString(c.indexString());
+            String indexPattern = c.unquotedIndexString() != null ? c.unquotedIndexString().getText() : visitIndexString(c.indexString());
             String clusterString = visitClusterString(c.clusterString());
             String selectorString = visitSelectorString(c.selectorString());
             // skip validating index on remote cluster, because the behavior of remote cluster is not consistent with local cluster

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

@@ -80,18 +80,23 @@ public static String randomIndexPattern(Feature... features) {
             index.insert(0, "-");
         }
 
+        boolean requiresQuote = false;
         var pattern = index.toString();
         if (pattern.contains("|")) {
-            pattern = quote(pattern);
+            requiresQuote = true;
         }
-        pattern = maybeQuote(pattern);
 
         if (canAdd(Features.CROSS_CLUSTER, features)) {
-            var cluster = maybeQuote(randomIdentifier());
-            pattern = maybeQuote(cluster + ":" + pattern);
+            var cluster = randomIdentifier();
+            pattern = cluster + ":" + pattern;
         } else if (EsqlCapabilities.Cap.INDEX_COMPONENT_SELECTORS.isEnabled() && canAdd(Features.INDEX_SELECTOR, features)) {
-            pattern = maybeQuote(pattern + "::" + randomFrom(IndexComponentSelector.values()).getKey());
+            pattern = pattern + "::" + randomFrom(IndexComponentSelector.values()).getKey();
         }
+
+        if (requiresQuote) {
+            pattern = quote(pattern);
+        }
+
         return pattern;
     }
 

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

@@ -483,7 +483,13 @@ public void testStringAsIndexPattern() {
             assertStringAsIndexPattern("`backtick`,``multiple`back``ticks```", command + " `backtick`, ``multiple`back``ticks```");
             assertStringAsIndexPattern("test,metadata,metaata,.metadata", command + " test,\"metadata\", metaata, .metadata");
             assertStringAsIndexPattern(".dot", command + " .dot");
-            assertStringAsIndexPattern("cluster:index|pattern", command + " cluster:\"index|pattern\"");
+            String lineNumber = command.equals("FROM") ? "line 1:14: " : "line 1:12: ";
+            expectErrorWithLineNumber(
+                command + " cluster:\"index|pattern\"",
+                " cluster:\"index|pattern\"",
+                lineNumber,
+                "mismatched input '\"index|pattern\"' expecting UNQUOTED_SOURCE"
+            );
             assertStringAsIndexPattern("*:index|pattern", command + " \"*:index|pattern\"");
             clusterAndIndexAsIndexPattern(command, "cluster:index");
             clusterAndIndexAsIndexPattern(command, "cluster:.index");
@@ -496,13 +502,37 @@ public void testStringAsIndexPattern() {
             if (EsqlCapabilities.Cap.INDEX_COMPONENT_SELECTORS.isEnabled()) {
                 assertStringAsIndexPattern("foo::data", command + " foo::data");
                 assertStringAsIndexPattern("foo::failures", command + " foo::failures");
-                assertStringAsIndexPattern("cluster:foo::failures", command + " cluster:\"foo::failures\"");
-                assertStringAsIndexPattern("*,-foo::data", command + " *, \"-foo\"::data");
+                expectErrorWithLineNumber(
+                    command + " cluster:\"foo::data\"",
+                    " cluster:\"foo::data\"",
+                    lineNumber,
+                    "mismatched input '\"foo::data\"' expecting UNQUOTED_SOURCE"
+                );
+                expectErrorWithLineNumber(
+                    command + " cluster:\"foo::failures\"",
+                    " cluster:\"foo::failures\"",
+                    lineNumber,
+                    "mismatched input '\"foo::failures\"' expecting UNQUOTED_SOURCE"
+                );
+                lineNumber = command.equals("FROM") ? "line 1:15: " : "line 1:13: ";
+                expectErrorWithLineNumber(
+                    command + " *, \"-foo\"::data",
+                    " *, \"-foo\"::data",
+                    lineNumber,
+                    "mismatched input '::' expecting {<EOF>, '|', ',', 'metadata'}"
+                );
                 assertStringAsIndexPattern("*,-foo::data", command + " *, \"-foo::data\"");
                 assertStringAsIndexPattern("*::data", command + " *::data");
+                lineNumber = command.equals("FROM") ? "line 1:79: " : "line 1:77: ";
+                expectErrorWithLineNumber(
+                    command + " \"<logstash-{now/M{yyyy.MM}}>::data,<logstash-{now/d{yyyy.MM.dd|+12:00}}>\"::failures",
+                    " \"<logstash-{now/M{yyyy.MM}}>::data,<logstash-{now/d{yyyy.MM.dd|+12:00}}>\"::failures",
+                    lineNumber,
+                    "mismatched input '::' expecting {<EOF>, '|', ',', 'metadata'}"
+                );
                 assertStringAsIndexPattern(
                     "<logstash-{now/M{yyyy.MM}}>::data,<logstash-{now/d{yyyy.MM.dd|+12:00}}>::failures",
-                    command + " <logstash-{now/M{yyyy.MM}}>::data, \"<logstash-{now/d{yyyy.MM.dd|+12:00}}>\"::failures"
+                    command + " <logstash-{now/M{yyyy.MM}}>::data, \"<logstash-{now/d{yyyy.MM.dd|+12:00}}>::failures\""
                 );
             }
         }
@@ -595,8 +625,19 @@ public void testInvalidCharacterInIndexPattern() {
                 expectDoubleColonErrorWithLineNumber(command, "cluster:foo::data", parseLineNumber + 11);
                 expectDoubleColonErrorWithLineNumber(command, "cluster:foo::failures", parseLineNumber + 11);
 
-                expectDoubleColonErrorWithLineNumber(command, "cluster:\"foo\"::data", parseLineNumber + 13);
-                expectDoubleColonErrorWithLineNumber(command, "cluster:\"foo\"::failures", parseLineNumber + 13);
+                // Index pattern cannot be quoted if cluster string is present.
+                expectErrorWithLineNumber(
+                    command,
+                    "cluster:\"foo\"::data",
+                    "line 1:14: ",
+                    "mismatched input '\"foo\"' expecting UNQUOTED_SOURCE"
+                );
+                expectErrorWithLineNumber(
+                    command,
+                    "cluster:\"foo\"::failures",
+                    "line 1:14: ",
+                    "mismatched input '\"foo\"' expecting UNQUOTED_SOURCE"
+                );
 
                 // TODO: Edge case that will be invalidated in follow up (https://github.com/elastic/elasticsearch/issues/122651)
                 // expectDoubleColonErrorWithLineNumber(command, "\"cluster:foo\"::data", parseLineNumber + 13);
@@ -641,8 +682,12 @@ public void testInvalidCharacterInIndexPattern() {
                     "Invalid usage of :: separator"
                 );
 
-                // TODO: Edge case that will be invalidated in follow up (https://github.com/elastic/elasticsearch/issues/122651)
-                expectDoubleColonErrorWithLineNumber(command, "cluster:\"index,index2\"::failures", parseLineNumber + 22);
+                expectErrorWithLineNumber(
+                    command,
+                    "cluster:\"index,index2\"::failures",
+                    "line 1:14: ",
+                    "mismatched input '\"index,index2\"' expecting UNQUOTED_SOURCE"
+                );
             }
         }
 
@@ -669,12 +714,11 @@ public void testInvalidCharacterInIndexPattern() {
                     "-index",
                     "must not start with '_', '-', or '+'"
                 );
-                expectInvalidIndexNameErrorWithLineNumber(
+                expectErrorWithLineNumber(
                     command,
                     "indexpattern, \"--index\"::data",
-                    lineNumber,
-                    "-index",
-                    "must not start with '_', '-', or '+'"
+                    "line 1:29: ",
+                    "mismatched input '::' expecting {<EOF>, '|', ',', 'metadata'}"
                 );
                 expectInvalidIndexNameErrorWithLineNumber(
                     command,
@@ -709,7 +753,12 @@ public void testInvalidCharacterInIndexPattern() {
                 clustersAndIndices(command, "*", "-<--logstash-{now/M{yyyy.MM}}>::data");
                 clustersAndIndices(command, "index*", "-<--logstash#-{now/M{yyyy.MM}}>::data");
                 // Throw on invalid date math
-                expectDateMathErrorWithLineNumber(command, "*, \"-<-logstash-{now/D}>\"::data", lineNumber, dateMathError);
+                expectDateMathErrorWithLineNumber(
+                    command,
+                    "*, \"-<-logstash-{now/D}>\"::data",
+                    "line 1:31: ",
+                    "mismatched input '::' expecting {<EOF>, '|', ',', 'metadata'}"
+                );
                 expectDateMathErrorWithLineNumber(command, "*, -<-logstash-{now/D}>::data", lineNumber, dateMathError);
                 // Check that invalid selectors throw (they're resolved first in /_search, and always validated)
                 expectInvalidIndexNameErrorWithLineNumber(
@@ -2450,7 +2499,7 @@ public void testInvalidAlias() {
     }
 
     public void testInvalidRemoteClusterPattern() {
-        expectError("from \"rem:ote\":index", "cluster string [rem:ote] must not contain ':'");
+        expectError("from \"rem:ote\":index", "mismatched input ':' expecting {<EOF>, '|', ',', 'metadata'}");
     }
 
     private LogicalPlan unresolvedRelation(String index) {
@@ -3174,7 +3223,7 @@ public void testInvalidJoinPatterns() {
                 joinPattern = unquoteIndexPattern(joinPattern);
                 expectError(
                     "FROM " + randomIndexPatterns(without(CROSS_CLUSTER)) + " | LOOKUP JOIN " + joinPattern + " ON " + randomIdentifier(),
-                    "extraneous input ':' expecting {QUOTED_STRING, UNQUOTED_SOURCE}"
+                    "extraneous input ':' expecting UNQUOTED_SOURCE"
                 );
             }
             {