Add support for parameters in LIMIT command (elastic#128464)

Author: luigidellaquila

docs/changelog/128464.yaml

@@ -0,0 +1,5 @@
+pr: 128464
+summary: Add support for parameters in LIMIT command
+area: ES|QL
+type: enhancement
+issues: []

x-pack/plugin/esql/src/main/antlr/EsqlBaseParser.g4

@@ -187,7 +187,7 @@ identifierOrParameter
     ;
 
 limitCommand
-    : LIMIT INTEGER_LITERAL
+    : LIMIT constant
     ;
 
 sortCommand

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java

@@ -1128,7 +1128,12 @@ public enum Cap {
         /**
          * Dense vector field type support
          */
-        DENSE_VECTOR_FIELD_TYPE(EsqlCorePlugin.DENSE_VECTOR_FEATURE_FLAG);
+        DENSE_VECTOR_FIELD_TYPE(EsqlCorePlugin.DENSE_VECTOR_FEATURE_FLAG),
+
+        /**
+         * Support parameters for LiMIT command.
+         */
+        PARAMETER_FOR_LIMIT;
 
         private final boolean enabled;
 

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

@@ -95,12 +95,10 @@
 import static org.elasticsearch.xpack.esql.parser.ParserUtils.typedParsing;
 import static org.elasticsearch.xpack.esql.parser.ParserUtils.visitList;
 import static org.elasticsearch.xpack.esql.plan.logical.Enrich.Mode;
-import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.stringToInt;
 
 /**
  * Translates what we get back from Antlr into the data structures the rest of the planner steps will act on.  Generally speaking, things
  * which change the grammar will need to make changes here as well.
- *
  */
 public class LogicalPlanBuilder extends ExpressionBuilder {
 
@@ -384,8 +382,18 @@ public PlanFactory visitWhereCommand(EsqlBaseParser.WhereCommandContext ctx) {
     @Override
     public PlanFactory visitLimitCommand(EsqlBaseParser.LimitCommandContext ctx) {
         Source source = source(ctx);
-        int limit = stringToInt(ctx.INTEGER_LITERAL().getText());
-        return input -> new Limit(source, new Literal(source, limit, DataType.INTEGER), input);
+        Object val = expression(ctx.constant()).fold(FoldContext.small() /* TODO remove me */);
+        if (val instanceof Integer i) {
+            if (i < 0) {
+                throw new ParsingException(source, "Invalid value for LIMIT [" + val + "], expecting a non negative integer");
+            }
+            return input -> new Limit(source, new Literal(source, i, DataType.INTEGER), input);
+        } else {
+            throw new ParsingException(
+                source,
+                "Invalid value for LIMIT [" + val + ": " + val.getClass().getSimpleName() + "], expecting a non negative integer"
+            );
+        }
     }
 
     @Override

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

@@ -3304,7 +3304,7 @@ public void testForkError() {
                    ( WHERE emp_no > 5 )
                    ( WHERE emp_no > 6 | SORT emp_no | LIMIT 5 )
             """));
-        assertThat(pe.getMessage(), containsString("mismatched input 'me' expecting INTEGER_LITERAL"));
+        assertThat(pe.getMessage(), containsString("mismatched input 'me' expecting {"));
 
         e = expectThrows(VerificationException.class, () -> analyze("""
             FROM test

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

@@ -156,6 +156,12 @@ public void testJoinTwiceOnTheSameField_TwoLookups() {
         );
     }
 
+    public void testInvalidLimit() {
+        assertEquals("1:13: Invalid value for LIMIT [foo: String], expecting a non negative integer", error("row a = 1 | limit \"foo\""));
+        assertEquals("1:13: Invalid value for LIMIT [1.2: Double], expecting a non negative integer", error("row a = 1 | limit 1.2"));
+        assertEquals("1:13: Invalid value for LIMIT [-1], expecting a non negative integer", error("row a = 1 | limit -1"));
+    }
+
     private String error(String query) {
         ParsingException e = expectThrows(ParsingException.class, () -> defaultAnalyzer.analyze(parser.createStatement(query)));
         String message = e.getMessage();

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

@@ -858,7 +858,7 @@ public void testBasicLimitCommand() {
     }
 
     public void testLimitConstraints() {
-        expectError("from text | limit -1", "line 1:19: extraneous input '-' expecting INTEGER_LITERAL");
+        expectError("from text | limit -1", "line 1:13: Invalid value for LIMIT [-1], expecting a non negative integer");
     }
 
     public void testBasicSortCommand() {
@@ -3029,7 +3029,7 @@ public void testNamedFunctionArgumentInInvalidPositions() {
             Map.entry("drop {}", "line 1:18: token recognition error at: '{'"),
             Map.entry("rename a as {}", "line 1:25: token recognition error at: '{'"),
             Map.entry("mv_expand {}", "line 1:23: token recognition error at: '{'"),
-            Map.entry("limit {}", "line 1:19: mismatched input '{' expecting INTEGER_LITERAL"),
+            Map.entry("limit {}", "line 1:19: extraneous input '{' expecting {QUOTED_STRING"),
             Map.entry("enrich idx2 on f1 with f2 = {}", "line 1:41: token recognition error at: '{'"),
             Map.entry("dissect {} \"%{bar}\"", "line 1:21: extraneous input '{' expecting {QUOTED_STRING, INTEGER_LITERAL"),
             Map.entry("grok {} \"%{WORD:foo}\"", "line 1:18: extraneous input '{' expecting {QUOTED_STRING, INTEGER_LITERAL")

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

@@ -412,6 +412,41 @@ FROM EVAL SORT LIMIT with documents_found:
   - match: {values.1: ["1",2.0,null,true,123,1674835275193]}
   - match: {values.2: ["1",2.0,null,true,123,1674835275193]}
 
+---
+"Test Unnamed Input Params Also For Limit":
+  - requires:
+        test_runner_features: [ capabilities ]
+        capabilities:
+          - method: POST
+            path: /_query
+            parameters: [ ]
+            capabilities: [ parameter_for_limit ]
+        reason: "named or positional parameters"
+  - do:
+      esql.query:
+        body:
+          query: 'from test | eval x = ?, y = ?, z = ?, t = ?, u = ?, v = ? | keep x, y, z, t, u, v | limit ?'
+          params: ["1", 2.0, null, true, 123, 1674835275193, 3]
+
+  - length: {columns: 6}
+  - match: {columns.0.name: "x"}
+  - match: {columns.0.type: "keyword"}
+  - match: {columns.1.name: "y"}
+  - match: {columns.1.type: "double"}
+  - match: {columns.2.name: "z"}
+  - match: {columns.2.type: "null"}
+  - match: {columns.3.name: "t"}
+  - match: {columns.3.type: "boolean"}
+  - match: {columns.4.name: "u"}
+  - match: {columns.4.type: "integer"}
+  - match: {columns.5.name: "v"}
+  - match: {columns.5.type: "long"}
+  - length: {values: 3}
+  - match: {values.0: ["1",2.0,null,true,123,1674835275193]}
+  - match: {values.1: ["1",2.0,null,true,123,1674835275193]}
+  - match: {values.2: ["1",2.0,null,true,123,1674835275193]}
+
+
 ---
 "Test Named Input Params":
   - requires:
@@ -420,14 +455,14 @@ FROM EVAL SORT LIMIT with documents_found:
         - method: POST
           path: /_query
           parameters: [ ]
-          capabilities: [ named_positional_parameter ]
+          capabilities: [ parameter_for_limit ]
       reason: "named or positional parameters"
 
   - do:
       esql.query:
         body:
-          query: 'from test | eval x = ?, y = ?, z = ?, t = ?, u = ?, v = ? | keep x, y, z, t, u, v | limit 3'
-          params: [{"n1" : "1"}, {"n2" : 2.0}, {"n3" : null}, {"n4" : true}, {"n5" : 123}, {"n6": 1674835275193}]
+          query: 'from test | eval x = ?, y = ?, z = ?, t = ?, u = ?, v = ? | keep x, y, z, t, u, v | limit ?'
+          params: [{"n1" : "1"}, {"n2" : 2.0}, {"n3" : null}, {"n4" : true}, {"n5" : 123}, {"n6": 1674835275193}, {"l": 3}]
 
   - length: {columns: 6}
   - match: {columns.0.name: "x"}
@@ -484,14 +519,14 @@ FROM EVAL SORT LIMIT with documents_found:
         - method: POST
           path: /_query
           parameters: [ ]
-          capabilities: [ named_parameter_for_field_and_function_names_simplified_syntax ]
+          capabilities: [ parameter_for_limit ]
       reason: "named or positional parameters for field names"
 
   - do:
       esql.query:
         body:
-          query: 'from test | stats x = count(?f1), y = sum(?f2) by ?f3 | sort ?f3 | keep ?f3, x, y  | limit 3'
-          params: [{"f1" : {"identifier" : "time"}}, {"f2" : { "identifier" : "count" }}, {"f3" : { "identifier" : "color"}}]
+          query: 'from test | stats x = count(?f1), y = sum(?f2) by ?f3 | sort ?f3 | keep ?f3, x, y  | limit ?l'
+          params: [{"f1" : {"identifier" : "time"}}, {"f2" : { "identifier" : "count" }}, {"f3" : { "identifier" : "color"}}, {"l": 3}]
 
   - length: {columns: 3}
   - match: {columns.0.name: "color"}
@@ -505,6 +540,27 @@ FROM EVAL SORT LIMIT with documents_found:
   - match: {values.1: ["green",10,440]}
   - match: {values.2: ["red",20,860]}
 
+
+---
+"Test wrong LIMIT parameter":
+  - requires:
+      test_runner_features: [ capabilities ]
+      capabilities:
+        - method: POST
+          path: /_query
+          parameters: [ ]
+          capabilities: [ parameter_for_limit ]
+      reason: "named or positional parameters for field names"
+
+  - do:
+      catch: "/Invalid value for LIMIT \\[foo: String\\], expecting a non negative integer/"
+      esql.query:
+        body:
+          query: 'from test | limit ?l'
+          params: [{"l": "foo"}]
+
+
+
 ---
 version is not allowed:
   - requires: