Skip to content

Commit f484c9f

Browse files
Add support for parameters in LIMIT command (#128464) (#128655)
1 parent b1cfb39 commit f484c9f

File tree

9 files changed

+186
-104
lines changed

9 files changed

+186
-104
lines changed

docs/changelog/128464.yaml

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
pr: 128464
2+
summary: Add support for parameters in LIMIT command
3+
area: ES|QL
4+
type: enhancement
5+
issues: []

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -244,7 +244,7 @@ identifierOrParameter
244244
;
245245

246246
limitCommand
247-
: LIMIT INTEGER_LITERAL
247+
: LIMIT constant
248248
;
249249

250250
sortCommand

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

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -893,7 +893,12 @@ public enum Cap {
893893
/**
894894
* Guards a bug fix matching {@code TO_LOWER(f) == ""}.
895895
*/
896-
TO_LOWER_EMPTY_STRING;
896+
TO_LOWER_EMPTY_STRING,
897+
898+
/**
899+
* Support parameters for LiMIT command.
900+
*/
901+
PARAMETER_FOR_LIMIT;
897902

898903
private final boolean enabled;
899904

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

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 91 additions & 89 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,12 +85,10 @@
8585
import static org.elasticsearch.xpack.esql.parser.ParserUtils.typedParsing;
8686
import static org.elasticsearch.xpack.esql.parser.ParserUtils.visitList;
8787
import static org.elasticsearch.xpack.esql.plan.logical.Enrich.Mode;
88-
import static org.elasticsearch.xpack.esql.type.EsqlDataTypeConverter.stringToInt;
8988

9089
/**
9190
* Translates what we get back from Antlr into the data structures the rest of the planner steps will act on. Generally speaking, things
9291
* which change the grammar will need to make changes here as well.
93-
*
9492
*/
9593
public class LogicalPlanBuilder extends ExpressionBuilder {
9694

@@ -372,8 +370,18 @@ public PlanFactory visitWhereCommand(EsqlBaseParser.WhereCommandContext ctx) {
372370
@Override
373371
public PlanFactory visitLimitCommand(EsqlBaseParser.LimitCommandContext ctx) {
374372
Source source = source(ctx);
375-
int limit = stringToInt(ctx.INTEGER_LITERAL().getText());
376-
return input -> new Limit(source, new Literal(source, limit, DataType.INTEGER), input);
373+
Object val = expression(ctx.constant()).fold(FoldContext.small() /* TODO remove me */);
374+
if (val instanceof Integer i) {
375+
if (i < 0) {
376+
throw new ParsingException(source, "Invalid value for LIMIT [" + val + "], expecting a non negative integer");
377+
}
378+
return input -> new Limit(source, new Literal(source, i, DataType.INTEGER), input);
379+
} else {
380+
throw new ParsingException(
381+
source,
382+
"Invalid value for LIMIT [" + val + ": " + val.getClass().getSimpleName() + "], expecting a non negative integer"
383+
);
384+
}
377385
}
378386

379387
@Override

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

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,12 @@ private String functionName(EsqlFunctionRegistry registry, Expression functionCa
161161
throw new IllegalArgumentException("can't find name for " + functionCall);
162162
}
163163

164+
public void testInvalidLimit() {
165+
assertEquals("1:13: Invalid value for LIMIT [foo: String], expecting a non negative integer", error("row a = 1 | limit \"foo\""));
166+
assertEquals("1:13: Invalid value for LIMIT [1.2: Double], expecting a non negative integer", error("row a = 1 | limit 1.2"));
167+
assertEquals("1:13: Invalid value for LIMIT [-1], expecting a non negative integer", error("row a = 1 | limit -1"));
168+
}
169+
164170
private String error(String query) {
165171
ParsingException e = expectThrows(ParsingException.class, () -> defaultAnalyzer.analyze(parser.createStatement(query)));
166172
String message = e.getMessage();

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -885,7 +885,7 @@ public void testBasicLimitCommand() {
885885
}
886886

887887
public void testLimitConstraints() {
888-
expectError("from text | limit -1", "line 1:19: extraneous input '-' expecting INTEGER_LITERAL");
888+
expectError("from text | limit -1", "line 1:13: Invalid value for LIMIT [-1], expecting a non negative integer");
889889
}
890890

891891
public void testBasicSortCommand() {
@@ -3051,7 +3051,7 @@ public void testNamedFunctionArgumentInInvalidPositions() {
30513051
Map.entry("drop {}", "line 1:18: token recognition error at: '{'"),
30523052
Map.entry("rename a as {}", "line 1:25: token recognition error at: '{'"),
30533053
Map.entry("mv_expand {}", "line 1:23: token recognition error at: '{'"),
3054-
Map.entry("limit {}", "line 1:19: mismatched input '{' expecting INTEGER_LITERAL"),
3054+
Map.entry("limit {}", "line 1:19: extraneous input '{' expecting {QUOTED_STRING"),
30553055
Map.entry("enrich idx2 on f1 with f2 = {}", "line 1:41: token recognition error at: '{'"),
30563056
Map.entry("dissect {} \"%{bar}\"", "line 1:21: extraneous input '{' expecting {QUOTED_STRING, INTEGER_LITERAL"),
30573057
Map.entry("grok {} \"%{WORD:foo}\"", "line 1:18: extraneous input '{' expecting {QUOTED_STRING, INTEGER_LITERAL")

0 commit comments

Comments
 (0)