diff --git a/docs/changelog/131817.yaml b/docs/changelog/131817.yaml new file mode 100644 index 0000000000000..84cf6bbef0117 --- /dev/null +++ b/docs/changelog/131817.yaml @@ -0,0 +1,5 @@ +pr: 131817 +summary: Change equals and hashcode for `ConstantNullBlock` +area: ES|QL +type: bug +issues: [] diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/ConstantNullBlock.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/ConstantNullBlock.java index 2ed905f4299ca..967f91a3ee471 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/ConstantNullBlock.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/data/ConstantNullBlock.java @@ -14,7 +14,6 @@ import org.elasticsearch.core.ReleasableIterator; import java.io.IOException; -import java.util.Objects; /** * Block implementation representing a constant null value. @@ -120,15 +119,28 @@ public long ramBytesUsed() { @Override public boolean equals(Object obj) { - if (obj instanceof ConstantNullBlock that) { - return this.getPositionCount() == that.getPositionCount(); + if (obj instanceof Block that) { + return this.getPositionCount() == 0 && that.getPositionCount() == 0 + || this.getPositionCount() == that.getPositionCount() && that.areAllValuesNull(); + } + if (obj instanceof Vector that) { + return this.getPositionCount() == 0 && that.getPositionCount() == 0; } return false; } @Override public int hashCode() { - return Objects.hash(getPositionCount()); + // The hashcode for ConstantNullBlock is calculated in this way so that + // we return the same hashcode for ConstantNullBlock as we would for block + // types that ConstantNullBlock implements that contain only null values. + // Example: a DoubleBlock with 8 positions that are all null will return + // the same hashcode as a ConstantNullBlock with a positionCount of 8. + int result = 1; + for (int pos = 0; pos < positionCount; pos++) { + result = 31 * result - 1; + } + return result; } @Override diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BooleanBlockEqualityTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BooleanBlockEqualityTests.java index 7c0c4c48e97de..6bb31b141597f 100644 --- a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BooleanBlockEqualityTests.java +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BooleanBlockEqualityTests.java @@ -52,7 +52,8 @@ public void testEmptyBlock() { blockFactory.newConstantBooleanBlockWith(randomBoolean(), 0), blockFactory.newBooleanBlockBuilder(0).build(), blockFactory.newBooleanBlockBuilder(0).appendBoolean(randomBoolean()).build().filter(), - blockFactory.newBooleanBlockBuilder(0).appendNull().build().filter() + blockFactory.newBooleanBlockBuilder(0).appendNull().build().filter(), + (ConstantNullBlock) blockFactory.newConstantNullBlock(0) ); assertAllEquals(blocks); } @@ -252,6 +253,28 @@ public void testBlockInequality() { assertAllNotEquals(notEqualBlocks); } + public void testSimpleBlockWithManyNulls() { + int positions = randomIntBetween(1, 256); + boolean grow = randomBoolean(); + BooleanBlock.Builder builder1 = blockFactory.newBooleanBlockBuilder(grow ? 0 : positions); + BooleanBlock.Builder builder2 = blockFactory.newBooleanBlockBuilder(grow ? 0 : positions); + ConstantNullBlock.Builder builder3 = new ConstantNullBlock.Builder(blockFactory); + for (int p = 0; p < positions; p++) { + builder1.appendNull(); + builder2.appendNull(); + builder3.appendNull(); + } + BooleanBlock block1 = builder1.build(); + BooleanBlock block2 = builder2.build(); + Block block3 = builder3.build(); + assertEquals(positions, block1.getPositionCount()); + assertTrue(block1.mayHaveNulls()); + assertTrue(block1.isNull(0)); + + List blocks = List.of(block1, block2, block3); + assertAllEquals(blocks); + } + static void assertAllEquals(List objs) { for (Object obj1 : objs) { for (Object obj2 : objs) { diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BytesRefBlockEqualityTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BytesRefBlockEqualityTests.java index ddf8b1a28bc26..5a81c3b4f0a8e 100644 --- a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BytesRefBlockEqualityTests.java +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/BytesRefBlockEqualityTests.java @@ -63,7 +63,8 @@ public void testEmptyBlock() { blockFactory.newConstantBytesRefBlockWith(new BytesRef(), 0), blockFactory.newBytesRefBlockBuilder(0).build(), blockFactory.newBytesRefBlockBuilder(0).appendBytesRef(new BytesRef()).build().filter(), - blockFactory.newBytesRefBlockBuilder(0).appendNull().build().filter() + blockFactory.newBytesRefBlockBuilder(0).appendNull().build().filter(), + (ConstantNullBlock) blockFactory.newConstantNullBlock(0) ); assertAllEquals(blocks); } @@ -357,17 +358,20 @@ public void testSimpleBlockWithManyNulls() { boolean grow = randomBoolean(); BytesRefBlock.Builder builder1 = blockFactory.newBytesRefBlockBuilder(grow ? 0 : positions); BytesRefBlock.Builder builder2 = blockFactory.newBytesRefBlockBuilder(grow ? 0 : positions); + ConstantNullBlock.Builder builder3 = new ConstantNullBlock.Builder(blockFactory); for (int p = 0; p < positions; p++) { builder1.appendNull(); builder2.appendNull(); + builder3.appendNull(); } BytesRefBlock block1 = builder1.build(); BytesRefBlock block2 = builder2.build(); + Block block3 = builder3.build(); assertEquals(positions, block1.getPositionCount()); assertTrue(block1.mayHaveNulls()); assertTrue(block1.isNull(0)); - List blocks = List.of(block1, block2); + List blocks = List.of(block1, block2, block3); assertAllEquals(blocks); } diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/DoubleBlockEqualityTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/DoubleBlockEqualityTests.java index 0bdcd8a29add9..0882c57e538a6 100644 --- a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/DoubleBlockEqualityTests.java +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/DoubleBlockEqualityTests.java @@ -51,7 +51,8 @@ public void testEmptyBlock() { blockFactory.newConstantDoubleBlockWith(0, 0), blockFactory.newDoubleBlockBuilder(0).build(), blockFactory.newDoubleBlockBuilder(0).appendDouble(1).build().filter(), - blockFactory.newDoubleBlockBuilder(0).appendNull().build().filter() + blockFactory.newDoubleBlockBuilder(0).appendNull().build().filter(), + (ConstantNullBlock) blockFactory.newConstantNullBlock(0) ); assertAllEquals(blocks); Releasables.close(blocks); @@ -234,17 +235,20 @@ public void testSimpleBlockWithManyNulls() { boolean grow = randomBoolean(); DoubleBlock.Builder builder1 = blockFactory.newDoubleBlockBuilder(grow ? 0 : positions); DoubleBlock.Builder builder2 = blockFactory.newDoubleBlockBuilder(grow ? 0 : positions); + ConstantNullBlock.Builder builder3 = new ConstantNullBlock.Builder(blockFactory); for (int p = 0; p < positions; p++) { builder1.appendNull(); builder2.appendNull(); + builder3.appendNull(); } DoubleBlock block1 = builder1.build(); DoubleBlock block2 = builder2.build(); + Block block3 = builder3.build(); assertEquals(positions, block1.getPositionCount()); assertTrue(block1.mayHaveNulls()); assertTrue(block1.isNull(0)); - List blocks = List.of(block1, block2); + List blocks = List.of(block1, block2, block3); assertAllEquals(blocks); } diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/FloatBlockEqualityTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/FloatBlockEqualityTests.java index 046567ff5987d..e63597d7ff4b0 100644 --- a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/FloatBlockEqualityTests.java +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/FloatBlockEqualityTests.java @@ -51,7 +51,8 @@ public void testEmptyBlock() { blockFactory.newConstantFloatBlockWith(0, 0), blockFactory.newFloatBlockBuilder(0).build(), blockFactory.newFloatBlockBuilder(0).appendFloat(1).build().filter(), - blockFactory.newFloatBlockBuilder(0).appendNull().build().filter() + blockFactory.newFloatBlockBuilder(0).appendNull().build().filter(), + (ConstantNullBlock) blockFactory.newConstantNullBlock(0) ); assertAllEquals(blocks); Releasables.close(blocks); @@ -234,17 +235,20 @@ public void testSimpleBlockWithManyNulls() { boolean grow = randomBoolean(); FloatBlock.Builder builder1 = blockFactory.newFloatBlockBuilder(grow ? 0 : positions); FloatBlock.Builder builder2 = blockFactory.newFloatBlockBuilder(grow ? 0 : positions); + ConstantNullBlock.Builder builder3 = new ConstantNullBlock.Builder(blockFactory); for (int p = 0; p < positions; p++) { builder1.appendNull(); builder2.appendNull(); + builder3.appendNull(); } FloatBlock block1 = builder1.build(); FloatBlock block2 = builder2.build(); + Block block3 = builder3.build(); assertEquals(positions, block1.getPositionCount()); assertTrue(block1.mayHaveNulls()); assertTrue(block1.isNull(0)); - List blocks = List.of(block1, block2); + List blocks = List.of(block1, block2, block3); assertAllEquals(blocks); } diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/IntBlockEqualityTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/IntBlockEqualityTests.java index eb68bdf7a59d6..7e6a53054277d 100644 --- a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/IntBlockEqualityTests.java +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/IntBlockEqualityTests.java @@ -50,7 +50,8 @@ public void testEmptyBlock() { blockFactory.newConstantIntBlockWith(0, 0), blockFactory.newIntBlockBuilder(0).build(), blockFactory.newIntBlockBuilder(0).appendInt(1).build().filter(), - blockFactory.newIntBlockBuilder(0).appendNull().build().filter() + blockFactory.newIntBlockBuilder(0).appendNull().build().filter(), + (ConstantNullBlock) blockFactory.newConstantNullBlock(0) ); assertAllEquals(blocks); } @@ -203,17 +204,20 @@ public void testSimpleBlockWithManyNulls() { boolean grow = randomBoolean(); IntBlock.Builder builder1 = blockFactory.newIntBlockBuilder(grow ? 0 : positions); IntBlock.Builder builder2 = blockFactory.newIntBlockBuilder(grow ? 0 : positions); + ConstantNullBlock.Builder builder3 = new ConstantNullBlock.Builder(blockFactory); for (int p = 0; p < positions; p++) { builder1.appendNull(); builder2.appendNull(); + builder3.appendNull(); } IntBlock block1 = builder1.build(); IntBlock block2 = builder2.build(); + Block block3 = builder3.build(); assertEquals(positions, block1.getPositionCount()); assertTrue(block1.mayHaveNulls()); assertTrue(block1.isNull(0)); - List blocks = List.of(block1, block2); + List blocks = List.of(block1, block2, block3); assertAllEquals(blocks); } diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/LongBlockEqualityTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/LongBlockEqualityTests.java index 6d6a832d27e54..6dc21a56ad27f 100644 --- a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/LongBlockEqualityTests.java +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/data/LongBlockEqualityTests.java @@ -50,7 +50,8 @@ public void testEmptyBlock() { blockFactory.newConstantLongBlockWith(0, 0), blockFactory.newLongBlockBuilder(0).build(), blockFactory.newLongBlockBuilder(0).appendLong(1).build().filter(), - blockFactory.newLongBlockBuilder(0).appendNull().build().filter() + blockFactory.newLongBlockBuilder(0).appendNull().build().filter(), + (ConstantNullBlock) blockFactory.newConstantNullBlock(0) ); assertAllEquals(blocks); } @@ -201,17 +202,20 @@ public void testSimpleBlockWithManyNulls() { boolean grow = randomBoolean(); LongBlock.Builder builder1 = blockFactory.newLongBlockBuilder(grow ? 0 : positions); LongBlock.Builder builder2 = blockFactory.newLongBlockBuilder(grow ? 0 : positions); + ConstantNullBlock.Builder builder3 = new ConstantNullBlock.Builder(blockFactory); for (int p = 0; p < positions; p++) { builder1.appendNull(); builder2.appendNull(); + builder3.appendNull(); } LongBlock block1 = builder1.build(); LongBlock block2 = builder2.build(); + Block block3 = builder3.build(); assertEquals(positions, block1.getPositionCount()); assertTrue(block1.mayHaveNulls()); assertTrue(block1.isNull(0)); - List blocks = List.of(block1, block2); + List blocks = List.of(block1, block2, block3); assertAllEquals(blocks); }