Skip to content

Add support for parameters in LIMIT command - #128464

Merged
luigidellaquila merged 5 commits into
elastic:mainfrom
luigidellaquila:esql/limit_with_param
May 27, 2025
Merged

luigidellaquila merged 5 commits into
elastic:mainfrom
luigidellaquila:esql/limit_with_param

Conversation

@luigidellaquila

Copy link
Copy Markdown
Member

Adding support for positional and named parameters in LIMIT command, eg.

{
  "query":"FROM foo | LIMIT ?lmt",
  "params":[{"lmt":10}]
}

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 */);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doing this here for consistency with the previous behavior and to fail fast, but it could be moved to the verification phase

@elasticsearchmachine elasticsearchmachine added needs:triage Requires assignment of a team area label v9.1.0 labels May 26, 2025
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label May 27, 2025
@elasticsearchmachine

Copy link
Copy Markdown
Collaborator

Pinging @elastic/es-analytical-engine (Team:Analytics)

@elasticsearchmachine elasticsearchmachine removed the needs:triage Requires assignment of a team area label label May 27, 2025
@elasticsearchmachine

Copy link
Copy Markdown
Collaborator

Hi @luigidellaquila, I've created a changelog YAML for you.

@astefan
astefan requested a review from fang-xing-esql May 27, 2025 07:49

@fang-xing-esql fang-xing-esql left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thank you @luigidellaquila !

@luigidellaquila luigidellaquila added auto-backport Automatically create backport pull requests when merged v8.19.0 labels May 27, 2025
@luigidellaquila
luigidellaquila enabled auto-merge (squash) May 27, 2025 14:55
@luigidellaquila
luigidellaquila merged commit 6d63670 into elastic:main May 27, 2025
luigidellaquila added a commit to luigidellaquila/elasticsearch that referenced this pull request May 30, 2025
@luigidellaquila

Copy link
Copy Markdown
Member Author

Manual backport: #128655
(for some reason the automatic backport didn't trigger)

@stratoula stratoula added the ES|QL-ui Impacts ES|QL UI label Jun 2, 2025
rylnd added a commit to elastic/kibana that referenced this pull request Jun 6, 2025
… creation (#222467)

A [recent change](elastic/elasticsearch#128464)
to the ES|QL syntax seems to have broken this test, which assumed that
the clause `limit 5 true` is a syntax error in ES|QL.

Regardless of whether or not that's correct, the test just needs a
syntactically-invalid (and non-aggregating) query, which it now has,
again. Closes #222182.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 6, 2025
… creation (elastic#222467)

A [recent change](elastic/elasticsearch#128464)
to the ES|QL syntax seems to have broken this test, which assumed that
the clause `limit 5 true` is a syntax error in ES|QL.

Regardless of whether or not that's correct, the test just needs a
syntactically-invalid (and non-aggregating) query, which it now has,
again. Closes elastic#222182.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 0fb44f8)
kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Jun 6, 2025
… creation (elastic#222467)

A [recent change](elastic/elasticsearch#128464)
to the ES|QL syntax seems to have broken this test, which assumed that
the clause `limit 5 true` is a syntax error in ES|QL.

Regardless of whether or not that's correct, the test just needs a
syntactically-invalid (and non-aggregating) query, which it now has,
again. Closes elastic#222182.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios

(cherry picked from commit 0fb44f8)
pmuellr pushed a commit to pmuellr/kibana that referenced this pull request Jun 11, 2025
… creation (elastic#222467)

A [recent change](elastic/elasticsearch#128464)
to the ES|QL syntax seems to have broken this test, which assumed that
the clause `limit 5 true` is a syntax error in ES|QL.

Regardless of whether or not that's correct, the test just needs a
syntactically-invalid (and non-aggregating) query, which it now has,
again. Closes elastic#222182.

### Checklist

- [x] [Unit or functional
tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html)
were updated or added to match the most common scenarios
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL auto-backport Automatically create backport pull requests when merged >enhancement ES|QL-ui Impacts ES|QL UI Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.19.0 v9.1.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants