ES|QL: Check if cluster aliases and index patterns are valid before executing query

docs/changelog/122497.yaml

@@ -0,0 +1,5 @@
+pr: 122497
+summary: Check if index patterns conform to valid format before validation
+area: CCS
+type: bug
+issues: []

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

@@ -15,16 +15,17 @@
 import org.elasticsearch.core.Tuple;
 import org.elasticsearch.indices.InvalidIndexNameException;
 import org.elasticsearch.transport.RemoteClusterService;
-import org.elasticsearch.xpack.esql.core.util.Holder;
 import org.elasticsearch.xpack.esql.parser.EsqlBaseParser.IdentifierContext;
 import org.elasticsearch.xpack.esql.parser.EsqlBaseParser.IndexStringContext;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.List;
 
 import static org.elasticsearch.cluster.metadata.IndexNameExpressionResolver.SelectorResolver.SELECTOR_SEPARATOR;
 import static org.elasticsearch.transport.RemoteClusterAware.REMOTE_CLUSTER_INDEX_SEPARATOR;
 import static org.elasticsearch.transport.RemoteClusterAware.isRemoteIndexName;
+import static org.elasticsearch.transport.RemoteClusterAware.splitIndexName;
 import static org.elasticsearch.xpack.esql.core.util.StringUtils.EXCLUSION;
 import static org.elasticsearch.xpack.esql.core.util.StringUtils.WILDCARD;
 import static org.elasticsearch.xpack.esql.parser.ParserUtils.source;
@@ -87,33 +88,12 @@ public String visitSelectorString(EsqlBaseParser.SelectorStringContext ctx) {
 
     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 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
-            // For example, invalid#index is an invalid index name, however FROM *:invalid#index does not return an error
-            if (clusterString == null) {
-                hasSeenStar.set(indexPattern.contains(WILDCARD) || hasSeenStar.get());
-                validateIndexPattern(indexPattern, c, hasSeenStar.get());
-                // Other instances of Elasticsearch may have differing selectors so only validate selector string if remote cluster
-                // string is unset
-                if (selectorString != null) {
-                    try {
-                        // Ensures that the selector provided is one of the valid kinds
-                        IndexNameExpressionResolver.SelectorResolver.validateIndexSelectorString(indexPattern, selectorString);
-                    } catch (InvalidIndexNameException e) {
-                        throw new ParsingException(e, source(c), e.getMessage());
-                    }
-                }
-            } else {
-                validateClusterString(clusterString, c);
-                // Do not allow selectors on remote cluster expressions until they are supported
-                if (selectorString != null) {
-                    throwOnMixingSelectorWithCluster(reassembleIndexName(clusterString, indexPattern, selectorString), c);
-                }
-            }
+
+            validateClusterAndIndexPatterns(indexPattern, c, clusterString, selectorString);
             patterns.add(reassembleIndexName(clusterString, indexPattern, selectorString));
         });
         return Strings.collectionToDelimitedString(patterns, ",");
@@ -148,35 +128,109 @@ protected static void validateClusterString(String clusterString, EsqlBaseParser
         }
     }
 
