Skip to content

Commit 21f206b

Browse files
bpinteabpintea
authored
ESQL: fix the column position in errors (#117153)
This fixes the off-by-one error of the column position in some of the error messages.
1 parent 75f4232 commit 21f206b

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
@@ -511,7 +511,7 @@ private static void checkRow(LogicalPlan p, Set<Failure> failures) {
511511
if (p instanceof Row row) {
512512
row.fields().forEach(a -> {
513513
if (DataType.isRepresentable(a.dataType()) == false) {
514-
failures.add(fail(a, "cannot use [{}] directly in a row assignment", a.child().sourceText()));
514+
failures.add(fail(a.child(), "cannot use [{}] directly in a row assignment", a.child().sourceText()));
515515
}
516516
});
517517
}

‎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
@@ -771,40 +771,40 @@ public void testWrongInputParam() {
771771

772772
public void testPeriodAndDurationInRowAssignment() {
773773
for (var unit : TIME_DURATIONS) {
774-
assertEquals("1:5: cannot use [1 " + unit + "] directly in a row assignment", error("row a = 1 " + unit));
774+
assertEquals("1:9: cannot use [1 " + unit + "] directly in a row assignment", error("row a = 1 " + unit));
775775
assertEquals(
776-
"1:5: cannot use [1 " + unit + "::time_duration] directly in a row assignment",
776+
"1:9: cannot use [1 " + unit + "::time_duration] directly in a row assignment",
777777
error("row a = 1 " + unit + "::time_duration")
778778
);
779779
assertEquals(
780-
"1:5: cannot use [\"1 " + unit + "\"::time_duration] directly in a row assignment",
780+
"1:9: cannot use [\"1 " + unit + "\"::time_duration] directly in a row assignment",
781781
error("row a = \"1 " + unit + "\"::time_duration")
782782
);
783783
assertEquals(
784-
"1:5: cannot use [to_timeduration(1 " + unit + ")] directly in a row assignment",
784+
"1:9: cannot use [to_timeduration(1 " + unit + ")] directly in a row assignment",
785785
error("row a = to_timeduration(1 " + unit + ")")
786786
);
787787
assertEquals(
788-
"1:5: cannot use [to_timeduration(\"1 " + unit + "\")] directly in a row assignment",
788+
"1:9: cannot use [to_timeduration(\"1 " + unit + "\")] directly in a row assignment",
789789
error("row a = to_timeduration(\"1 " + unit + "\")")
790790
);
791791
}
792792
for (var unit : DATE_PERIODS) {
793-
assertEquals("1:5: cannot use [1 " + unit + "] directly in a row assignment", error("row a = 1 " + unit));
793+
assertEquals("1:9: cannot use [1 " + unit + "] directly in a row assignment", error("row a = 1 " + unit));
794794
assertEquals(
795-
"1:5: cannot use [1 " + unit + "::date_period] directly in a row assignment",
795+
"1:9: cannot use [1 " + unit + "::date_period] directly in a row assignment",
796796
error("row a = 1 " + unit + "::date_period")
797797
);
798798
assertEquals(
799-
"1:5: cannot use [\"1 " + unit + "\"::date_period] directly in a row assignment",
799+
"1:9: cannot use [\"1 " + unit + "\"::date_period] directly in a row assignment",
800800
error("row a = \"1 " + unit + "\"::date_period")
801801
);
802802
assertEquals(
803-
"1:5: cannot use [to_dateperiod(1 " + unit + ")] directly in a row assignment",
803+
"1:9: cannot use [to_dateperiod(1 " + unit + ")] directly in a row assignment",
804804
error("row a = to_dateperiod(1 " + unit + ")")
805805
);
806806
assertEquals(
807-
"1:5: cannot use [to_dateperiod(\"1 " + unit + "\")] directly in a row assignment",
807+
"1:9: cannot use [to_dateperiod(\"1 " + unit + "\")] directly in a row assignment",
808808
error("row a = to_dateperiod(\"1 " + unit + "\")")
809809
);
810810
}

‎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
@@ -2563,15 +2563,15 @@ public void testSimplifyRLikeMatchAll() {
25632563

25642564
public void testRLikeWrongPattern() {
25652565
String query = "from test | where first_name rlike \"(?i)(^|[^a-zA-Z0-9_-])nmap($|\\\\.)\"";
2566-
String error = "line 1:20: Invalid regex pattern for RLIKE [(?i)(^|[^a-zA-Z0-9_-])nmap($|\\.)]: "
2566+
String error = "line 1:19: Invalid regex pattern for RLIKE [(?i)(^|[^a-zA-Z0-9_-])nmap($|\\.)]: "
25672567
+ "[invalid range: from (95) cannot be > to (93)]";
25682568
ParsingException e = expectThrows(ParsingException.class, () -> plan(query));
25692569
assertThat(e.getMessage(), is(error));
25702570
}
25712571

25722572
public void testLikeWrongPattern() {
25732573
String query = "from test | where first_name like \"(?i)(^|[^a-zA-Z0-9_-])nmap($|\\\\.)\"";
2574-
String error = "line 1:20: Invalid pattern for LIKE [(?i)(^|[^a-zA-Z0-9_-])nmap($|\\.)]: "
2574+
String error = "line 1:19: Invalid pattern for LIKE [(?i)(^|[^a-zA-Z0-9_-])nmap($|\\.)]: "
25752575
+ "[Invalid sequence - escape character is not followed by special wildcard char]";
25762576
ParsingException e = expectThrows(ParsingException.class, () -> plan(query));
25772577
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
@@ -525,10 +525,10 @@ private void clusterAndIndexAsLookupIndexPattern(String clusterAndIndex) {
525525

526526
public void testInvalidCharacterInIndexPattern() {
527527
Map<String, String> commands = new HashMap<>();
528-
commands.put("FROM {}", "line 1:7: ");
528+
commands.put("FROM {}", "line 1:6: ");
529529
if (Build.current().isSnapshot()) {
530-
commands.put("METRICS {}", "line 1:10: ");
531-
commands.put("ROW x = 1 | LOOKUP_🐔 {} ON j", "line 1:23: ");
530+
commands.put("METRICS {}", "line 1:9: ");
531+
commands.put("ROW x = 1 | LOOKUP_🐔 {} ON j", "line 1:22: ");
532532
}
533533
String lineNumber;
534534
for (String command : commands.keySet()) {
@@ -572,7 +572,7 @@ public void testInvalidCharacterInIndexPattern() {
572572
continue;
573573
}
574574

575-
lineNumber = command.contains("FROM") ? "line 1:21: " : "line 1:24: ";
575+
lineNumber = command.contains("FROM") ? "line 1:20: " : "line 1:23: ";
576576
expectInvalidIndexNameErrorWithLineNumber(command, "indexpattern, --indexpattern", lineNumber, "-indexpattern");
577577
expectInvalidIndexNameErrorWithLineNumber(command, "indexpattern, \"--indexpattern\"", lineNumber, "-indexpattern");
578578
expectInvalidIndexNameErrorWithLineNumber(command, "\"indexpattern, --indexpattern\"", commands.get(command), "-indexpattern");
@@ -585,7 +585,7 @@ public void testInvalidCharacterInIndexPattern() {
585585
if (command.contains("LOOKUP_🐔")) {
586586
continue;
587587
}
588-
lineNumber = command.contains("FROM") ? "line 1:10: " : "line 1:13: ";
588+
lineNumber = command.contains("FROM") ? "line 1:9: " : "line 1:12: ";
589589
clustersAndIndices(command, "*", "-index#pattern");
590590
clustersAndIndices(command, "index*", "-index#pattern");
591591
clustersAndIndices(command, "*", "-<--logstash-{now/M{yyyy.MM}}>");
@@ -885,18 +885,18 @@ public void testSuggestAvailableProcessingCommandsOnParsingError() {
885885
public void testDeprecatedIsNullFunction() {
886886
expectError(
887887
"from test | eval x = is_null(f)",
888-
"line 1:23: is_null function is not supported anymore, please use 'is null'/'is not null' predicates instead"
888+
"line 1:22: is_null function is not supported anymore, please use 'is null'/'is not null' predicates instead"
889889
);
890890
expectError(
891891
"row x = is_null(f)",
892-
"line 1:10: is_null function is not supported anymore, please use 'is null'/'is not null' predicates instead"
892+
"line 1:9: is_null function is not supported anymore, please use 'is null'/'is not null' predicates instead"
893893
);
894894

895895
if (Build.current().isSnapshot()) {
896896
expectError(
897897
"from test | eval x = ?fn1(f)",
898898
List.of(paramAsIdentifier("fn1", "IS_NULL")),
899-
"line 1:23: is_null function is not supported anymore, please use 'is null'/'is not null' predicates instead"
899+
"line 1:22: is_null function is not supported anymore, please use 'is null'/'is not null' predicates instead"
900900
);
901901
}
902902
}
@@ -911,23 +911,23 @@ public void testMetadataFieldOnOtherSources() {
911911
}
912912

913913
public void testMetadataFieldMultipleDeclarations() {
914-
expectError("from test metadata _index, _version, _index", "1:39: metadata field [_index] already declared [@1:20]");
914+
expectError("from test metadata _index, _version, _index", "1:38: metadata field [_index] already declared [@1:20]");
915915
}
916916

917917
public void testMetadataFieldUnsupportedPrimitiveType() {
918-
expectError("from test metadata _tier", "line 1:21: unsupported metadata field [_tier]");
918+
expectError("from test metadata _tier", "line 1:20: unsupported metadata field [_tier]");
919919
}
920920

921921
public void testMetadataFieldUnsupportedCustomType() {
922-
expectError("from test metadata _feature", "line 1:21: unsupported metadata field [_feature]");
922+
expectError("from test metadata _feature", "line 1:20: unsupported metadata field [_feature]");
923923
}
924924

925925
public void testMetadataFieldNotFoundNonExistent() {
926-
expectError("from test metadata _doesnot_compute", "line 1:21: unsupported metadata field [_doesnot_compute]");
926+
expectError("from test metadata _doesnot_compute", "line 1:20: unsupported metadata field [_doesnot_compute]");
927927
}
928928

929929
public void testMetadataFieldNotFoundNormalField() {
930-
expectError("from test metadata emp_no", "line 1:21: unsupported metadata field [emp_no]");
930+
expectError("from test metadata emp_no", "line 1:20: unsupported metadata field [emp_no]");
931931
}
932932

933933
public void testDissectPattern() {
@@ -985,13 +985,13 @@ public void testGrokPattern() {
985985

986986
expectError(
987987
"row a = \"foo bar\" | GROK a \"%{NUMBER:foo} %{WORD:foo}\"",
988-
"line 1:22: Invalid GROK pattern [%{NUMBER:foo} %{WORD:foo}]:"
988+
"line 1:21: Invalid GROK pattern [%{NUMBER:foo} %{WORD:foo}]:"
989989
+ " the attribute [foo] is defined multiple times with different types"
990990
);
991991

992992
expectError(
993993
"row a = \"foo\" | GROK a \"(?P<justification>.+)\"",
994-
"line 1:18: Invalid grok pattern [(?P<justification>.+)]: [undefined group option]"
994+
"line 1:17: Invalid grok pattern [(?P<justification>.+)]: [undefined group option]"
995995
);
996996
}
997997

@@ -1015,7 +1015,7 @@ public void testLikeRLike() {
10151015

10161016
expectError(
10171017
"from a | where foo like \"(?i)(^|[^a-zA-Z0-9_-])nmap($|\\\\.)\"",
1018-
"line 1:17: Invalid pattern for LIKE [(?i)(^|[^a-zA-Z0-9_-])nmap($|\\.)]: "
1018+
"line 1:16: Invalid pattern for LIKE [(?i)(^|[^a-zA-Z0-9_-])nmap($|\\.)]: "
10191019
+ "[Invalid sequence - escape character is not followed by special wildcard char]"
10201020
);
10211021
}
@@ -1076,7 +1076,7 @@ public void testEnrich() {
10761076
);
10771077
expectError(
10781078
"from a | enrich typo:countries on foo",
1079-
"line 1:18: Unrecognized value [typo], ENRICH policy qualifier needs to be one of [_ANY, _COORDINATOR, _REMOTE]"
1079+
"line 1:17: Unrecognized value [typo], ENRICH policy qualifier needs to be one of [_ANY, _COORDINATOR, _REMOTE]"
10801080
);
10811081
}
10821082

@@ -1261,8 +1261,8 @@ public void testInvalidPositionalParams() {
12611261
expectError(
12621262
"from test | where x < ?0 and y < ?2",
12631263
List.of(paramAsConstant(null, 5)),
1264-
"line 1:24: No parameter is defined for position 0, did you mean position 1?; "
1265-
+ "line 1:35: No parameter is defined for position 2, did you mean position 1?"
1264+
"line 1:23: No parameter is defined for position 0, did you mean position 1?; "
1265+
+ "line 1:34: No parameter is defined for position 2, did you mean position 1?"
12661266
);
12671267

12681268
expectError(
@@ -2107,11 +2107,11 @@ public void testEnrichOnMatchField() {
21072107
}
21082108

21092109
public void testInlineConvertWithNonexistentType() {
2110-
expectError("ROW 1::doesnotexist", "line 1:9: Unknown data type named [doesnotexist]");
2111-
expectError("ROW \"1\"::doesnotexist", "line 1:11: Unknown data type named [doesnotexist]");
2112-
expectError("ROW false::doesnotexist", "line 1:13: Unknown data type named [doesnotexist]");
2113-
expectError("ROW abs(1)::doesnotexist", "line 1:14: Unknown data type named [doesnotexist]");
2114-
expectError("ROW (1+2)::doesnotexist", "line 1:13: Unknown data type named [doesnotexist]");
2110+
expectError("ROW 1::doesnotexist", "line 1:8: Unknown data type named [doesnotexist]");
2111+
expectError("ROW \"1\"::doesnotexist", "line 1:10: Unknown data type named [doesnotexist]");
2112+
expectError("ROW false::doesnotexist", "line 1:12: Unknown data type named [doesnotexist]");
2113+
expectError("ROW abs(1)::doesnotexist", "line 1:13: Unknown data type named [doesnotexist]");
2114+
expectError("ROW (1+2)::doesnotexist", "line 1:12: Unknown data type named [doesnotexist]");
21152115
}
21162116

21172117
public void testLookup() {
@@ -2131,7 +2131,7 @@ public void testLookup() {
21312131
}
21322132

21332133
public void testInlineConvertUnsupportedType() {
2134-
expectError("ROW 3::BYTE", "line 1:6: Unsupported conversion to type [BYTE]");
2134+
expectError("ROW 3::BYTE", "line 1:5: Unsupported conversion to type [BYTE]");
21352135
}
21362136

21372137
public void testMetricsWithoutStats() {

0 commit comments

Comments
 (0)