[8.18] [ES|QL] Correct line and column numbers of missing named parameters (#120852)

docs/changelog/120852.yaml

@@ -0,0 +1,5 @@
+pr: 120852
+summary: Correct line and column numbers of missing named parameters
+area: ES|QL
+type: bug
+issues: []

x-pack/plugin/esql/qa/server/src/main/java/org/elasticsearch/xpack/esql/qa/rest/RestEsqlTestCase.java

@@ -774,6 +774,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",

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

@@ -110,17 +110,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);
         }
     }
 

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

@@ -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 {
@@ -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);
     }
@@ -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;
     }