Skip to content

Commit

Permalink
[ES|QL] Correct line and column numbers of missing named parameters (#…
Browse files Browse the repository at this point in the history
…120852)

* correct line and column numbers of missing named parameters
  • Loading branch information
fang-xing-esql authored Jan 31, 2025
1 parent d9da7c9 commit 06fee76
Show file tree
Hide file tree
Showing 4 changed files with 71 additions and 11 deletions.
5 changes: 5 additions & 0 deletions docs/changelog/120852.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
pr: 120852
summary: Correct line and column numbers of missing named parameters
area: ES|QL
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -773,6 +773,33 @@ public void testNamedParamsForIdentifierAndIdentifierPatterns() throws IOExcepti
}
}

public void testErrorMessageForMissingParams() throws IOException {
ResponseException re = expectThrows(
ResponseException.class,
() -> runEsql(requestObjectBuilder().query("from idx | where x == ?n1").params("[]"))
);
assertThat(
EntityUtils.toString(re.getResponse().getEntity()).replaceAll("\\\\\n\s+\\\\", ""),
containsString("line 1:23: Unknown query parameter [n1]")
);

re = expectThrows(
ResponseException.class,
() -> runEsql(requestObjectBuilder().query("from idx | where x == ?n1 and y == ?n2").params("[{\"n\" : \"v\"}]"))
);
assertThat(EntityUtils.toString(re.getResponse().getEntity()).replaceAll("\\\\\n\s+\\\\", ""), containsString("""
line 1:23: Unknown query parameter [n1], did you mean [n]?; line 1:36: Unknown query parameter [n2], did you mean [n]?"""));

re = expectThrows(
ResponseException.class,
() -> runEsql(requestObjectBuilder().query("from idx | where x == ?n1 and y == ?n2").params("[{\"n1\" : \"v1\"}]"))
);
assertThat(
EntityUtils.toString(re.getResponse().getEntity()).replaceAll("\\\\\n\s+\\\\", ""),
containsString("line 1:36: Unknown query parameter [n2], did you mean [n1]")
);
}

public void testErrorMessageForLiteralDateMathOverflow() throws IOException {
List<String> dateMathOverflowExpressions = List.of(
"2147483647 day + 1 day",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -108,17 +108,7 @@ protected LogicalPlan plan(ParseTree ctx) {
if (errors.hasNext() == false) {
return p;
} else {
StringBuilder message = new StringBuilder();
int i = 0;

while (errors.hasNext()) {
if (i > 0) {
message.append("; ");
}
message.append(errors.next().getMessage());
i++;
}
throw new ParsingException(message.toString());
throw ParsingException.combineParsingExceptions(errors);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
import org.elasticsearch.xpack.esql.EsqlClientException;
import org.elasticsearch.xpack.esql.core.tree.Source;

import java.util.Iterator;

import static org.elasticsearch.common.logging.LoggerMessageFormat.format;

public class ParsingException extends EsqlClientException {
Expand All @@ -21,6 +23,10 @@ public ParsingException(String message, Exception cause, int line, int charPosit
this.charPositionInLine = charPositionInLine + 1;
}

/**
* To be used only if the exception cannot be associated with a specific position in the query.
* Error message will start with {@code line -1:-1:} instead of using specific location.
*/
public ParsingException(String message, Object... args) {
this(Source.EMPTY, message, args);
}
Expand All @@ -37,6 +43,38 @@ public ParsingException(Exception cause, Source source, String message, Object..
this.charPositionInLine = source.source().getColumnNumber();
}

private ParsingException(int line, int charPositionInLine, String message, Object... args) {
super(message, args);
this.line = line;
this.charPositionInLine = charPositionInLine;
}

/**
* Combine multiple {@code ParsingException} into one, this is used by {@code LogicalPlanBuilder} to
* consolidate multiple named parameters related {@code ParsingException}.
*/
public static ParsingException combineParsingExceptions(Iterator<ParsingException> parsingExceptions) {
StringBuilder message = new StringBuilder();
int i = 0;
int line = -1;
int charPositionInLine = -1;

while (parsingExceptions.hasNext()) {
ParsingException e = parsingExceptions.next();
if (i > 0) {
message.append("; ");
message.append(e.getMessage());
} else {
// line and column numbers are the associated with the first error
line = e.getLineNumber();
charPositionInLine = e.getColumnNumber();
message.append(e.getErrorMessage());
}
i++;
}
return new ParsingException(line, charPositionInLine, message.toString());
}

public int getLineNumber() {
return line;
}
Expand Down

0 comments on commit 06fee76

Please sign in to comment.