Skip to content

ESQL: Fix bug in pushed LENGTH(kwd)#143288

Merged
nik9000 merged 7 commits intoelastic:mainfrom
nik9000:esql_fix_offset_bug_in_utf8_read
Feb 28, 2026
Merged

ESQL: Fix bug in pushed LENGTH(kwd)#143288
nik9000 merged 7 commits intoelastic:mainfrom
nik9000:esql_fix_offset_bug_in_utf8_read

Conversation

@nik9000
Copy link
Copy Markdown
Member

@nik9000 nik9000 commented Feb 27, 2026

Fixes a bug when ESQL pushes LENGTH to block loadering of ordinals.

Closes #143222
Closes #143223
Closes #143224

This was caused by #143224

Fixes a bug when ESQL pushes `LENGTH` to block loadering of ordinals.

Closes elastic#143222
Closes elastic#143223
Closes elastic#143224

This was caused by elastic#143224
@nik9000 nik9000 requested a review from ivancea February 27, 2026 18:10
@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Feb 27, 2026
@elasticsearchmachine
Copy link
Copy Markdown
Collaborator

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

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.

🚀

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes an ArrayIndexOutOfBoundsException that occurred when ESQL's LENGTH, BYTE_LENGTH, and BIT_LENGTH functions were pushed down to block loading of keyword field ordinals. The bug was introduced when support for reading blocks with non-zero offsets was added but the array indexing wasn't properly adjusted.

Changes:

  • Fixed array indexing in Utf8CodePointsFromOrdsBlockLoader to use i - offset instead of i when offset > 0
  • Refactored test infrastructure to consolidate duplicate read() helper methods into the base test class
  • Unmuted three previously failing CSV specification tests

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated no comments.

Show a summary per file
File Description
server/src/main/java/org/elasticsearch/index/mapper/blockloader/docvalues/fn/Utf8CodePointsFromOrdsBlockLoader.java Fixed array indexing bug by using i - offset for both SortedDocValues and SortedSetDocValues cases
test/framework/src/main/java/org/elasticsearch/index/mapper/TestBlock.java Made constructor public to allow test infrastructure to create TestBlock instances
test/framework/src/main/java/org/elasticsearch/index/mapper/AbstractBlockLoaderTestCase.java Added centralized read() helper method that tests reading with various offset scenarios
server/src/test/java/org/elasticsearch/index/mapper/blockloader/docvalues/fn/*.java Removed duplicate read() methods and unused imports from 7 test classes
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/ValuesFromManyReader.java Expanded TODO comment with additional context
muted-tests.yml Unmuted 3 CSV tests that were failing due to the fixed bug
Comments suppressed due to low confidence (1)

server/src/test/java/org/elasticsearch/index/mapper/blockloader/docvalues/fn/Utf8CodePointsFromOrdsBlockLoaderTests.java:52

  • Inconsistent try-with-resources statement formatting. Line 52 was updated to remove the trailing semicolon, but lines 59 and 66 in the same file still have trailing semicolons. For consistency, either all should have the trailing semicolon or none should.
            try (TestBlock strings = read(stringsReader, docs); TestBlock codePoints = read(codePointsReader, docs)) {

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

@nik9000
Copy link
Copy Markdown
Member Author

nik9000 commented Feb 27, 2026

  • Inconsistent try-with-resources statement formatting. Line 52 was updated to remove the trailing semicolon, but lines 59 and 66 in the same file still have trailing semicolons. For consistency, either all should have the trailing semicolon or none should.

Copilot, that's pretty good feedback. It's easy to fix and will save someone some confusion later.

@nik9000 nik9000 merged commit f7daa1c into elastic:main Feb 28, 2026
35 checks passed
tballison pushed a commit to tballison/elasticsearch that referenced this pull request Mar 3, 2026
Fixes a bug when ESQL pushes `LENGTH` to block loadering of ordinals.

Closes elastic#143222
Closes elastic#143223
Closes elastic#143224

This was caused by elastic#143224
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

4 participants