Skip to content

Check block isNull array on copy #26404

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Aug 15, 2025
Merged

Check block isNull array on copy #26404

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
f284b60
Check isNull array during range compaction
pettyjamesm Aug 13, 2025
ec806f1
Check isNull array during positions copy
pettyjamesm Aug 13, 2025
46c9992
Improve isNull check in MapBlock positions copy
pettyjamesm Aug 13, 2025
fd3ba4e
Improve isNull check in ArrayBlock positions copy
pettyjamesm Aug 13, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/trino-main/src/test/java/io/trino/execution/buffer/TestPagesSerde.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,7 @@ public void testBigintSerializedSize()
// empty page
Page page = new Page(builder.build());
int pageSize = serializedSize(ImmutableList.of(BIGINT), page);
assertThat(pageSize).isEqualTo(40);
assertThat(pageSize).isEqualTo(36);

// page with one value
BIGINT.writeLong(builder, 123);
Expand Down
13 changes: 7 additions & 6 deletions core/trino-spi/src/main/java/io/trino/spi/block/ArrayBlock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
import static io.trino.spi.block.BlockUtil.checkArrayRange;
import static io.trino.spi.block.BlockUtil.checkReadablePosition;
import static io.trino.spi.block.BlockUtil.checkValidRegion;
import static io.trino.spi.block.BlockUtil.compactArray;
import static io.trino.spi.block.BlockUtil.compactIsNull;
import static io.trino.spi.block.BlockUtil.compactOffsets;
import static io.trino.spi.block.BlockUtil.copyIsNullAndAppendNull;
import static io.trino.spi.block.BlockUtil.copyOffsetsAndAppendNull;
Expand Down Expand Up @@ -232,12 +232,15 @@ public ArrayBlock copyPositions(int[] positions, int offset, int length)

int[] newOffsets = new int[length + 1];
newOffsets[0] = 0;
boolean hasNull = false;
boolean[] newValueIsNull = valueIsNull == null ? null : new boolean[length];

IntArrayList valuesPositions = new IntArrayList();
for (int i = 0; i < length; i++) {
int position = positions[offset + i];
if (newValueIsNull != null && isNull(position)) {
checkReadablePosition(this, position);
if (valueIsNull != null && valueIsNull[position + arrayOffset]) {
hasNull = true;
newValueIsNull[i] = true;
newOffsets[i + 1] = newOffsets[i];
}
Expand All @@ -254,7 +257,7 @@ public ArrayBlock copyPositions(int[] positions, int offset, int length)
}
}
Block newValues = values.copyPositions(valuesPositions.elements(), 0, valuesPositions.size());
return createArrayBlockInternal(0, length, newValueIsNull, newOffsets, newValues);
return createArrayBlockInternal(0, length, hasNull ? newValueIsNull : null, newOffsets, newValues);
}

@Override
Expand Down Expand Up @@ -294,9 +297,7 @@ public ArrayBlock copyRegion(int position, int length)
Block newValues = values.copyRegion(startValueOffset, endValueOffset - startValueOffset);

int[] newOffsets = compactOffsets(offsets, position + arrayOffset, length);
boolean[] valueIsNull = this.valueIsNull;
boolean[] newValueIsNull;
newValueIsNull = valueIsNull == null ? null : compactArray(valueIsNull, position + arrayOffset, length);
boolean[] newValueIsNull = compactIsNull(valueIsNull, position + arrayOffset, length);

if (newValues == values && newOffsets == offsets && newValueIsNull == valueIsNull) {
return this;
Expand Down
25 changes: 25 additions & 0 deletions core/trino-spi/src/main/java/io/trino/spi/block/BlockUtil.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,31 @@ static int calculateBlockResetBytes(int currentBytes)
return (int) newBytes;
}

/**
* Returns an array containing elements in the specified range of the specified <code>isNull</code> array.
* If the input array is null, null is returned. If the range matches the entire array, the input array
* will be returned. Otherwise, a copy will be returned.
*/
static boolean[] compactIsNull(@Nullable boolean[] isNull, int index, int length)
{
if (isNull == null) {
return null;
}
checkArrayRange(isNull, index, length);
if (index == 0 && length == isNull.length) {
return isNull;
}
for (int i = 0; i < length; i++) {
if (isNull[i + index]) {
boolean[] result = new boolean[length];
System.arraycopy(isNull, i + index, result, i, length - i);
return result;
}
}
// No nulls encountered, return null as the result
return null;
}

/**
* Recalculate the <code>offsets</code> array for the specified range.
* The returned <code>offsets</code> array contains <code>length + 1</code> integers
Expand Down
10 changes: 7 additions & 3 deletions core/trino-spi/src/main/java/io/trino/spi/block/ByteArrayBlock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
import static io.trino.spi.block.BlockUtil.checkReadablePosition;
import static io.trino.spi.block.BlockUtil.checkValidRegion;
import static io.trino.spi.block.BlockUtil.compactArray;
import static io.trino.spi.block.BlockUtil.compactIsNull;
import static io.trino.spi.block.BlockUtil.copyIsNullAndAppendNull;
import static io.trino.spi.block.BlockUtil.ensureCapacity;

Expand Down Expand Up @@ -177,6 +178,7 @@ public ByteArrayBlock copyPositions(int[] positions, int offset, int length)
{
checkArrayRange(positions, offset, length);

boolean hasNull = false;
boolean[] newValueIsNull = null;
if (valueIsNull != null) {
newValueIsNull = new boolean[length];
Expand All @@ -186,11 +188,13 @@ public ByteArrayBlock copyPositions(int[] positions, int offset, int length)
int position = positions[offset + i];
checkReadablePosition(this, position);
if (valueIsNull != null) {
newValueIsNull[i] = valueIsNull[position + arrayOffset];
boolean isNull = valueIsNull[position + arrayOffset];
newValueIsNull[i] = isNull;
hasNull |= isNull;
}
newValues[i] = values[position + arrayOffset];
}
return new ByteArrayBlock(0, length, newValueIsNull, newValues);
return new ByteArrayBlock(0, length, hasNull ? newValueIsNull : null, newValues);
}

@Override
Expand All @@ -207,7 +211,7 @@ public ByteArrayBlock copyRegion(int positionOffset, int length)
checkValidRegion(getPositionCount(), positionOffset, length);

positionOffset += arrayOffset;
boolean[] newValueIsNull = valueIsNull == null ? null : compactArray(valueIsNull, positionOffset, length);
boolean[] newValueIsNull = compactIsNull(valueIsNull, positionOffset, length);
byte[] newValues = compactArray(values, positionOffset, length);

if (newValueIsNull == valueIsNull && newValues == values) {
Expand Down
10 changes: 7 additions & 3 deletions core/trino-spi/src/main/java/io/trino/spi/block/Fixed12Block.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import static io.trino.spi.block.BlockUtil.checkReadablePosition;
import static io.trino.spi.block.BlockUtil.checkValidRegion;
import static io.trino.spi.block.BlockUtil.compactArray;
import static io.trino.spi.block.BlockUtil.compactIsNull;
import static io.trino.spi.block.BlockUtil.copyIsNullAndAppendNull;
import static io.trino.spi.block.BlockUtil.ensureCapacity;

Expand Down Expand Up @@ -182,6 +183,7 @@ public Fixed12Block copyPositions(int[] positions, int offset, int length)
checkArrayRange(positions, offset, length);

boolean[] newValueIsNull = null;
boolean hasNull = false;
if (valueIsNull != null) {
newValueIsNull = new boolean[length];
}
Expand All @@ -190,15 +192,17 @@ public Fixed12Block copyPositions(int[] positions, int offset, int length)
int position = positions[offset + i];
checkReadablePosition(this, position);
if (valueIsNull != null) {
newValueIsNull[i] = valueIsNull[position + positionOffset];
boolean isNull = valueIsNull[position + positionOffset];
newValueIsNull[i] = isNull;
hasNull |= isNull;
}
int valuesIndex = (position + positionOffset) * 3;
int newValuesIndex = i * 3;
newValues[newValuesIndex] = values[valuesIndex];
newValues[newValuesIndex + 1] = values[valuesIndex + 1];
newValues[newValuesIndex + 2] = values[valuesIndex + 2];
}
return new Fixed12Block(0, length, newValueIsNull, newValues);
return new Fixed12Block(0, length, hasNull ? newValueIsNull : null, newValues);
}

@Override
Expand All @@ -215,7 +219,7 @@ public Fixed12Block copyRegion(int positionOffset, int length)
checkValidRegion(getPositionCount(), positionOffset, length);

positionOffset += this.positionOffset;
boolean[] newValueIsNull = valueIsNull == null ? null : compactArray(valueIsNull, positionOffset, length);
boolean[] newValueIsNull = compactIsNull(valueIsNull, positionOffset, length);
int[] newValues = compactArray(values, positionOffset * 3, length * 3);

if (newValueIsNull == valueIsNull && newValues == values) {
Expand Down
10 changes: 7 additions & 3 deletions core/trino-spi/src/main/java/io/trino/spi/block/Int128ArrayBlock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
import static io.trino.spi.block.BlockUtil.checkReadablePosition;
import static io.trino.spi.block.BlockUtil.checkValidRegion;
import static io.trino.spi.block.BlockUtil.compactArray;
import static io.trino.spi.block.BlockUtil.compactIsNull;
import static io.trino.spi.block.BlockUtil.copyIsNullAndAppendNull;
import static io.trino.spi.block.BlockUtil.ensureCapacity;

Expand Down Expand Up @@ -176,6 +177,7 @@ public Int128ArrayBlock copyPositions(int[] positions, int offset, int length)
{
checkArrayRange(positions, offset, length);

boolean hasNull = false;
boolean[] newValueIsNull = null;
if (valueIsNull != null) {
newValueIsNull = new boolean[length];
Expand All @@ -185,12 +187,14 @@ public Int128ArrayBlock copyPositions(int[] positions, int offset, int length)
int position = positions[offset + i];
checkReadablePosition(this, position);
if (valueIsNull != null) {
newValueIsNull[i] = valueIsNull[position + positionOffset];
boolean isNull = valueIsNull[position + positionOffset];
newValueIsNull[i] = isNull;
hasNull |= isNull;
}
newValues[i * 2] = values[(position + positionOffset) * 2];
newValues[(i * 2) + 1] = values[((position + positionOffset) * 2) + 1];
}
return new Int128ArrayBlock(0, length, newValueIsNull, newValues);
return new Int128ArrayBlock(0, length, hasNull ? newValueIsNull : null, newValues);
}

@Override
Expand All @@ -207,7 +211,7 @@ public Int128ArrayBlock copyRegion(int positionOffset, int length)
checkValidRegion(getPositionCount(), positionOffset, length);

positionOffset += this.positionOffset;
boolean[] newValueIsNull = valueIsNull == null ? null : compactArray(valueIsNull, positionOffset, length);
boolean[] newValueIsNull = compactIsNull(valueIsNull, positionOffset, length);
long[] newValues = compactArray(values, positionOffset * 2, length * 2);

if (newValueIsNull == valueIsNull && newValues == values) {
Expand Down
10 changes: 7 additions & 3 deletions core/trino-spi/src/main/java/io/trino/spi/block/IntArrayBlock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import static io.trino.spi.block.BlockUtil.checkReadablePosition;
import static io.trino.spi.block.BlockUtil.checkValidRegion;
import static io.trino.spi.block.BlockUtil.compactArray;
import static io.trino.spi.block.BlockUtil.compactIsNull;
import static io.trino.spi.block.BlockUtil.copyIsNullAndAppendNull;
import static io.trino.spi.block.BlockUtil.ensureCapacity;

Expand Down Expand Up @@ -159,6 +160,7 @@ public IntArrayBlock copyPositions(int[] positions, int offset, int length)
{
checkArrayRange(positions, offset, length);

boolean hasNull = false;
boolean[] newValueIsNull = null;
if (valueIsNull != null) {
newValueIsNull = new boolean[length];
Expand All @@ -168,11 +170,13 @@ public IntArrayBlock copyPositions(int[] positions, int offset, int length)
int position = positions[offset + i];
checkReadablePosition(this, position);
if (valueIsNull != null) {
newValueIsNull[i] = valueIsNull[position + arrayOffset];
boolean isNull = valueIsNull[position + arrayOffset];
newValueIsNull[i] = isNull;
hasNull |= isNull;
}
newValues[i] = values[position + arrayOffset];
}
return new IntArrayBlock(0, length, newValueIsNull, newValues);
return new IntArrayBlock(0, length, hasNull ? newValueIsNull : null, newValues);
}

@Override
Expand All @@ -189,7 +193,7 @@ public IntArrayBlock copyRegion(int positionOffset, int length)
checkValidRegion(getPositionCount(), positionOffset, length);

positionOffset += arrayOffset;
boolean[] newValueIsNull = valueIsNull == null ? null : compactArray(valueIsNull, positionOffset, length);
boolean[] newValueIsNull = compactIsNull(valueIsNull, positionOffset, length);
int[] newValues = compactArray(values, positionOffset, length);

if (newValueIsNull == valueIsNull && newValues == values) {
Expand Down
10 changes: 7 additions & 3 deletions core/trino-spi/src/main/java/io/trino/spi/block/LongArrayBlock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
import static io.trino.spi.block.BlockUtil.checkReadablePosition;
import static io.trino.spi.block.BlockUtil.checkValidRegion;
import static io.trino.spi.block.BlockUtil.compactArray;
import static io.trino.spi.block.BlockUtil.compactIsNull;
import static io.trino.spi.block.BlockUtil.copyIsNullAndAppendNull;
import static io.trino.spi.block.BlockUtil.ensureCapacity;

Expand Down Expand Up @@ -159,6 +160,7 @@ public LongArrayBlock copyPositions(int[] positions, int offset, int length)
{
checkArrayRange(positions, offset, length);

boolean hasNull = false;
boolean[] newValueIsNull = null;
if (valueIsNull != null) {
newValueIsNull = new boolean[length];
Expand All @@ -168,11 +170,13 @@ public LongArrayBlock copyPositions(int[] positions, int offset, int length)
int position = positions[offset + i];
checkReadablePosition(this, position);
if (valueIsNull != null) {
newValueIsNull[i] = valueIsNull[position + arrayOffset];
boolean isNull = valueIsNull[position + arrayOffset];
newValueIsNull[i] = isNull;
hasNull |= isNull;
}
newValues[i] = values[position + arrayOffset];
}
return new LongArrayBlock(0, length, newValueIsNull, newValues);
return new LongArrayBlock(0, length, hasNull ? newValueIsNull : null, newValues);
}

@Override
Expand All @@ -189,7 +193,7 @@ public LongArrayBlock copyRegion(int positionOffset, int length)
checkValidRegion(getPositionCount(), positionOffset, length);

positionOffset += arrayOffset;
boolean[] newValueIsNull = valueIsNull == null ? null : compactArray(valueIsNull, positionOffset, length);
boolean[] newValueIsNull = compactIsNull(valueIsNull, positionOffset, length);
long[] newValues = compactArray(values, positionOffset, length);

if (newValueIsNull == valueIsNull && newValues == values) {
Expand Down
19 changes: 13 additions & 6 deletions core/trino-spi/src/main/java/io/trino/spi/block/MapBlock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import static io.trino.spi.block.BlockUtil.checkReadablePosition;
import static io.trino.spi.block.BlockUtil.checkValidRegion;
import static io.trino.spi.block.BlockUtil.compactArray;
import static io.trino.spi.block.BlockUtil.compactIsNull;
import static io.trino.spi.block.BlockUtil.compactOffsets;
import static io.trino.spi.block.BlockUtil.copyIsNullAndAppendNull;
import static io.trino.spi.block.BlockUtil.copyOffsetsAndAppendNull;
Expand Down Expand Up @@ -328,13 +329,18 @@ public MapBlock copyPositions(int[] positions, int offset, int length)
checkArrayRange(positions, offset, length);

int[] newOffsets = new int[length + 1];
boolean[] newMapIsNull = new boolean[length];
boolean hasNull = false;
boolean[] newMapIsNull = null;
if (mapIsNull != null) {
newMapIsNull = new boolean[length];
}

IntArrayList entriesPositions = new IntArrayList();
int newPosition = 0;
for (int i = offset; i < offset + length; ++i) {
int position = positions[i];
if (isNull(position)) {
if (mapIsNull != null && mapIsNull[position + startOffset]) {
hasNull = true;
newMapIsNull[newPosition] = true;
newOffsets[newPosition + 1] = newOffsets[newPosition];
}
Expand All @@ -351,6 +357,9 @@ public MapBlock copyPositions(int[] positions, int offset, int length)
}
newPosition++;
}
if (!hasNull) {
newMapIsNull = null;
}

int[] rawHashTables = hashTables.tryGet().orElse(null);
int[] newRawHashTables = null;
Expand All @@ -375,7 +384,7 @@ public MapBlock copyPositions(int[] positions, int offset, int length)
mapType,
0,
length,
Optional.of(newMapIsNull),
Optional.ofNullable(newMapIsNull),
newOffsets,
newKeys,
newValues,
Expand Down Expand Up @@ -427,9 +436,7 @@ public MapBlock copyRegion(int position, int length)
Block newValues = valueBlock.copyRegion(startValueOffset, endValueOffset - startValueOffset);

int[] newOffsets = compactOffsets(offsets, position + startOffset, length);
boolean[] mapIsNull = this.mapIsNull;
boolean[] newMapIsNull;
newMapIsNull = mapIsNull == null ? null : compactArray(mapIsNull, position + startOffset, length);
boolean[] newMapIsNull = compactIsNull(mapIsNull, position + startOffset, length);
int[] rawHashTables = hashTables.tryGet().orElse(null);
int[] newRawHashTables = null;
int expectedNewHashTableEntries = (endValueOffset - startValueOffset) * HASH_MULTIPLIER;
Expand Down
14 changes: 10 additions & 4 deletions core/trino-spi/src/main/java/io/trino/spi/block/RowBlock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
import static io.trino.spi.block.BlockUtil.checkArrayRange;
import static io.trino.spi.block.BlockUtil.checkReadablePosition;
import static io.trino.spi.block.BlockUtil.checkValidRegion;
import static io.trino.spi.block.BlockUtil.compactArray;
import static io.trino.spi.block.BlockUtil.compactIsNull;
import static java.lang.String.format;
import static java.util.Objects.requireNonNull;

Expand Down Expand Up @@ -238,9 +238,15 @@ public RowBlock copyPositions(int[] positions, int offset, int length)

boolean[] newRowIsNull = null;
if (rowIsNull != null) {
boolean hasNull = false;
newRowIsNull = new boolean[length];
for (int i = 0; i < length; i++) {
newRowIsNull[i] = rowIsNull[positions[offset + i]];
boolean isNull = rowIsNull[positions[offset + i]];
newRowIsNull[i] = isNull;
hasNull |= isNull;
}
if (!hasNull) {
newRowIsNull = null;
}
}

Expand All @@ -255,7 +261,7 @@ public RowBlock getRegion(int positionOffset, int length)
// This copies the null array, but this dramatically simplifies this class.
// Without a copy here, we would need a null array offset, and that would mean that the
// null array would be offset while the field blocks are not offset, which is confusing.
boolean[] newRowIsNull = rowIsNull == null ? null : compactArray(rowIsNull, positionOffset, length);
boolean[] newRowIsNull = compactIsNull(rowIsNull, positionOffset, length);
Block[] newBlocks = new Block[fieldBlocks.length];
for (int i = 0; i < newBlocks.length; i++) {
newBlocks[i] = fieldBlocks[i].getRegion(positionOffset, length);
Expand Down Expand Up @@ -285,7 +291,7 @@ public RowBlock copyRegion(int positionOffset, int length)
newBlocks[i] = fieldBlocks[i].copyRegion(positionOffset, length);
}

boolean[] newRowIsNull = rowIsNull == null ? null : compactArray(rowIsNull, positionOffset, length);
boolean[] newRowIsNull = compactIsNull(rowIsNull, positionOffset, length);
if (newRowIsNull == rowIsNull && arraySame(newBlocks, fieldBlocks)) {
return this;
}
Expand Down
Loading