Skip to content

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

Merged
luigidellaquila merged 4 commits intoelastic:mainfrom
luigidellaquila:esql/fix_limit_no_blocks
Mar 3, 2026
Merged

ES|QL: Fix LIMIT after all columns are dropped#143463
luigidellaquila merged 4 commits intoelastic:mainfrom
luigidellaquila:esql/fix_limit_no_blocks

Conversation

@luigidellaquila
Copy link
Copy Markdown
Contributor

Fixes: #142473

Fix LIMIT when all the columns are dropped, by explicitly setting the page size.

@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

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

@elasticsearchmachine elasticsearchmachine added Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.4.0 labels Mar 3, 2026
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

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

- match: { values.1.1: null }

---
"Limit truncation with zero columns #142473":
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.

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

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

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

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.

Copy link
Copy Markdown
Contributor

@ivancea ivancea left a comment

Choose a reason for hiding this comment

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

:shipit:

@luigidellaquila luigidellaquila enabled auto-merge (squash) March 3, 2026 14:02
@luigidellaquila luigidellaquila merged commit 880ad61 into elastic:main Mar 3, 2026
35 checks passed
GalLalouche pushed a commit to GalLalouche/elasticsearch that referenced this pull request Mar 3, 2026
szybia added a commit to szybia/elasticsearch that referenced this pull request Mar 3, 2026
…locations

* upstream/main: (51 commits)
  ESQL: Remaining serialization tests (elastic#143470)
  Eagerly release resources in `TransportAwaitClusterStateVersionAppliedAction` (elastic#143477)
  Stop and relocate sliced reindex on shutdown (elastic#143183)
  Documentation for query_vector base64 parameter (elastic#142675)
  ES|QL: Fix LIMIT after all columns are dropped (elastic#143463)
  Update docs-build.yml (elastic#142958)
  Fix KnnIndexTester to work with byte vectors (elastic#143493)
  Fix IndexInputUtils.withSlice to produce native-safe MemorySegments on Java 21 (elastic#143479)
  CPS fix: include only relevant projects in the search response metadata (elastic#143367)
  apm-data: explicit map of timestamp.us to long (elastic#143173)
  [Inference API] Add custom headers for Azure OpenAI Service (elastic#142969)
  ESQL: Add name IDs to golden tests and fix synthetic names (elastic#143450)
  Add getUnavailableShards to BaseBroadcastResponse (elastic#143406)
  Add description to reindex API without sensitive info (elastic#143112)
  SQL: fix CLI tests (elastic#143451)
  ES|QL: Add note of future removal of FORK implicit LIMIT (elastic#143457)
  [Test] Randomly disable doc values skippers in time-series indices (elastic#143389)
  Improve pattern text downgrade license test (elastic#143102)
  [Transform] Stop transforms at the end of tests (elastic#139783)
  Mute org.elasticsearch.compute.lucene.read.ValueSourceReaderTypeConversionTests testLoadAll elastic#143471
  ...
shmuelhanoch pushed a commit to shmuelhanoch/elasticsearch that referenced this pull request Mar 4, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

:Analytics/ES|QL AKA ESQL >bug Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v9.4.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ES|QL: illegal_argument_exception - "blocks is empty"

5 participants