ESQL: Add generative tests for LIMIT BY by ivancea · Pull Request #144238 · elastic/elasticsearch

ivancea · 2026-03-13T17:07:21Z

Generative tests generators for LIMIT BY, which was added in #144069 and #144279

Copilot

Pull request overview

Adds generative test support for the new LIMIT BY command in ESQL, which retains at most N rows per group defined by grouping key expressions.

Changes:

New LimitByGenerator that generates random LIMIT N BY expr1, expr2, ... commands and validates output row counts per group
Registers LimitByGenerator in EsqlQueryGenerator's pipe command list
Blocks full-text functions after LIMIT BY (same as after LIMIT)

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
LimitByGenerator.java	New generator for LIMIT BY commands with per-group row count validation
EsqlQueryGenerator.java	Registers LimitByGenerator in the pipe commands list
FullTextFunctionGenerator.java	Disallows full-text functions after a LIMIT BY command

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

...ures/src/main/java/org/elasticsearch/xpack/esql/generator/command/pipe/LimitByGenerator.java

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

luigidellaquila · 2026-03-16T12:16:54Z

...ures/src/main/java/org/elasticsearch/xpack/esql/generator/command/pipe/LimitByGenerator.java

+    ) {
+        int limit = (int) commandDescription.context().get(LIMIT);
+
+        if (output.size() > previousOutput.size()) {


This check is too strict in general.
Consider a query like following:

FROM idx | LIMIT 10 | WHERE foo > 5 | LIMIT BY bar

The result of the first LIMIT is completely non-deterministic, so the result of the WHERE filtering (also the number of returned records) could be different at every run.
It's not a problem with LIMIT BY, it's just that ES|QL without a sort is not fully deterministic.

I believe the checks are correct as they're very lenient. They check that:

LIMIT BY does not produce more rows than the previous one

There's no more than rows per group

In neither case it's really checking values or strange expectations

Oh my. Fixing it, thanks!

Fixed! We'll ask for another review when the planning logic gets merged

ivancea · 2026-03-18T16:41:31Z

...ures/src/main/java/org/elasticsearch/xpack/esql/generator/command/pipe/LimitByGenerator.java

+        QuerySchema schema,
+        QueryExecutor executor
+    ) {
+        if (previousCommands.stream().anyMatch(cmd -> cmd.commandName().equals(SortGenerator.SORT))) {


Remove this after merging TOP-N BY

elasticsearchmachine · 2026-04-06T12:12:06Z

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

Copilot

Pull request overview

Adds generative-query support for the new ESQL LIMIT … BY … command in the QA random query generator, including validation that the output respects per-group limits, and updates a few generator internals to integrate the new command and normalize multi-word command rendering.

Changes:

Add a new LimitByGenerator that generates LIMIT N BY … pipelines and validates per-group row counts.
Register LimitByGenerator in the pipeline command list and disallow full-text expressions after LIMIT BY.
Normalize StatsGenerator command rendering for underscore-based command names and rename inline-stats’ command name constant.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.

Show a summary per file

File	Description
x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/generator/function/FullTextFunctionGenerator.java	Disallows full-text expressions after `limit_by` (and refactors comparisons to use command constants).
x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/generator/EsqlQueryGenerator.java	Adds `LimitByGenerator` to the set of pipe commands used by the random pipeline generator.
x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/generator/command/pipe/StatsGenerator.java	Renders underscore command names as spaced command text when building the query string.
x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/generator/command/pipe/LimitByGenerator.java	New generator + validator for `LIMIT … BY …` behavior.
x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/generator/command/pipe/InlineStatsGenerator.java	Changes inline-stats command identifier to `inline_stats` (with rendering handled by `StatsGenerator`).

Comments suppressed due to low confidence (1)

x-pack/plugin/esql/qa/testFixtures/src/main/java/org/elasticsearch/xpack/esql/generator/command/pipe/InlineStatsGenerator.java:22

INLINE_STATS was changed from "inline stats" to "inline_stats" (commandName), but there is existing QA logic that switches on command.commandName() using the spaced form (e.g. GenerativeRestTest.updateIndexMapped has case "stats", "inline stats"). With this change, inline stats commands will no longer hit that branch and indexMapped propagation for inline stats will be wrong. Consider updating those consumers to accept "inline_stats" (or better, reference InlineStatsGenerator.INLINE_STATS / normalize by replacing '_' with ' ' consistently).

public class InlineStatsGenerator extends StatsGenerator {
    public static final String INLINE_STATS = "inline_stats";
    public static final CommandGenerator INSTANCE = new InlineStatsGenerator();

    @Override
    public String commandName() {
        return INLINE_STATS;
    }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

...src/main/java/org/elasticsearch/xpack/esql/generator/function/FullTextFunctionGenerator.java

...ures/src/main/java/org/elasticsearch/xpack/esql/generator/command/pipe/LimitByGenerator.java

…improve LimitBy validation of columns

ncordon

LGTM, left minor comments only

...ures/src/main/java/org/elasticsearch/xpack/esql/generator/command/pipe/LimitByGenerator.java

ncordon · 2026-04-06T17:49:50Z

...ures/src/main/java/org/elasticsearch/xpack/esql/generator/command/pipe/LimitByGenerator.java

+        int limit = (int) commandDescription.context().get(LIMIT_CONTEXT);
+
+        if (limit == 0 && output.isEmpty() == false) {
+            return new ValidationResult(false, "LIMIT 0 BY should return no rows, got [" + output.size() + "]");


We are not checking expectSameColumns in this case. Should we?

If any of them fail, we'll report a failure. We can check both things and make a composed message, but I think it's not worth it here (?)

* upstream/main: Mute org.elasticsearch.xpack.esql.expression.function.aggregate.FirstDocIdGroupingAggregatorFunctionTests testSimple elastic#145923 Reindex relocation: store source TaskResult at destination node (elastic#145488) Bump versions after 9.2.8 release [CI] DLMFrozenTransitionServiceTests testCheckForFrozenIndicesReturnsEarlyWhenCapacityExhausted failing [elastic#145778] (elastic#145906) Update branches.json for 9.2.8 release ESQL: Clarify inheriting from Attributes (elastic#145898) Bump versions after 9.3.3 release Update branches.json for 9.3.3 release Prune changelogs after 8.19.14 release Bump versions after 8.19.14 release Update branches.json for 8.19.14 release [ML] Call old inference API (elastic#145690) ESQL: Unmute CsvIT sumWithOverflowRow (elastic#145893) Index a document when testing runtime fields shadowing dimensions & metrics (elastic#145882) [TEST] Fix version check in testSequenceNumbersDisabled (elastic#145879) [ESQL] Per-file filter pushdown awareness (elastic#145755) Unmute testGetReindexFollowsRelocation (elastic#145841) Correctly ignore system indices when validating dot-prefixed indices (elastic#128868) [Transform] Remove tests for deleted code (elastic#145685) ESQL: Add generative tests for LIMIT BY (elastic#144238)

Generative tests generators for LIMIT BY, which was added in elastic#144069 and elastic#144279

ESQL: Add generative tests for LIMIT BY

ee61a68

ivancea requested review from Copilot, luigidellaquila and ncordon March 13, 2026 17:07

ivancea added >test Issues or PRs that are addressing/adding tests Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) :Analytics/ES|QL AKA ESQL v9.4.0 labels Mar 13, 2026

Copilot started reviewing on behalf of ivancea March 13, 2026 17:08 View session

Copilot AI reviewed Mar 13, 2026

View reviewed changes

...ures/src/main/java/org/elasticsearch/xpack/esql/generator/command/pipe/LimitByGenerator.java Show resolved Hide resolved

Potential fix for pull request finding

cbe663a

Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>

luigidellaquila reviewed Mar 16, 2026

View reviewed changes

ivancea added 4 commits March 16, 2026 13:24

Minor improvements

15fc7f1

Merge branch 'main' into esql-limit-by-generative-tests

004861c

Merge branch 'main' into esql-limit-by-generative-tests

d33a9b7

Removed flaky check

746ce4f

ivancea commented Mar 18, 2026

View reviewed changes

ivancea and others added 4 commits April 6, 2026 11:41

Merge branch 'main' into esql-limit-by-generative-tests

55438c6

Fix INLINE_STATS command name in Generative tests

488a421

Remove SORT+LIMIT BY limitation from LimitByGenerator

b6a06b4

Merge branch 'main' into esql-limit-by-generative-tests

4c34a35

ivancea marked this pull request as ready for review April 6, 2026 12:11

ivancea requested review from Copilot and luigidellaquila April 6, 2026 12:11

Copilot started reviewing on behalf of ivancea April 6, 2026 12:12 View session

Copilot AI reviewed Apr 6, 2026

View reviewed changes

...src/main/java/org/elasticsearch/xpack/esql/generator/function/FullTextFunctionGenerator.java Show resolved Hide resolved

...ures/src/main/java/org/elasticsearch/xpack/esql/generator/command/pipe/LimitByGenerator.java Outdated Show resolved Hide resolved

Fix using command gemerator static labels in GenerativeRestTests and …

da51338

…improve LimitBy validation of columns

ncordon approved these changes Apr 6, 2026

View reviewed changes

Make field private

f271e75

ivancea enabled auto-merge (squash) April 7, 2026 11:19

ivancea added 2 commits April 8, 2026 12:42

Merge branch 'main' into esql-limit-by-generative-tests

edeb4fc

Merge branch 'main' into esql-limit-by-generative-tests

79d85d6

ivancea merged commit fe9239c into elastic:main Apr 8, 2026
35 checks passed

ivancea deleted the esql-limit-by-generative-tests branch April 8, 2026 14:36

mromaios pushed a commit to mromaios/elasticsearch that referenced this pull request Apr 9, 2026

ESQL: Add generative tests for LIMIT BY (elastic#144238)

01fa653

Generative tests generators for LIMIT BY, which was added in elastic#144069 and elastic#144279

Conversation

ivancea commented Mar 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

luigidellaquila Mar 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ivancea Mar 16, 2026

Choose a reason for hiding this comment

Uh oh!

luigidellaquila Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ivancea Mar 17, 2026

Choose a reason for hiding this comment

Uh oh!

ivancea Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

ivancea Mar 18, 2026

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Apr 6, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

ncordon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ncordon Apr 6, 2026

Choose a reason for hiding this comment

Uh oh!

ivancea Apr 7, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

ivancea commented Mar 13, 2026 •

edited

Loading

luigidellaquila Mar 16, 2026 •

edited

Loading

luigidellaquila Mar 17, 2026 •

edited

Loading