ESQL: fix the column position in errors (#117153) (#117255)

This fixes the off-by-one error of the column position in some of the
error messages.

(cherry picked from commit 21f206b70c0b140f1737ca3fa0260399796adfa4)

docs/changelog/117153.yaml

@@ -0,0 +1,5 @@
+pr: 117153
+summary: "ESQL: fix the column position in errors"
+area: ES|QL
+type: bug
+issues: []

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

@@ -511,7 +511,7 @@ public class Verifier {
         if (p instanceof Row row) {
             row.fields().forEach(a -> {
                 if (DataType.isRepresentable(a.dataType()) == false) {
-                    failures.add(fail(a, "cannot use [{}] directly in a row assignment", a.child().sourceText()));
+                    failures.add(fail(a.child(), "cannot use [{}] directly in a row assignment", a.child().sourceText()));
                 }
             });
         }

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

@@ -18,7 +18,7 @@ public class ParsingException extends EsqlClientException {
     public ParsingException(String message, Exception cause, int line, int charPositionInLine) {
         super(message, cause);
         this.line = line;
-        this.charPositionInLine = charPositionInLine;
+        this.charPositionInLine = charPositionInLine + 1;
     }
 
     ParsingException(String message, Object... args) {
@@ -42,7 +42,7 @@ public class ParsingException extends EsqlClientException {
     }
 
     public int getColumnNumber() {
-        return charPositionInLine + 1;
+        return charPositionInLine;
     }
 
     public String getErrorMessage() {

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java

@@ -771,40 +771,40 @@ public class VerifierTests extends ESTestCase {
 
     public void testPeriodAndDurationInRowAssignment() {
         for (var unit : TIME_DURATIONS) {
-            assertEquals("1:5: cannot use [1 " + unit + "] directly in a row assignment", error("row a = 1 " + unit));
+            assertEquals("1:9: cannot use [1 " + unit + "] directly in a row assignment", error("row a = 1 " + unit));
             assertEquals(
-                "1:5: cannot use [1 " + unit + "::time_duration] directly in a row assignment",
+                "1:9: cannot use [1 " + unit + "::time_duration] directly in a row assignment",
                 error("row a = 1 " + unit + "::time_duration")
             );
             assertEquals(
-                "1:5: cannot use [\"1 " + unit + "\"::time_duration] directly in a row assignment",
+                "1:9: cannot use [\"1 " + unit + "\"::time_duration] directly in a row assignment",
                 error("row a = \"1 " + unit + "\"::time_duration")
             );
             assertEquals(
-                "1:5: cannot use [to_timeduration(1 " + unit + ")] directly in a row assignment",
+                "1:9: cannot use [to_timeduration(1 " + unit + ")] directly in a row assignment",
                 error("row a = to_timeduration(1 " + unit + ")")
             );
             assertEquals(
-                "1:5: cannot use [to_timeduration(\"1 " + unit + "\")] directly in a row assignment",
+                "1:9: cannot use [to_timeduration(\"1 " + unit + "\")] directly in a row assignment",
                 error("row a = to_timeduration(\"1 " + unit + "\")")
             );
         }
         for (var unit : DATE_PERIODS) {
-            assertEquals("1:5: cannot use [1 " + unit + "] directly in a row assignment", error("row a = 1 " + unit));
+            assertEquals("1:9: cannot use [1 " + unit + "] directly in a row assignment", error("row a = 1 " + unit));
             assertEquals(
-                "1:5: cannot use [1 " + unit + "::date_period] directly in a row assignment",
+                "1:9: cannot use [1 " + unit + "::date_period] directly in a row assignment",
                 error("row a = 1 " + unit + "::date_period")
             );
             assertEquals(
-                "1:5: cannot use [\"1 " + unit + "\"::date_period] directly in a row assignment",
+                "1:9: cannot use [\"1 " + unit + "\"::date_period] directly in a row assignment",
                 error("row a = \"1 " + unit + "\"::date_period")
             );
             assertEquals(
-                "1:5: cannot use [to_dateperiod(1 " + unit + ")] directly in a row assignment",
+                "1:9: cannot use [to_dateperiod(1 " + unit + ")] directly in a row assignment",
                 error("row a = to_dateperiod(1 " + unit + ")")
             );
             assertEquals(
-                "1:5: cannot use [to_dateperiod(\"1 " + unit + "\")] directly in a row assignment",
+                "1:9: cannot use [to_dateperiod(\"1 " + unit + "\")] directly in a row assignment",
                 error("row a = to_dateperiod(\"1 " + unit + "\")")
             );
         }

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/LogicalPlanOptimizerTests.java

@@ -2563,7 +2563,7 @@ public class LogicalPlanOptimizerTests extends ESTestCase {
 
     public void testRLikeWrongPattern() {
         String query = "from test | where first_name rlike \"(?i)(^|[^a-zA-Z0-9_-])nmap($|\\\\.)\"";
-        String error = "line 1:20: Invalid regex pattern for RLIKE [(?i)(^|[^a-zA-Z0-9_-])nmap($|\\.)]: "
+        String error = "line 1:19: Invalid regex pattern for RLIKE [(?i)(^|[^a-zA-Z0-9_-])nmap($|\\.)]: "
             + "[invalid range: from (95) cannot be > to (93)]";
         ParsingException e = expectThrows(ParsingException.class, () -> plan(query));
         assertThat(e.getMessage(), is(error));
@@ -2571,7 +2571,7 @@ public class LogicalPlanOptimizerTests extends ESTestCase {
 
     public void testLikeWrongPattern() {
         String query = "from test | where first_name like \"(?i)(^|[^a-zA-Z0-9_-])nmap($|\\\\.)\"";
-        String error = "line 1:20: Invalid pattern for LIKE [(?i)(^|[^a-zA-Z0-9_-])nmap($|\\.)]: "
+        String error = "line 1:19: Invalid pattern for LIKE [(?i)(^|[^a-zA-Z0-9_-])nmap($|\\.)]: "
             + "[Invalid sequence - escape character is not followed by special wildcard char]";
         ParsingException e = expectThrows(ParsingException.class, () -> plan(query));
         assertThat(e.getMessage(), is(error));

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/ExpressionTests.java

@@ -134,7 +134,7 @@ public class ExpressionTests extends ESTestCase {
         );
 
         var number = "1" + IntStream.range(0, 309).mapToObj(ignored -> "0").collect(Collectors.joining());
-        assertParsingException(() -> parse("row foo == " + number), "line 1:13: Number [" + number + "] is too large");
+        assertParsingException(() -> parse("row foo == " + number), "line 1:12: Number [" + number + "] is too large");
     }
 
     public void testBooleanLiteralsCondition() {
@@ -442,20 +442,20 @@ public class ExpressionTests extends ESTestCase {
         for (String unit : List.of("milliseconds", "seconds", "minutes", "hours")) {
             assertParsingException(
                 () -> parse("row x = 9223372036854775808 " + unit), // unsigned_long (Long.MAX_VALUE + 1)
-                "line 1:10: Number [9223372036854775808] outside of [" + unit + "] range"
+                "line 1:9: Number [9223372036854775808] outside of [" + unit + "] range"
             );
             assertParsingException(
                 () -> parse("row x = 18446744073709551616 " + unit), // double (UNSIGNED_LONG_MAX + 1)
-                "line 1:10: Number [18446744073709551616] outside of [" + unit + "] range"
+                "line 1:9: Number [18446744073709551616] outside of [" + unit + "] range"
             );
         }
         assertParsingException(
             () -> parse("row x = 153722867280912931 minutes"), // Long.MAX_VALUE / 60 + 1
-            "line 1:10: Number [153722867280912931] outside of [minutes] range"
+            "line 1:9: Number [153722867280912931] outside of [minutes] range"
         );
         assertParsingException(
             () -> parse("row x = 2562047788015216 hours"), // Long.MAX_VALUE / 3600 + 1
-            "line 1:10: Number [2562047788015216] outside of [hours] range"
+            "line 1:9: Number [2562047788015216] outside of [hours] range"
         );
     }
 
@@ -463,12 +463,12 @@ public class ExpressionTests extends ESTestCase {
         for (String unit : List.of("days", "weeks", "months", "years")) {
             assertParsingException(
                 () -> parse("row x = 2147483648 " + unit), // long (Integer.MAX_VALUE + 1)
-                "line 1:10: Number [2147483648] outside of [" + unit + "] range"
+                "line 1:9: Number [2147483648] outside of [" + unit + "] range"
             );
         }
         assertParsingException(
             () -> parse("row x = 306783379 weeks"), // Integer.MAX_VALUE / 7 + 1
-            "line 1:10: Number [306783379] outside of [weeks] range"
+            "line 1:9: Number [306783379] outside of [weeks] range"
         );
     }
 
@@ -544,7 +544,7 @@ public class ExpressionTests extends ESTestCase {
     }
 
     public void testForbidWildcardProjectAway() {
-        assertParsingException(() -> dropExpression("foo, *"), "line 1:21: Removing all fields is not allowed [*]");
+        assertParsingException(() -> dropExpression("foo, *"), "line 1:20: Removing all fields is not allowed [*]");
     }
 
     public void testForbidMultipleIncludeStar() {
@@ -608,7 +608,7 @@ public class ExpressionTests extends ESTestCase {
     }
 
     public void testForbidWildcardProjectRename() {
-        assertParsingException(() -> renameExpression("b* AS a*"), "line 1:18: Using wildcards [*] in RENAME is not allowed [b* AS a*]");
+        assertParsingException(() -> renameExpression("b* AS a*"), "line 1:17: Using wildcards [*] in RENAME is not allowed [b* AS a*]");
     }
 
     public void testSimplifyInWithSingleElementList() {

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

@@ -525,10 +525,10 @@ public class StatementParserTests extends AbstractStatementParserTests {
 
     public void testInvalidCharacterInIndexPattern() {
         Map<String, String> commands = new HashMap<>();
-        commands.put("FROM {}", "line 1:7: ");
+        commands.put("FROM {}", "line 1:6: ");
         if (Build.current().isSnapshot()) {
-            commands.put("METRICS {}", "line 1:10: ");
-            commands.put("ROW x = 1 | LOOKUP_🐔 {} ON j", "line 1:23: ");
+            commands.put("METRICS {}", "line 1:9: ");
+            commands.put("ROW x = 1 | LOOKUP_🐔 {} ON j", "line 1:22: ");
         }
         String lineNumber;
         for (String command : commands.keySet()) {
@@ -572,7 +572,7 @@ public class StatementParserTests extends AbstractStatementParserTests {
                 continue;
             }
 
-            lineNumber = command.contains("FROM") ? "line 1:21: " : "line 1:24: ";
+            lineNumber = command.contains("FROM") ? "line 1:20: " : "line 1:23: ";
             expectInvalidIndexNameErrorWithLineNumber(command, "indexpattern, --indexpattern", lineNumber, "-indexpattern");
             expectInvalidIndexNameErrorWithLineNumber(command, "indexpattern, \"--indexpattern\"", lineNumber, "-indexpattern");
             expectInvalidIndexNameErrorWithLineNumber(command, "\"indexpattern, --indexpattern\"", commands.get(command), "-indexpattern");
@@ -585,7 +585,7 @@ public class StatementParserTests extends AbstractStatementParserTests {
             if (command.contains("LOOKUP_🐔")) {
                 continue;
             }
-            lineNumber = command.contains("FROM") ? "line 1:10: " : "line 1:13: ";
+            lineNumber = command.contains("FROM") ? "line 1:9: " : "line 1:12: ";
             clustersAndIndices(command, "*", "-index#pattern");
             clustersAndIndices(command, "index*", "-index#pattern");
             clustersAndIndices(command, "*", "-<--logstash-{now/M{yyyy.MM}}>");
@@ -885,18 +885,18 @@ public class StatementParserTests extends AbstractStatementParserTests {
     public void testDeprecatedIsNullFunction() {
         expectError(
             "from test | eval x = is_null(f)",
-            "line 1:23: is_null function is not supported anymore, please use 'is null'/'is not null' predicates instead"
+            "line 1:22: is_null function is not supported anymore, please use 'is null'/'is not null' predicates instead"
         );
         expectError(
             "row x = is_null(f)",
-            "line 1:10: is_null function is not supported anymore, please use 'is null'/'is not null' predicates instead"
+            "line 1:9: is_null function is not supported anymore, please use 'is null'/'is not null' predicates instead"
         );
 
         if (Build.current().isSnapshot()) {
             expectError(
                 "from test | eval x = ?fn1(f)",
                 List.of(paramAsIdentifier("fn1", "IS_NULL")),
-                "line 1:23: is_null function is not supported anymore, please use 'is null'/'is not null' predicates instead"
+                "line 1:22: is_null function is not supported anymore, please use 'is null'/'is not null' predicates instead"
             );
         }
     }
@@ -911,23 +911,23 @@ public class StatementParserTests extends AbstractStatementParserTests {
     }
 
     public void testMetadataFieldMultipleDeclarations() {
-        expectError("from test metadata _index, _version, _index", "1:39: metadata field [_index] already declared [@1:20]");
+        expectError("from test metadata _index, _version, _index", "1:38: metadata field [_index] already declared [@1:20]");
     }
 
     public void testMetadataFieldUnsupportedPrimitiveType() {
-        expectError("from test metadata _tier", "line 1:21: unsupported metadata field [_tier]");
+        expectError("from test metadata _tier", "line 1:20: unsupported metadata field [_tier]");
     }
 
     public void testMetadataFieldUnsupportedCustomType() {
-        expectError("from test metadata _feature", "line 1:21: unsupported metadata field [_feature]");
+        expectError("from test metadata _feature", "line 1:20: unsupported metadata field [_feature]");
     }
 
     public void testMetadataFieldNotFoundNonExistent() {
-        expectError("from test metadata _doesnot_compute", "line 1:21: unsupported metadata field [_doesnot_compute]");
+        expectError("from test metadata _doesnot_compute", "line 1:20: unsupported metadata field [_doesnot_compute]");
     }
 
     public void testMetadataFieldNotFoundNormalField() {
-        expectError("from test metadata emp_no", "line 1:21: unsupported metadata field [emp_no]");
+        expectError("from test metadata emp_no", "line 1:20: unsupported metadata field [emp_no]");
     }
 
     public void testDissectPattern() {
@@ -985,13 +985,13 @@ public class StatementParserTests extends AbstractStatementParserTests {
 
         expectError(
             "row a = \"foo bar\" | GROK a \"%{NUMBER:foo} %{WORD:foo}\"",
-            "line 1:22: Invalid GROK pattern [%{NUMBER:foo} %{WORD:foo}]:"
+            "line 1:21: Invalid GROK pattern [%{NUMBER:foo} %{WORD:foo}]:"
                 + " the attribute [foo] is defined multiple times with different types"
         );
 
         expectError(
             "row a = \"foo\" | GROK a \"(?P<justification>.+)\"",
-            "line 1:18: Invalid grok pattern [(?P<justification>.+)]: [undefined group option]"
+            "line 1:17: Invalid grok pattern [(?P<justification>.+)]: [undefined group option]"
         );
     }
 
@@ -1015,7 +1015,7 @@ public class StatementParserTests extends AbstractStatementParserTests {
 
         expectError(
             "from a | where foo like \"(?i)(^|[^a-zA-Z0-9_-])nmap($|\\\\.)\"",
-            "line 1:17: Invalid pattern for LIKE [(?i)(^|[^a-zA-Z0-9_-])nmap($|\\.)]: "
+            "line 1:16: Invalid pattern for LIKE [(?i)(^|[^a-zA-Z0-9_-])nmap($|\\.)]: "
                 + "[Invalid sequence - escape character is not followed by special wildcard char]"
         );
     }
@@ -1076,7 +1076,7 @@ public class StatementParserTests extends AbstractStatementParserTests {
         );
         expectError(
             "from a | enrich typo:countries on foo",
-            "line 1:18: Unrecognized value [typo], ENRICH policy qualifier needs to be one of [_ANY, _COORDINATOR, _REMOTE]"
+            "line 1:17: Unrecognized value [typo], ENRICH policy qualifier needs to be one of [_ANY, _COORDINATOR, _REMOTE]"
         );
     }
 
@@ -1261,8 +1261,8 @@ public class StatementParserTests extends AbstractStatementParserTests {
         expectError(
             "from test | where x < ?0 and y < ?2",
             List.of(paramAsConstant(null, 5)),
-            "line 1:24: No parameter is defined for position 0, did you mean position 1?; "
-                + "line 1:35: No parameter is defined for position 2, did you mean position 1?"
+            "line 1:23: No parameter is defined for position 0, did you mean position 1?; "
+                + "line 1:34: No parameter is defined for position 2, did you mean position 1?"
         );
 
         expectError(
@@ -2107,11 +2107,11 @@ public class StatementParserTests extends AbstractStatementParserTests {
     }
 
     public void testInlineConvertWithNonexistentType() {
-        expectError("ROW 1::doesnotexist", "line 1:9: Unknown data type named [doesnotexist]");
-        expectError("ROW \"1\"::doesnotexist", "line 1:11: Unknown data type named [doesnotexist]");
-        expectError("ROW false::doesnotexist", "line 1:13: Unknown data type named [doesnotexist]");
-        expectError("ROW abs(1)::doesnotexist", "line 1:14: Unknown data type named [doesnotexist]");
-        expectError("ROW (1+2)::doesnotexist", "line 1:13: Unknown data type named [doesnotexist]");
+        expectError("ROW 1::doesnotexist", "line 1:8: Unknown data type named [doesnotexist]");
+        expectError("ROW \"1\"::doesnotexist", "line 1:10: Unknown data type named [doesnotexist]");
+        expectError("ROW false::doesnotexist", "line 1:12: Unknown data type named [doesnotexist]");
+        expectError("ROW abs(1)::doesnotexist", "line 1:13: Unknown data type named [doesnotexist]");
+        expectError("ROW (1+2)::doesnotexist", "line 1:12: Unknown data type named [doesnotexist]");
     }
 
     public void testLookup() {
@@ -2131,7 +2131,7 @@ public class StatementParserTests extends AbstractStatementParserTests {
     }
 
     public void testInlineConvertUnsupportedType() {
-        expectError("ROW 3::BYTE", "line 1:6: Unsupported conversion to type [BYTE]");
+        expectError("ROW 3::BYTE", "line 1:5: Unsupported conversion to type [BYTE]");
     }
 
     public void testMetricsWithoutStats() {