From a1f6a10ee57ee166d0f474c58073119f73da6442 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Sun, 12 Oct 2025 18:01:10 -0700 Subject: [PATCH] Fix getBytesRef in ConstantBytesRefVector (#136445) 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. --- .../compute/data/ConstantBytesRefVector.java | 7 +++++-- .../elasticsearch/compute/data/X-ConstantVector.java.st | 8 ++++++-- .../org/elasticsearch/compute/data/BasicBlockTests.java | 6 ++++++ 3 files changed, 17 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/ConstantBytesRefVector.java b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/ConstantBytesRefVector.java index 5b4ccfdb12bf4..55a46ddb50c8c 100644 --- a/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/ConstantBytesRefVector.java +++ b/x-pack/plugin/esql/compute/src/main/generated-src/org/elasticsearch/compute/data/ConstantBytesRefVector.java @@ -32,8 +32,11 @@ final class ConstantBytesRefVector extends AbstractVector implements BytesRefVec } @Override - public BytesRef getBytesRef(int position, BytesRef ignore) { - return value; + public BytesRef getBytesRef(int position, BytesRef scratch) { + scratch.bytes = value.bytes; + scratch.offset = value.offset; + scratch.length = value.length; + return scratch; } @Override diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-ConstantVector.java.st b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-ConstantVector.java.st index 37cd9e4a82b14..2b52c7234f04b 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-ConstantVector.java.st +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/X-ConstantVector.java.st @@ -40,11 +40,15 @@ $endif$ @Override $if(BytesRef)$ - public BytesRef getBytesRef(int position, BytesRef ignore) { + public BytesRef getBytesRef(int position, BytesRef scratch) { + scratch.bytes = value.bytes; + scratch.offset = value.offset; + scratch.length = value.length; + return scratch; $else$ public $type$ get$Type$(int position) { -$endif$ return value; +$endif$ } @Override diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BasicBlockTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BasicBlockTests.java index 7192146939ec5..f5586ebb0828f 100644 --- a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BasicBlockTests.java +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BasicBlockTests.java @@ -786,6 +786,12 @@ public void testConstantBytesRefBlock() { assertEmptyLookup(blockFactory, block); assertInsertNulls(block); releaseAndAssertBreaker(block); + // modify the offset + var v0 = block.getBytesRef(randomInt(positionCount), new BytesRef()); + var v1 = block.getBytesRef(randomInt(positionCount), new BytesRef()); + v1.length = 0; + var v2 = block.getBytesRef(randomInt(positionCount), new BytesRef()); + assertThat(v2, equalTo(v0)); } }