Browse Source

Fix: prevent duplication of "invalid index name" string in the final exception error message (#130027)

* Use `throwInvalidIndexNameException()` to throw invalid ex after
dropping asterisk in `IdentifierBuilder#resolveAndValidateIndex()`

* Assert the message in test

* Refactor

* drop invalid chars from assertion string due to randomisation issue

* Re-assert invalid chars

* Update docs/changelog/130027.yaml
Pawan Kartik 3 months ago
parent
commit
2667a2d4ba

+ 6 - 0
docs/changelog/130027.yaml

@@ -0,0 +1,6 @@
+pr: 130027
+summary: "Fix: prevent duplication of \"invalid index name\" string in the final exception\
+  \ error message"
+area: ES|QL
+type: bug
+issues: []

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

@@ -34,6 +34,8 @@ abstract class IdentifierBuilder extends AbstractBuilder {
 
     private static final String BLANK_INDEX_ERROR_MESSAGE = "Blank index specified in index pattern";
 
+    private static final String INVALID_ESQL_CHARS = Strings.INVALID_FILENAME_CHARS.replace("'*',", "");
+
     @Override
     public String visitIdentifier(IdentifierContext ctx) {
         return ctx == null ? null : unquoteIdentifier(ctx.QUOTED_IDENTIFIER(), ctx.UNQUOTED_IDENTIFIER());
@@ -305,17 +307,16 @@ abstract class IdentifierBuilder extends AbstractBuilder {
                 return;
             }
 
-            InvalidIndexNameException errorToThrow = e;
             /*
              * We only modify this particular message because it mentions '*' as an invalid char.
              * However, we do allow asterisk in the index patterns: wildcarded patterns. Let's not
              * mislead the user by mentioning this char in the error message.
              */
             if (e.getMessage().contains("must not contain the following characters")) {
-                errorToThrow = new InvalidIndexNameException(index, e.getMessage().replace("'*',", ""));
+                throwInvalidIndexNameException(index, "must not contain the following characters " + INVALID_ESQL_CHARS, ctx);
             }
 
-            throw new ParsingException(errorToThrow, source(ctx), errorToThrow.getMessage());
+            throw new ParsingException(e, source(ctx), e.getMessage());
         }
     }
 

+ 8 - 2
x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java

@@ -3230,9 +3230,15 @@ public class StatementParserTests extends AbstractStatementParserTests {
             var randomInvalidChar = randomFrom(invalidChars);
 
             // Construct the new invalid index pattern.
-            var remoteIndexWithInvalidChar = quote(randomIdentifier() + ":" + "foo" + randomInvalidChar + "bar");
+            var invalidIndexName = "foo" + randomInvalidChar + "bar";
+            var remoteIndexWithInvalidChar = quote(randomIdentifier() + ":" + invalidIndexName);
             var query = "FROM " + randomIndex + "," + remoteIndexWithInvalidChar;
-            expectError(query, "must not contain the following characters [' ','\"',',','/','<','>','?','\\','|']");
+            expectError(
+                query,
+                "Invalid index name ["
+                    + invalidIndexName
+                    + "], must not contain the following characters [' ','\"',',','/','<','>','?','\\','|']"
+            );
         }
 
         // Colon outside a quoted string should result in an ANTLR error: a comma is expected.