Skip to content

Conversation

dnhatn
Copy link
Member

@dnhatn dnhatn commented Oct 11, 2025

We should not return the BytesRef directly from ConstantBytesRefBlock, but instead copy its slice to a scratch buffer. Although current usage does not modify the offset and length, the contract should allow callers to change these fields, as long as the content of the bytes array remains unchanged. The usage pattern below should be fine, but currently it can change ConstantBytesRefBlock.

var scratch = new BytesRef();

BytesRef v = block.getBytesRef(p, scratch);
// Process the returned BytesRef by incrementally increasing the offset and length
// until the remaining length reaches zero.

@dnhatn dnhatn added >non-issue auto-backport Automatically create backport pull requests when merged :Analytics/ES|QL AKA ESQL v8.19.6 v9.1.6 v9.2.1 v9.3.0 labels Oct 11, 2025
@dnhatn dnhatn marked this pull request as ready for review October 12, 2025 17:54
@elasticsearchmachine
Copy link
Collaborator

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

@elasticsearchmachine elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Oct 12, 2025
Copy link
Contributor

@ChrisHegarty ChrisHegarty left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks @dnhatn

@dnhatn
Copy link
Member Author

dnhatn commented Oct 13, 2025

Thanks Chris!

@dnhatn dnhatn merged commit 42b0876 into elastic:main Oct 13, 2025
33 of 34 checks passed
@dnhatn dnhatn deleted the fix-constant-bytes branch October 13, 2025 01:01
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Oct 13, 2025
We should not return the BytesRef directly from ConstantBytesRefBlock, 
but instead copy its slice to a scratch buffer. Although current usage
does not modify the offset and length, the contract should allow callers
to change these fields, as long as the content of the bytes array
remains unchanged. The usage pattern below should be fine, but currently
it can change ConstantBytesRefBlock.
@elasticsearchmachine
Copy link
Collaborator

💚 Backport successful

Status Branch Result
8.19
9.1
9.2

dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Oct 13, 2025
We should not return the BytesRef directly from ConstantBytesRefBlock, 
but instead copy its slice to a scratch buffer. Although current usage
does not modify the offset and length, the contract should allow callers
to change these fields, as long as the content of the bytes array
remains unchanged. The usage pattern below should be fine, but currently
it can change ConstantBytesRefBlock.
dnhatn added a commit to dnhatn/elasticsearch that referenced this pull request Oct 13, 2025
We should not return the BytesRef directly from ConstantBytesRefBlock, 
but instead copy its slice to a scratch buffer. Although current usage
does not modify the offset and length, the contract should allow callers
to change these fields, as long as the content of the bytes array
remains unchanged. The usage pattern below should be fine, but currently
it can change ConstantBytesRefBlock.
elasticsearchmachine pushed a commit that referenced this pull request Oct 13, 2025
We should not return the BytesRef directly from ConstantBytesRefBlock, 
but instead copy its slice to a scratch buffer. Although current usage
does not modify the offset and length, the contract should allow callers
to change these fields, as long as the content of the bytes array
remains unchanged. The usage pattern below should be fine, but currently
it can change ConstantBytesRefBlock.
elasticsearchmachine pushed a commit that referenced this pull request Oct 13, 2025
We should not return the BytesRef directly from ConstantBytesRefBlock, 
but instead copy its slice to a scratch buffer. Although current usage
does not modify the offset and length, the contract should allow callers
to change these fields, as long as the content of the bytes array
remains unchanged. The usage pattern below should be fine, but currently
it can change ConstantBytesRefBlock.
elasticsearchmachine pushed a commit that referenced this pull request Oct 13, 2025
We should not return the BytesRef directly from ConstantBytesRefBlock, 
but instead copy its slice to a scratch buffer. Although current usage
does not modify the offset and length, the contract should allow callers
to change these fields, as long as the content of the bytes array
remains unchanged. The usage pattern below should be fine, but currently
it can change ConstantBytesRefBlock.
jfreden pushed a commit to jfreden/elasticsearch that referenced this pull request Oct 13, 2025
We should not return the BytesRef directly from ConstantBytesRefBlock, 
but instead copy its slice to a scratch buffer. Although current usage
does not modify the offset and length, the contract should allow callers
to change these fields, as long as the content of the bytes array
remains unchanged. The usage pattern below should be fine, but currently
it can change ConstantBytesRefBlock.
georgewallace pushed a commit to georgewallace/elasticsearch that referenced this pull request Oct 13, 2025
We should not return the BytesRef directly from ConstantBytesRefBlock, 
but instead copy its slice to a scratch buffer. Although current usage
does not modify the offset and length, the contract should allow callers
to change these fields, as long as the content of the bytes array
remains unchanged. The usage pattern below should be fine, but currently
it can change ConstantBytesRefBlock.
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 >non-issue Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) v8.19.6 v9.1.6 v9.2.1 v9.3.0

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants