update according to review comments - refactor, comments

Author: fang-xing-esql

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

@@ -108,25 +108,7 @@ protected LogicalPlan plan(ParseTree ctx) {
         if (errors.hasNext() == false) {
             return p;
         } else {
-            StringBuilder message = new StringBuilder();
-            int i = 0;
-            int line = -1;
-            int charPositionInLine = -1;
-
-            while (errors.hasNext()) {
-                ParsingException e = errors.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++;
-            }
-            throw new ParsingException(line, charPositionInLine, 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,12 +43,38 @@ public ParsingException(Exception cause, Source source, String message, Object..
         this.charPositionInLine = source.source().getColumnNumber();
     }
 
-    public ParsingException(int line, int charPositionInLine, String message, Object... args) {
+    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;
     }