Skip to content

ES|QL: Fix LIMIT after all columns are dropped #143463

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
luigidellaquila merged 4 commits into from
Mar 3, 2026
Merged

ES|QL: Fix LIMIT after all columns are dropped #143463

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
2b42b1a
ES|QL: Fix LIMIT after all columns are dropped
luigidellaquila Mar 3, 2026
fd46db2
Update docs/changelog/143463.yaml
luigidellaquila Mar 3, 2026
96f7ab8
Merge branch 'main' into esql/fix_limit_no_blocks
luigidellaquila Mar 3, 2026
e558b53
Merge branch 'main' into esql/fix_limit_no_blocks
luigidellaquila Mar 3, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/changelog/143463.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
area: ES|QL
issues:
- 142473
pr: 143463
summary: Fix LIMIT after all columns are dropped
type: bug
2 changes: 1 addition & 1 deletion ...k/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/LimitOperator.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,7 @@ private static Page truncatePage(Page page, int upTo) {
for (int b = 0; b < blocks.length; b++) {
blocks[b] = page.getBlock(b).filter(false, filter);
}
result = new Page(blocks);
result = new Page(upTo, blocks);
} finally {
if (result == null) {
Releasables.closeExpectNoException(page::releaseBlocks, Releasables.wrap(blocks));
Expand Down
18 changes: 18 additions & 0 deletions ...gin/esql/compute/src/test/java/org/elasticsearch/compute/operator/LimitOperatorTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,24 @@ public void testBlockPreciselyRemaining() {
}
}

public void testTruncatePageWithZeroBlocks() {
try (var op = simple().get(driverContext())) {
assertTrue(op.needsInput());
Page p = new Page(200);
op.addInput(p);
assertFalse(op.needsInput());
Page result = op.getOutput();
try {
assertThat(result.getPositionCount(), equalTo(100));
assertThat(result.getBlockCount(), equalTo(0));
} finally {
result.releaseBlocks();
}
assertFalse(op.needsInput());
assertTrue(op.isFinished());
}
}

public void testEarlyTermination() {
int numDrivers = between(1, 4);
final List<Driver> drivers = new ArrayList<>();
Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1699,6 +1699,12 @@ public enum Cap {
*/
FIX_NO_COLUMNS,

/**
* Fix LimitOperator truncation with zero columns
* https://github.com/elastic/elasticsearch/issues/142473
*/
FIX_LIMIT_TRUNCATION_WITH_ZERO_COLUMNS,

/**
* Support for dots in FUSE attributes
*/
Expand Down
49 changes: 49 additions & 0 deletions x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/100_bug_fix.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -390,3 +390,52 @@
- match: { values.0.1: null }
- match: { values.1.0: null }
- match: { values.1.1: null }

---
"Limit truncation with zero columns #142473":
Copy link
Copy Markdown
Member

@ncordon ncordon Mar 3, 2026

Choose a reason for hiding this comment

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

This bug was caused by a csv test right? Do we want a csv test apart from the yaml one? Or when do we use one vs the other?

Copy link
Copy Markdown
Contributor

@ivancea ivancea Mar 3, 2026

Choose a reason for hiding this comment

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

About YAML, we (or I) usually use them when we want to test not just query and output, but also the request/response. And I think sometimes we add extra special checks for YAML, and they're also validated against the API spec.

And my guess is that, because the output has no columns, CSV tests wouldn't support it. But Luigi will confirm

Copy link
Copy Markdown
Contributor Author

@luigidellaquila luigidellaquila Mar 3, 2026

Choose a reason for hiding this comment

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

In this case, I used yaml because I wanted a query that returns no columns (and I can't validate the output with CSV).
The failure happened on the CSV dataset, but it was on Generative tests

Copy link
Copy Markdown
Contributor Author

@luigidellaquila luigidellaquila Mar 3, 2026

Choose a reason for hiding this comment

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

Or when do we use one vs the other?

I use yaml tests when:

  • I need a very specific dataset or a data combination I don't have in CSV, and that is not general enough to deserve an addition to that.
  • I need more fine-grained validation (as Ivan said above)
  • The test just can't be written in csv-spec (eg. a query that returns no columns)

In all the other cases, I try to use csv-spec just because it's easier to write, and because we run it in different configurations.

- requires:
test_runner_features: [capabilities]
capabilities:
- method: POST
path: /_query
parameters: []
capabilities: [fix_limit_truncation_with_zero_columns]
reason: "fixed LimitOperator truncation with zero columns"
- do:
indices.create:
index: test_limit_zero_cols
body:
mappings:
properties:
value:
type: integer
- do:
bulk:
refresh: true
body:
- { "index": { "_index": "test_limit_zero_cols" } }
- { "value": 1 }
- { "index": { "_index": "test_limit_zero_cols" } }
- { "value": 2 }
- { "index": { "_index": "test_limit_zero_cols" } }
- { "value": 3 }
- { "index": { "_index": "test_limit_zero_cols" } }
- { "value": 4 }
- { "index": { "_index": "test_limit_zero_cols" } }
- { "value": 5 }
- { "index": { "_index": "test_limit_zero_cols" } }
- { "value": 6 }
- { "index": { "_index": "test_limit_zero_cols" } }
- { "value": 7 }
- { "index": { "_index": "test_limit_zero_cols" } }
- { "value": 8 }
- { "index": { "_index": "test_limit_zero_cols" } }
- { "value": 9 }
- { "index": { "_index": "test_limit_zero_cols" } }
- { "value": 10 }
- do:
esql.query:
body:
query: 'FROM test_limit_zero_cols | EVAL x = "test" | KEEP x | DROP x | LIMIT 5'
- length: { columns: 0 }
- length: { values: 5 }