-    private static void validateIndexPattern(String indexPattern, EsqlBaseParser.IndexPatternContext ctx, boolean hasSeenStar) {
+    private static void validateClusterAndIndexPatterns(
+        String indexPattern,
+        EsqlBaseParser.IndexPatternContext ctx,
+        String clusterString,
+        String selectorString
+    ) {
         // multiple index names can be in the same double quote, e.g. indexPattern = "idx1, *, -idx2"
-        String[] indices = indexPattern.split(",");
-        boolean hasExclusion = false;
+        String[] patterns = indexPattern.split(",");
+        boolean isFirstPattern = true;
+
+        for (String pattern : patterns) {
+            pattern = pattern.strip();
+            String[] indices = new String[] { pattern };
+
+            /*
+             * Just because there was no clusterString before this index pattern does not mean that the indices
+             * are local indices. Patterns can be clubbed with remote names within quotes such as:
+             * "remote_one:remote_index,local_index". In this case, clusterString will be null.
+             */
+            if (isRemoteIndexName(pattern)) {
+                /*
+                 * Handle scenarios like remote_one:"index1,remote_two:index2". The clusterString here is
+                 * remote_one and is associated with index1 and not index2.
+                 */
+                if (clusterString != null && isFirstPattern) {
+                    throw new ParsingException(
+                        source(ctx),
+                        "Index pattern [{}] contains a cluster alias despite specifying one [{}]",
+                        pattern,
+                        clusterString
+                    );
+                }
+
+                // {cluster_alias, indexName}
+                String[] clusterAliasAndIndex = splitIndexName(pattern);
+                clusterString = clusterAliasAndIndex[0];
+                indices[0] = clusterAliasAndIndex[1];
+
+                /*
+                 * What if the pattern is index1|index2? We cannot split at the pipe char blindly as that'd mess with
+                 * logstash-like examples. We only split this way if an index is associated with a remote cluster.
+                 */
+                indices = Arrays.stream(indices)
+                    .map(IdentifierBuilder::breakPatternIntoIndices)
+                    .flatMap(Arrays::stream)
+                    .toArray(String[]::new);
+            } else if (clusterString != null) {
+                // This is not a remote index pattern and the cluster string preceding this quoted pattern
+                // cannot be associated with it.
+                if (isFirstPattern == false) {
+                    clusterString = null;
+                }
+
+                // Cluster alias was prefixed to the pattern and did not occur within the pattern.
+                indices = Arrays.stream(indices)
+                    .map(IdentifierBuilder::breakPatternIntoIndices)
+                    .flatMap(Arrays::stream)
+                    .toArray(String[]::new);
+            }
+
+            if (clusterString != null) {
+                if (selectorString != null) {
+                    throwOnMixingSelectorWithCluster(reassembleIndexName(clusterString, indexPattern, selectorString), ctx);
+                }
+                validateClusterString(clusterString, ctx);
+            }
+
+            validateIndicesForCluster(clusterString, indices, ctx);
+            if (selectorString != null) {
+                try {
+                    // Ensures that the selector provided is one of the valid kinds
+                    IndexNameExpressionResolver.SelectorResolver.validateIndexSelectorString(indexPattern, selectorString);
+                } catch (InvalidIndexNameException e) {
+                    throw new ParsingException(e, source(ctx), e.getMessage());
+                }
+            }
+
+            isFirstPattern = false;
+        }
+    }
+
+    private static void validateIndicesForCluster(String clusterString, String[] indices, EsqlBaseParser.IndexPatternContext ctx) {
         for (String index : indices) {
             // Strip spaces off first because validation checks are not written to handle them
             index = index.strip();
-            if (isRemoteIndexName(index)) { // skip the validation if there is remote cluster
-                // Ensure that there are no selectors as they are not yet supported
-                if (index.contains(SELECTOR_SEPARATOR)) {
-                    throwOnMixingSelectorWithCluster(index, ctx);
-                }
-                continue;
-            }
+
             try {
                 Tuple<String, String> splitPattern = IndexNameExpressionResolver.splitSelectorExpression(index);
-                if (splitPattern.v2() != null) {
-                    index = splitPattern.v1();
+                if (splitPattern.v2() != null && clusterString != null) {
+                    throwOnMixingSelectorWithCluster(reassembleIndexName(clusterString, splitPattern.v1(), splitPattern.v2()), ctx);
                 }
+
+                index = splitPattern.v1();
             } catch (InvalidIndexNameException e) {
                 // throws exception if the selector expression is invalid. Selector resolution does not complain about exclusions
                 throw new ParsingException(e, source(ctx), e.getMessage());
             }
-            hasSeenStar = index.contains(WILDCARD) || hasSeenStar;
+            var hasSeenStar = index.contains(WILDCARD);
             index = index.replace(WILDCARD, "").strip();
             if (index.isBlank()) {
                 continue;
             }
-            hasExclusion = index.startsWith(EXCLUSION);
+            var hasExclusion = index.startsWith(EXCLUSION);
             index = removeExclusion(index);
             String tempName;
             try {
@@ -198,9 +252,44 @@ private static void validateIndexPattern(String indexPattern, EsqlBaseParser.Ind
                 }
                 throw new ParsingException(e, source(ctx), e.getMessage());
             }
+
         }
     }
 
+    private static String[] breakPatternIntoIndices(String pattern) {
+        if (pattern.codePoints().anyMatch(ch -> ch == ',')) {
+            throw new IllegalArgumentException("Found grouped index patterns, expecting a single pattern");
+        }
+
+        // Fast path: if there's no pipe char, no point in attempting to break down the string.
+        if (pattern.contains("|") == false) {
+            return new String[] { pattern };
+        }
+
+        var indices = new ArrayList<String>();
+        var sb = new StringBuilder();
+        var inDateMathExpr = false;
+        for (int i = 0; i < pattern.length(); i++) {
+            char c = pattern.charAt(i);
+            sb.append(c);
+            if (c == '<') {
+                inDateMathExpr = true;
+            } else if (c == '>') {
+                inDateMathExpr = false;
+            } else if (c == '|' && inDateMathExpr == false) {
+                sb.deleteCharAt(sb.length() - 1);
+                indices.add(sb.toString());
+                sb.setLength(0);
+            }
+        }
+
+        if (sb.isEmpty() == false) {
+            indices.add(sb.toString());
+        }
+
+        return indices.toArray(new String[0]);
+    }
+
     private static String removeExclusion(String indexPattern) {
         return indexPattern.charAt(0) == EXCLUSION.charAt(0) ? indexPattern.substring(1) : indexPattern;
     }