Skip to content

Commit 9fb1b18

Browse files
bpinteabpintea
authored
ESQL: fix the column position in errors (#117153) (#117259)
This fixes the off-by-one error of the column position in some of the error messages. (cherry picked from commit 21f206b)
1 parent 79c6336 commit 9fb1b18

File tree

7 files changed

+54
-49
lines changed

7 files changed

+54
-49
lines changed

‎docs/changelog/117153.yaml‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 117153
2+
summary: "ESQL: fix the column position in errors"
3+
area: ES|QL
4+
type: bug
5+
issues: []

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -417,7 +417,7 @@ private static void checkRow(LogicalPlan p, Set<Failure> failures) {
417417
if (p instanceof Row row) {
418418
row.fields().forEach(a -> {
419419
if (DataType.isRepresentable(a.dataType()) == false) {
420-
failures.add(fail(a, "cannot use [{}] directly in a row assignment", a.child().sourceText()));
420+
failures.add(fail(a.child(), "cannot use [{}] directly in a row assignment", a.child().sourceText()));
421421
}
422422
});
423423
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ public class ParsingException extends EsqlClientException {
1818
public ParsingException(String message, Exception cause, int line, int charPositionInLine) {
1919
super(message, cause);
2020
this.line = line;
21-
this.charPositionInLine = charPositionInLine;
21+
this.charPositionInLine = charPositionInLine + 1;
2222
}
2323

2424
ParsingException(String message, Object... args) {
@@ -42,7 +42,7 @@ public int getLineNumber() {
4242
}
4343

4444
public int getColumnNumber() {
45-
return charPositionInLine + 1;
45+
return charPositionInLine;
4646
}
4747

4848
public String getErrorMessage() {

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

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -744,40 +744,40 @@ public void testWrongInputParam() {
744744

745745
public void testPeriodAndDurationInRowAssignment() {
746746
for (var unit : TIME_DURATIONS) {
747-
assertEquals("1:5: cannot use [1 " + unit + "] directly in a row assignment", error("row a = 1 " + unit));
747+
assertEquals("1:9: cannot use [1 " + unit + "] directly in a row assignment", error("row a = 1 " + unit));
748748
assertEquals(
749-
"1:5: cannot use [1 " + unit + "::time_duration] directly in a row assignment",
749+
"1:9: cannot use [1 " + unit + "::time_duration] directly in a row assignment",
750750
error("row a = 1 " + unit + "::time_duration")
751751
);
752752
assertEquals(
753-
"1:5: cannot use [\"1 " + unit + "\"::time_duration] directly in a row assignment",
753+
"1:9: cannot use [\"1 " + unit + "\"::time_duration] directly in a row assignment",
754754
error("row a = \"1 " + unit + "\"::time_duration")
755755
);
756756
assertEquals(
757-
"1:5: cannot use [to_timeduration(1 " + unit + ")] directly in a row assignment",
757+
"1:9: cannot use [to_timeduration(1 " + unit + ")] directly in a row assignment",
758758
error("row a = to_timeduration(1 " + unit + ")")
759759
);
760760
assertEquals(
761-
"1:5: cannot use [to_timeduration(\"1 " + unit + "\")] directly in a row assignment",
761+
"1:9: cannot use [to_timeduration(\"1 " + unit + "\")] directly in a row assignment",
762762
error("row a = to_timeduration(\"1 " + unit + "\")")
763763
);
764764
}
765765
for (var unit : DATE_PERIODS) {
766-
assertEquals("1:5: cannot use [1 " + unit + "] directly in a row assignment", error("row a = 1 " + unit));
766+
assertEquals("1:9: cannot use [1 " + unit + "] directly in a row assignment", error("row a = 1 " + unit));
767767
assertEquals(
768-
"1:5: cannot use [1 " + unit + "::date_period] directly in a row assignment",
768+
"1:9: cannot use [1 " + unit + "::date_period] directly in a row assignment",
769769
error("row a = 1 " + unit + "::date_period")
770770
);
771771
assertEquals(
772-
"1:5: cannot use [\"1 " + unit + "\"::date_period] directly in a row assignment",
772+
"1:9: cannot use [\"1 " + unit + "\"::date_period] directly in a row assignment",
773773
error("row a = \"1 " + unit + "\"::date_period")
774774
);
775775
assertEquals(
776-
"1:5: cannot use [to_dateperiod(1 " + unit + ")] directly in a row assignment",
776+
"1:9: cannot use [to_dateperiod(1 " + unit + ")] directly in a row assignment",
777777
error("row a = to_dateperiod(1 " + unit + ")")
778778
);
779779
assertEquals(
780-
"1:5: cannot use [to_dateperiod(\"1 " + unit + "\")] directly in a row assignment",
780+
"1:9: cannot use [to_dateperiod(\"1 " + unit + "\")] directly in a row assignment",
781781
error("row a = to_dateperiod(\"1 " + unit + "\")")
782782
);
783783
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2020,15 +2020,15 @@ public void testSimplifyRLikeMatchAll() {
20202020

20212021
public void testRLikeWrongPattern() {
20222022
String query = "from test | where first_name rlike \"(?i)(^|[^a-zA-Z0-9_-])nmap($|\\\\.)\"";
2023-
String error = "line 1:20: Invalid regex pattern for RLIKE [(?i)(^|[^a-zA-Z0-9_-])nmap($|\\.)]: "
2023+
String error = "line 1:19: Invalid regex pattern for RLIKE [(?i)(^|[^a-zA-Z0-9_-])nmap($|\\.)]: "
20242024
+ "[invalid range: from (95) cannot be > to (93)]";
20252025
ParsingException e = expectThrows(ParsingException.class, () -> plan(query));
20262026
assertThat(e.getMessage(), is(error));
20272027
}
20282028

20292029
public void testLikeWrongPattern() {
20302030
String query = "from test | where first_name like \"(?i)(^|[^a-zA-Z0-9_-])nmap($|\\\\.)\"";
2031-
String error = "line 1:20: Invalid pattern for LIKE [(?i)(^|[^a-zA-Z0-9_-])nmap($|\\.)]: "
2031+
String error = "line 1:19: Invalid pattern for LIKE [(?i)(^|[^a-zA-Z0-9_-])nmap($|\\.)]: "
20322032
+ "[Invalid sequence - escape character is not followed by special wildcard char]";
20332033
ParsingException e = expectThrows(ParsingException.class, () -> plan(query));
20342034
assertThat(e.getMessage(), is(error));

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -134,7 +134,7 @@ public void testStringLiteralsExceptions() {
134134
);
135135

136136
var number = "1" + IntStream.range(0, 309).mapToObj(ignored -> "0").collect(Collectors.joining());
137-
assertParsingException(() -> parse("row foo == " + number), "line 1:13: Number [" + number + "] is too large");
137+
assertParsingException(() -> parse("row foo == " + number), "line 1:12: Number [" + number + "] is too large");
138138
}
139139

140140
public void testBooleanLiteralsCondition() {
@@ -442,33 +442,33 @@ public void testOverflowingValueForDuration() {
442442
for (String unit : List.of("milliseconds", "seconds", "minutes", "hours")) {
443443
assertParsingException(
444444
() -> parse("row x = 9223372036854775808 " + unit), // unsigned_long (Long.MAX_VALUE + 1)
445-
"line 1:10: Number [9223372036854775808] outside of [" + unit + "] range"
445+
"line 1:9: Number [9223372036854775808] outside of [" + unit + "] range"
446446
);
447447
assertParsingException(
448448
() -> parse("row x = 18446744073709551616 " + unit), // double (UNSIGNED_LONG_MAX + 1)
449-
"line 1:10: Number [18446744073709551616] outside of [" + unit + "] range"
449+
"line 1:9: Number [18446744073709551616] outside of [" + unit + "] range"
450450
);
451451
}
452452
assertParsingException(
453453
() -> parse("row x = 153722867280912931 minutes"), // Long.MAX_VALUE / 60 + 1
454-
"line 1:10: Number [153722867280912931] outside of [minutes] range"
454+
"line 1:9: Number [153722867280912931] outside of [minutes] range"
455455
);
456456
assertParsingException(
457457
() -> parse("row x = 2562047788015216 hours"), // Long.MAX_VALUE / 3600 + 1
458-
"line 1:10: Number [2562047788015216] outside of [hours] range"
458+
"line 1:9: Number [2562047788015216] outside of [hours] range"
459459
);
460460
}
461461

462462
public void testOverflowingValueForPeriod() {
463463
for (String unit : List.of("days", "weeks", "months", "years")) {
464464
assertParsingException(
465465
() -> parse("row x = 2147483648 " + unit), // long (Integer.MAX_VALUE + 1)
466-
"line 1:10: Number [2147483648] outside of [" + unit + "] range"
466+
"line 1:9: Number [2147483648] outside of [" + unit + "] range"
467467
);
468468
}
469469
assertParsingException(
470470
() -> parse("row x = 306783379 weeks"), // Integer.MAX_VALUE / 7 + 1
471-
"line 1:10: Number [306783379] outside of [weeks] range"
471+
"line 1:9: Number [306783379] outside of [weeks] range"
472472
);
473473
}
474474

@@ -544,7 +544,7 @@ public void testWildcardProjectAwayPatterns() {
544544
}
545545

546546
public void testForbidWildcardProjectAway() {
547-
assertParsingException(() -> dropExpression("foo, *"), "line 1:21: Removing all fields is not allowed [*]");
547+
assertParsingException(() -> dropExpression("foo, *"), "line 1:20: Removing all fields is not allowed [*]");
548548
}
549549

550550
public void testForbidMultipleIncludeStar() {
@@ -608,7 +608,7 @@ public void testMultipleProjectPatterns() {
608608
}
609609

610610
public void testForbidWildcardProjectRename() {
611-
assertParsingException(() -> renameExpression("b* AS a*"), "line 1:18: Using wildcards [*] in RENAME is not allowed [b* AS a*]");
611+
assertParsingException(() -> renameExpression("b* AS a*"), "line 1:17: Using wildcards [*] in RENAME is not allowed [b* AS a*]");
612612
}
613613

614614
public void testSimplifyInWithSingleElementList() {

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

Lines changed: 25 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -516,10 +516,10 @@ private void clusterAndIndexAsLookupIndexPattern(String clusterAndIndex) {
516516

517517
public void testInvalidCharacterInIndexPattern() {
518518
Map<String, String> commands = new HashMap<>();
519-
commands.put("FROM {}", "line 1:7: ");
519+
commands.put("FROM {}", "line 1:6: ");
520520
if (Build.current().isSnapshot()) {
521-
commands.put("METRICS {}", "line 1:10: ");
522-
commands.put("ROW x = 1 | LOOKUP {} ON j", "line 1:21: ");
521+
commands.put("METRICS {}", "line 1:9: ");
522+
commands.put("ROW x = 1 | LOOKUP {} ON j", "line 1:20: ");
523523
}
524524
String lineNumber;
525525
for (String command : commands.keySet()) {
@@ -563,7 +563,7 @@ public void testInvalidCharacterInIndexPattern() {
563563
continue;
564564
}
565565

566-
lineNumber = command.contains("FROM") ? "line 1:21: " : "line 1:24: ";
566+
lineNumber = command.contains("FROM") ? "line 1:20: " : "line 1:23: ";
567567
expectInvalidIndexNameErrorWithLineNumber(command, "indexpattern, --indexpattern", lineNumber, "-indexpattern");
568568
expectInvalidIndexNameErrorWithLineNumber(command, "indexpattern, \"--indexpattern\"", lineNumber, "-indexpattern");
569569
expectInvalidIndexNameErrorWithLineNumber(command, "\"indexpattern, --indexpattern\"", commands.get(command), "-indexpattern");
@@ -576,7 +576,7 @@ public void testInvalidCharacterInIndexPattern() {
576576
if (command.contains("LOOKUP")) {
577577
continue;
578578
}
579-
lineNumber = command.contains("FROM") ? "line 1:10: " : "line 1:13: ";
579+
lineNumber = command.contains("FROM") ? "line 1:9: " : "line 1:12: ";
580580
clustersAndIndices(command, "*", "-index#pattern");
581581
clustersAndIndices(command, "index*", "-index#pattern");
582582
clustersAndIndices(command, "*", "-<--logstash-{now/M{yyyy.MM}}>");
@@ -876,18 +876,18 @@ public void testSuggestAvailableProcessingCommandsOnParsingError() {
876876
public void testDeprecatedIsNullFunction() {
877877
expectError(
878878
"from test | eval x = is_null(f)",
879-
"line 1:23: is_null function is not supported anymore, please use 'is null'/'is not null' predicates instead"
879+
"line 1:22: is_null function is not supported anymore, please use 'is null'/'is not null' predicates instead"
880880
);
881881
expectError(
882882
"row x = is_null(f)",
883-
"line 1:10: is_null function is not supported anymore, please use 'is null'/'is not null' predicates instead"
883+
"line 1:9: is_null function is not supported anymore, please use 'is null'/'is not null' predicates instead"
884884
);
885885

886886
if (Build.current().isSnapshot()) {
887887
expectError(
888888
"from test | eval x = ?fn1(f)",
889889
List.of(paramAsIdentifier("fn1", "IS_NULL")),
890-
"line 1:23: is_null function is not supported anymore, please use 'is null'/'is not null' predicates instead"
890+
"line 1:22: is_null function is not supported anymore, please use 'is null'/'is not null' predicates instead"
891891
);
892892
}
893893
}
@@ -902,23 +902,23 @@ public void testMetadataFieldOnOtherSources() {
902902
}
903903

904904
public void testMetadataFieldMultipleDeclarations() {
905-
expectError("from test metadata _index, _version, _index", "1:39: metadata field [_index] already declared [@1:20]");
905+
expectError("from test metadata _index, _version, _index", "1:38: metadata field [_index] already declared [@1:20]");
906906
}
907907

908908
public void testMetadataFieldUnsupportedPrimitiveType() {
909-
expectError("from test metadata _tier", "line 1:21: unsupported metadata field [_tier]");
909+
expectError("from test metadata _tier", "line 1:20: unsupported metadata field [_tier]");
910910
}
911911

912912
public void testMetadataFieldUnsupportedCustomType() {
913-
expectError("from test metadata _feature", "line 1:21: unsupported metadata field [_feature]");
913+
expectError("from test metadata _feature", "line 1:20: unsupported metadata field [_feature]");
914914
}
915915

916916
public void testMetadataFieldNotFoundNonExistent() {
917-
expectError("from test metadata _doesnot_compute", "line 1:21: unsupported metadata field [_doesnot_compute]");
917+
expectError("from test metadata _doesnot_compute", "line 1:20: unsupported metadata field [_doesnot_compute]");
918918
}
919919

920920
public void testMetadataFieldNotFoundNormalField() {
921-
expectError("from test metadata emp_no", "line 1:21: unsupported metadata field [emp_no]");
921+
expectError("from test metadata emp_no", "line 1:20: unsupported metadata field [emp_no]");
922922
}
923923

924924
public void testDissectPattern() {
@@ -976,13 +976,13 @@ public void testGrokPattern() {
976976

977977
expectError(
978978
"row a = \"foo bar\" | GROK a \"%{NUMBER:foo} %{WORD:foo}\"",
979-
"line 1:22: Invalid GROK pattern [%{NUMBER:foo} %{WORD:foo}]:"
979+
"line 1:21: Invalid GROK pattern [%{NUMBER:foo} %{WORD:foo}]:"
980980
+ " the attribute [foo] is defined multiple times with different types"
981981
);
982982

983983
expectError(
984984
"row a = \"foo\" | GROK a \"(?P<justification>.+)\"",
985-
"line 1:18: Invalid grok pattern [(?P<justification>.+)]: [undefined group option]"
985+
"line 1:17: Invalid grok pattern [(?P<justification>.+)]: [undefined group option]"
986986
);
987987
}
988988

@@ -1006,7 +1006,7 @@ public void testLikeRLike() {
10061006

10071007
expectError(
10081008
"from a | where foo like \"(?i)(^|[^a-zA-Z0-9_-])nmap($|\\\\.)\"",
1009-
"line 1:17: Invalid pattern for LIKE [(?i)(^|[^a-zA-Z0-9_-])nmap($|\\.)]: "
1009+
"line 1:16: Invalid pattern for LIKE [(?i)(^|[^a-zA-Z0-9_-])nmap($|\\.)]: "
10101010
+ "[Invalid sequence - escape character is not followed by special wildcard char]"
10111011
);
10121012
}
@@ -1067,7 +1067,7 @@ public void testEnrich() {
10671067
);
10681068
expectError(
10691069
"from a | enrich typo:countries on foo",
1070-
"line 1:18: Unrecognized value [typo], ENRICH policy qualifier needs to be one of [_ANY, _COORDINATOR, _REMOTE]"
1070+
"line 1:17: Unrecognized value [typo], ENRICH policy qualifier needs to be one of [_ANY, _COORDINATOR, _REMOTE]"
10711071
);
10721072
}
10731073

@@ -1252,8 +1252,8 @@ public void testInvalidPositionalParams() {
12521252
expectError(
12531253
"from test | where x < ?0 and y < ?2",
12541254
List.of(paramAsConstant(null, 5)),
1255-
"line 1:24: No parameter is defined for position 0, did you mean position 1?; "
1256-
+ "line 1:35: No parameter is defined for position 2, did you mean position 1?"
1255+
"line 1:23: No parameter is defined for position 0, did you mean position 1?; "
1256+
+ "line 1:34: No parameter is defined for position 2, did you mean position 1?"
12571257
);
12581258

12591259
expectError(
@@ -2098,11 +2098,11 @@ public void testEnrichOnMatchField() {
20982098
}
20992099

21002100
public void testInlineConvertWithNonexistentType() {
2101-
expectError("ROW 1::doesnotexist", "line 1:9: Unknown data type named [doesnotexist]");
2102-
expectError("ROW \"1\"::doesnotexist", "line 1:11: Unknown data type named [doesnotexist]");
2103-
expectError("ROW false::doesnotexist", "line 1:13: Unknown data type named [doesnotexist]");
2104-
expectError("ROW abs(1)::doesnotexist", "line 1:14: Unknown data type named [doesnotexist]");
2105-
expectError("ROW (1+2)::doesnotexist", "line 1:13: Unknown data type named [doesnotexist]");
2101+
expectError("ROW 1::doesnotexist", "line 1:8: Unknown data type named [doesnotexist]");
2102+
expectError("ROW \"1\"::doesnotexist", "line 1:10: Unknown data type named [doesnotexist]");
2103+
expectError("ROW false::doesnotexist", "line 1:12: Unknown data type named [doesnotexist]");
2104+
expectError("ROW abs(1)::doesnotexist", "line 1:13: Unknown data type named [doesnotexist]");
2105+
expectError("ROW (1+2)::doesnotexist", "line 1:12: Unknown data type named [doesnotexist]");
21062106
}
21072107

21082108
public void testLookup() {
@@ -2122,7 +2122,7 @@ public void testLookup() {
21222122
}
21232123

21242124
public void testInlineConvertUnsupportedType() {
2125-
expectError("ROW 3::BYTE", "line 1:6: Unsupported conversion to type [BYTE]");
2125+
expectError("ROW 3::BYTE", "line 1:5: Unsupported conversion to type [BYTE]");
21262126
}
21272127

21282128
public void testMetricsWithoutStats() {

0 commit comments

Comments
 (0)