Implement review suggestions

Author: luigidellaquila

x-pack/plugin/esql/src/main/antlr/lexer/Set.g4

@@ -7,7 +7,7 @@
 lexer grammar Rename;
 
 //
-// SET key="value", ke2="value2"...
+// SET key="value"
 //
 SET : 'set'             -> pushMode(SET_MODE);
 

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

@@ -20,7 +20,7 @@
 import org.elasticsearch.logging.Logger;
 import org.elasticsearch.xpack.esql.core.util.StringUtils;
 import org.elasticsearch.xpack.esql.expression.function.EsqlFunctionRegistry;
-import org.elasticsearch.xpack.esql.plan.logical.EsqlQuery;
+import org.elasticsearch.xpack.esql.plan.logical.EsqlStatement;
 import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
 import org.elasticsearch.xpack.esql.session.Configuration;
 import org.elasticsearch.xpack.esql.telemetry.PlanTelemetry;
@@ -117,11 +117,11 @@ public LogicalPlan createStatement(String query, QueryParams params, PlanTelemet
     }
 
     // testing utility
-    public EsqlQuery createQuery(String query, QueryParams params, Configuration configuration) {
+    public EsqlStatement createQuery(String query, QueryParams params, Configuration configuration) {
         return createQuery(query, params, new PlanTelemetry(new EsqlFunctionRegistry()), configuration);
     }
 
-    public EsqlQuery createQuery(String query, QueryParams params, PlanTelemetry metrics, Configuration configuration) {
+    public EsqlStatement createQuery(String query, QueryParams params, PlanTelemetry metrics, Configuration configuration) {
         if (log.isDebugEnabled()) {
             log.debug("Parsing as statement: {}", query);
         }

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

@@ -53,7 +53,7 @@
 import org.elasticsearch.xpack.esql.plan.logical.Dissect;
 import org.elasticsearch.xpack.esql.plan.logical.Drop;
 import org.elasticsearch.xpack.esql.plan.logical.Enrich;
-import org.elasticsearch.xpack.esql.plan.logical.EsqlQuery;
+import org.elasticsearch.xpack.esql.plan.logical.EsqlStatement;
 import org.elasticsearch.xpack.esql.plan.logical.Eval;
 import org.elasticsearch.xpack.esql.plan.logical.Explain;
 import org.elasticsearch.xpack.esql.plan.logical.Filter;
@@ -120,8 +120,8 @@ public LogicalPlanBuilder(ParsingContext context) {
 
     private int queryDepth = 0;
 
-    protected EsqlQuery query(ParseTree ctx) {
-        EsqlQuery p = typedParsing(this, ctx, EsqlQuery.class);
+    protected EsqlStatement query(ParseTree ctx) {
+        EsqlStatement p = typedParsing(this, ctx, EsqlStatement.class);
         return p;
     }
 
@@ -143,14 +143,14 @@ protected LogicalPlan plan(ParseTree ctx) {
     }
 
     @Override
-    public EsqlQuery visitStatements(EsqlBaseParser.StatementsContext ctx) {
+    public EsqlStatement visitStatements(EsqlBaseParser.StatementsContext ctx) {
         List<QuerySetting> settings = new ArrayList<>();
         for (EsqlBaseParser.SetCommandContext setCommandContext : ctx.setCommand()) {
             settings.add(visitSetCommand(setCommandContext));
         }
 
         LogicalPlan query = visitSingleStatement(ctx.singleStatement());
-        return new EsqlQuery(query, settings);
+        return new EsqlStatement(query, settings);
     }
 
     protected List<LogicalPlan> plans(List<? extends ParserRuleContext> ctxs) {

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/EsqlQuery.java renamed to x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/EsqlStatement.java

@@ -11,10 +11,10 @@
 
 import java.util.List;
 
-public record EsqlQuery(LogicalPlan plan, List<QuerySetting> settings) {
+public record EsqlStatement(LogicalPlan plan, List<QuerySetting> settings) {
     /**
      * Returns the expression corresponding to a setting value.
-     * If the setting name appears multiple times (in one or more QuerySettings objects), this will return last occurrence.
+     * If the setting name appears multiple times, this will return last occurrence.
      *
      * @param name the setting name
      */

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java

@@ -60,7 +60,7 @@
 import org.elasticsearch.xpack.esql.parser.EsqlParser;
 import org.elasticsearch.xpack.esql.parser.QueryParams;
 import org.elasticsearch.xpack.esql.plan.IndexPattern;
-import org.elasticsearch.xpack.esql.plan.logical.EsqlQuery;
+import org.elasticsearch.xpack.esql.plan.logical.EsqlStatement;
 import org.elasticsearch.xpack.esql.plan.logical.Explain;
 import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
 import org.elasticsearch.xpack.esql.plan.logical.join.InlineJoin;
@@ -85,6 +85,7 @@
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import static java.util.stream.Collectors.joining;
 import static java.util.stream.Collectors.toSet;
 import static org.elasticsearch.index.query.QueryBuilders.boolQuery;
 import static org.elasticsearch.xpack.esql.core.tree.Source.EMPTY;
@@ -169,8 +170,8 @@ public void execute(EsqlQueryRequest request, EsqlExecutionInfo executionInfo, P
         assert ThreadPool.assertCurrentThreadPool(ThreadPool.Names.SEARCH);
         assert executionInfo != null : "Null EsqlExecutionInfo";
         LOGGER.debug("ESQL query:\n{}", request.query());
-        EsqlQuery query = parse(request.query(), request.params());
-        LogicalPlan plan = query.plan();
+        EsqlStatement statement = parse(request.query(), request.params());
+        LogicalPlan plan = statement.plan();
         if (plan instanceof Explain explain) {
             explainMode = true;
             plan = explain.query();
@@ -309,9 +310,15 @@ private static LocalRelation resultToPlan(LogicalPlan plan, Result result) {
         return new LocalRelation(plan.source(), schema, LocalSupplier.of(blocks));
     }
 
-    private EsqlQuery parse(String query, QueryParams params) {
+    private EsqlStatement parse(String query, QueryParams params) {
         var parsed = new EsqlParser().createQuery(query, params, planTelemetry, configuration);
-        LOGGER.debug("Parsed logical plan:\n{}", parsed);
+        if (LOGGER.isDebugEnabled()) {
+            LOGGER.debug("Parsed logical plan:\n{}", parsed.plan());
+            LOGGER.debug(
+                "Parsed settings:\n[{}]",
+                parsed.settings().stream().map(x -> x.field().name() + "=" + x.value()).collect(joining(", "))
+            );
+        }
         return parsed;
     }
 

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

@@ -25,7 +25,7 @@
 import org.elasticsearch.xpack.esql.parser.ParsingException;
 import org.elasticsearch.xpack.esql.parser.QueryParam;
 import org.elasticsearch.xpack.esql.parser.QueryParams;
-import org.elasticsearch.xpack.esql.plan.logical.EsqlQuery;
+import org.elasticsearch.xpack.esql.plan.logical.EsqlStatement;
 import org.elasticsearch.xpack.esql.plan.logical.Eval;
 import org.elasticsearch.xpack.esql.plan.logical.Limit;
 import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
@@ -213,7 +213,7 @@ public void testInvalidSample() {
     }
 
     public void testSet() {
-        EsqlQuery query = parse("SET foo = \"bar\"; row a = 1", new QueryParams());
+        EsqlStatement query = parse("SET foo = \"bar\"; row a = 1", new QueryParams());
         assertThat(query.plan(), is(instanceOf(Row.class)));
         assertThat(query.settings().size(), is(1));
         checkSetting(query, 0, "foo", BytesRefs.toBytesRef("bar"));
@@ -232,7 +232,7 @@ public void testSet() {
     }
 
     public void testSetWithTripleQuotes() {
-        EsqlQuery query = parse("SET foo = \"\"\"bar\"baz\"\"\"; row a = 1", new QueryParams());
+        EsqlStatement query = parse("SET foo = \"\"\"bar\"baz\"\"\"; row a = 1", new QueryParams());
         assertThat(query.plan(), is(instanceOf(Row.class)));
         assertThat(query.settings().size(), is(1));
         checkSetting(query, 0, "foo", BytesRefs.toBytesRef("bar\"baz"));
@@ -249,7 +249,7 @@ public void testSetWithTripleQuotes() {
     }
 
     public void testMultipleSet() {
-        EsqlQuery query = parse(
+        EsqlStatement query = parse(
             "SET foo = \"bar\"; SET bar = 2; SET foo = \"baz\"; SET x = 3.5; SET y = false; SET z = null; row a = 1",
             new QueryParams()
         );
@@ -265,7 +265,7 @@ public void testMultipleSet() {
     }
 
     public void testSetArrays() {
-        EsqlQuery query = parse("SET foo = [\"bar\", \"baz\"]; SET bar = [1, 2, 3]; row a = 1", new QueryParams());
+        EsqlStatement query = parse("SET foo = [\"bar\", \"baz\"]; SET bar = [1, 2, 3]; row a = 1", new QueryParams());
         assertThat(query.plan(), is(instanceOf(Row.class)));
         assertThat(query.settings().size(), is(2));
 
@@ -274,7 +274,7 @@ public void testSetArrays() {
     }
 
     public void testSetWithNamedParams() {
-        EsqlQuery query = parse(
+        EsqlStatement query = parse(
             "SET foo = \"bar\"; SET bar = ?a; SET foo = \"baz\"; SET x = ?x; row a = 1",
             new QueryParams(
                 List.of(
@@ -293,7 +293,7 @@ public void testSetWithNamedParams() {
     }
 
     public void testSetWithPositionalParams() {
-        EsqlQuery query = parse(
+        EsqlStatement query = parse(
             "SET foo = \"bar\"; SET bar = ?; SET foo = \"baz\"; SET x = ?; row a = ?",
             new QueryParams(
                 List.of(
@@ -319,7 +319,7 @@ public void testSetWithPositionalParams() {
      * @param name     the setting name
      * @param value    the setting value as it appears in the query at that position
      */
-    private void checkSetting(EsqlQuery query, int position, String name, Object value) {
+    private void checkSetting(EsqlStatement query, int position, String name, Object value) {
         checkSetting(query, position, name, value, value);
     }
 
@@ -331,17 +331,17 @@ private void checkSetting(EsqlQuery query, int position, String name, Object val
      * @param maskingValue the final value you'll obtain if you use query.setting(name).
      *                     It could be different from value in case of name collisions in the query
      */
-    private void checkSetting(EsqlQuery query, int position, String name, Object value, Object maskingValue) {
+    private void checkSetting(EsqlStatement query, int position, String name, Object value, Object maskingValue) {
         assertThat(settingName(query, position), is(name));
         assertThat(settingValue(query, position), is(value));
         assertThat(query.setting(name).fold(FoldContext.small()), is(maskingValue));
     }
 
-    private String settingName(EsqlQuery query, int position) {
+    private String settingName(EsqlStatement query, int position) {
         return query.settings().get(position).name();
     }
 
-    private Object settingValue(EsqlQuery query, int position) {
+    private Object settingValue(EsqlStatement query, int position) {
         return query.settings().get(position).value().fold(FoldContext.small());
     }
 
@@ -352,7 +352,7 @@ private String error(String query, QueryParams params) {
         return message.substring("line ".length());
     }
 
-    private EsqlQuery parse(String query, QueryParams params) {
+    private EsqlStatement parse(String query, QueryParams params) {
         return parser.createQuery(query, params, TEST_CFG);
     }