diff --git a/core/trino-main/src/main/java/io/trino/operator/output/BytePositionsAppender.java b/core/trino-main/src/main/java/io/trino/operator/output/BytePositionsAppender.java index fb065efa2794..e099fd367616 100644 --- a/core/trino-main/src/main/java/io/trino/operator/output/BytePositionsAppender.java +++ b/core/trino-main/src/main/java/io/trino/operator/output/BytePositionsAppender.java @@ -121,10 +121,13 @@ public void appendRle(RunLengthEncodedBlock block) @Override public Block build() { - if (!hasNonNullValue) { - return new RunLengthEncodedBlock(NULL_VALUE_BLOCK, positionCount); + Block result; + if (hasNonNullValue) { + result = new ByteArrayBlock(positionCount, hasNullValue ? Optional.of(valueIsNull) : Optional.empty(), values); + } + else { + result = new RunLengthEncodedBlock(NULL_VALUE_BLOCK, positionCount); } - ByteArrayBlock result = new ByteArrayBlock(positionCount, hasNullValue ? Optional.of(valueIsNull) : Optional.empty(), values); reset(); return result; } diff --git a/core/trino-main/src/main/java/io/trino/operator/output/Int128PositionsAppender.java b/core/trino-main/src/main/java/io/trino/operator/output/Int128PositionsAppender.java index 4046beb8d97f..cfa6daf264ef 100644 --- a/core/trino-main/src/main/java/io/trino/operator/output/Int128PositionsAppender.java +++ b/core/trino-main/src/main/java/io/trino/operator/output/Int128PositionsAppender.java @@ -134,10 +134,13 @@ public void appendRle(RunLengthEncodedBlock block) @Override public Block build() { - if (!hasNonNullValue) { - return new RunLengthEncodedBlock(NULL_VALUE_BLOCK, positionCount); + Block result; + if (hasNonNullValue) { + result = new Int128ArrayBlock(positionCount, hasNullValue ? Optional.of(valueIsNull) : Optional.empty(), values); + } + else { + result = new RunLengthEncodedBlock(NULL_VALUE_BLOCK, positionCount); } - Int128ArrayBlock result = new Int128ArrayBlock(positionCount, hasNullValue ? Optional.of(valueIsNull) : Optional.empty(), values); reset(); return result; } diff --git a/core/trino-main/src/main/java/io/trino/operator/output/Int96PositionsAppender.java b/core/trino-main/src/main/java/io/trino/operator/output/Int96PositionsAppender.java index 721006f95b0e..9f8549dfd583 100644 --- a/core/trino-main/src/main/java/io/trino/operator/output/Int96PositionsAppender.java +++ b/core/trino-main/src/main/java/io/trino/operator/output/Int96PositionsAppender.java @@ -129,10 +129,13 @@ public void appendRle(RunLengthEncodedBlock block) @Override public Block build() { - if (!hasNonNullValue) { - return new RunLengthEncodedBlock(NULL_VALUE_BLOCK, positionCount); + Block result; + if (hasNonNullValue) { + result = new Int96ArrayBlock(positionCount, hasNullValue ? Optional.of(valueIsNull) : Optional.empty(), high, low); + } + else { + result = new RunLengthEncodedBlock(NULL_VALUE_BLOCK, positionCount); } - Int96ArrayBlock result = new Int96ArrayBlock(positionCount, hasNullValue ? Optional.of(valueIsNull) : Optional.empty(), high, low); reset(); return result; } diff --git a/core/trino-main/src/main/java/io/trino/operator/output/IntPositionsAppender.java b/core/trino-main/src/main/java/io/trino/operator/output/IntPositionsAppender.java index 76f0c45840ac..f49186466dc5 100644 --- a/core/trino-main/src/main/java/io/trino/operator/output/IntPositionsAppender.java +++ b/core/trino-main/src/main/java/io/trino/operator/output/IntPositionsAppender.java @@ -121,10 +121,13 @@ public void appendRle(RunLengthEncodedBlock block) @Override public Block build() { - if (!hasNonNullValue) { - return new RunLengthEncodedBlock(NULL_VALUE_BLOCK, positionCount); + Block result; + if (hasNonNullValue) { + result = new IntArrayBlock(positionCount, hasNullValue ? Optional.of(valueIsNull) : Optional.empty(), values); + } + else { + result = new RunLengthEncodedBlock(NULL_VALUE_BLOCK, positionCount); } - IntArrayBlock result = new IntArrayBlock(positionCount, hasNullValue ? Optional.of(valueIsNull) : Optional.empty(), values); reset(); return result; } diff --git a/core/trino-main/src/main/java/io/trino/operator/output/LongPositionsAppender.java b/core/trino-main/src/main/java/io/trino/operator/output/LongPositionsAppender.java index 2ae9ff6237f2..5ec18a1fef63 100644 --- a/core/trino-main/src/main/java/io/trino/operator/output/LongPositionsAppender.java +++ b/core/trino-main/src/main/java/io/trino/operator/output/LongPositionsAppender.java @@ -121,10 +121,13 @@ public void appendRle(RunLengthEncodedBlock block) @Override public Block build() { - if (!hasNonNullValue) { - return new RunLengthEncodedBlock(NULL_VALUE_BLOCK, positionCount); + Block result; + if (hasNonNullValue) { + result = new LongArrayBlock(positionCount, hasNullValue ? Optional.of(valueIsNull) : Optional.empty(), values); + } + else { + result = new RunLengthEncodedBlock(NULL_VALUE_BLOCK, positionCount); } - LongArrayBlock result = new LongArrayBlock(positionCount, hasNullValue ? Optional.of(valueIsNull) : Optional.empty(), values); reset(); return result; } diff --git a/core/trino-main/src/main/java/io/trino/operator/output/ShortPositionsAppender.java b/core/trino-main/src/main/java/io/trino/operator/output/ShortPositionsAppender.java index b57305449ef8..0aa1b2748698 100644 --- a/core/trino-main/src/main/java/io/trino/operator/output/ShortPositionsAppender.java +++ b/core/trino-main/src/main/java/io/trino/operator/output/ShortPositionsAppender.java @@ -121,10 +121,13 @@ public void appendRle(RunLengthEncodedBlock block) @Override public Block build() { - if (!hasNonNullValue) { - return new RunLengthEncodedBlock(NULL_VALUE_BLOCK, positionCount); + Block result; + if (hasNonNullValue) { + result = new ShortArrayBlock(positionCount, hasNullValue ? Optional.of(valueIsNull) : Optional.empty(), values); + } + else { + result = new RunLengthEncodedBlock(NULL_VALUE_BLOCK, positionCount); } - ShortArrayBlock result = new ShortArrayBlock(positionCount, hasNullValue ? Optional.of(valueIsNull) : Optional.empty(), values); reset(); return result; } diff --git a/core/trino-main/src/main/java/io/trino/operator/output/SlicePositionsAppender.java b/core/trino-main/src/main/java/io/trino/operator/output/SlicePositionsAppender.java index 5210b04e2d85..63f8ddb3e30a 100644 --- a/core/trino-main/src/main/java/io/trino/operator/output/SlicePositionsAppender.java +++ b/core/trino-main/src/main/java/io/trino/operator/output/SlicePositionsAppender.java @@ -144,14 +144,17 @@ public void appendRle(RunLengthEncodedBlock block) @Override public Block build() { - if (!hasNonNullValue) { - return new RunLengthEncodedBlock(NULL_VALUE_BLOCK, positionCount); + Block result; + if (hasNonNullValue) { + result = new VariableWidthBlock( + positionCount, + Slices.wrappedBuffer(bytes, 0, currentOffset), + offsets, + hasNullValue ? Optional.of(valueIsNull) : Optional.empty()); + } + else { + result = new RunLengthEncodedBlock(NULL_VALUE_BLOCK, positionCount); } - VariableWidthBlock result = new VariableWidthBlock( - positionCount, - Slices.wrappedBuffer(bytes, 0, currentOffset), - offsets, - hasNullValue ? Optional.of(valueIsNull) : Optional.empty()); reset(); return result; } diff --git a/core/trino-main/src/test/java/io/trino/operator/output/TestPositionsAppender.java b/core/trino-main/src/test/java/io/trino/operator/output/TestPositionsAppender.java index 7d65d107f2e3..c9ec00c5effa 100644 --- a/core/trino-main/src/test/java/io/trino/operator/output/TestPositionsAppender.java +++ b/core/trino-main/src/test/java/io/trino/operator/output/TestPositionsAppender.java @@ -176,6 +176,45 @@ public void testMultipleRleWithTheSameValueProduceRle(Type type) assertInstanceOf(actual, RunLengthEncodedBlock.class); } + @Test(dataProvider = "types") + public void testConsecutiveBuilds(Type type) + { + PositionsAppender positionsAppender = POSITIONS_APPENDER_FACTORY.create(type, 10, DEFAULT_MAX_PAGE_SIZE_IN_BYTES); + + // empty block + positionsAppender.append(positions(), emptyBlock(type)); + assertEquals(positionsAppender.build().getPositionCount(), 0); + + Block block = createRandomBlockForType(type, 2, 0.5f); + // append only null position + int nullPosition = block.isNull(0) ? 0 : 1; + positionsAppender.append(positions(nullPosition), block); + Block actualNullBlock = positionsAppender.build(); + assertEquals(actualNullBlock.getPositionCount(), 1); + assertTrue(actualNullBlock.isNull(0)); + + // append null and not null position + positionsAppender.append(allPositions(2), block); + assertBlockEquals(type, positionsAppender.build(), block); + + // append not null rle + Block rleBlock = rleBlock(type, 1); + positionsAppender.append(allPositions(1), rleBlock); + assertBlockEquals(type, positionsAppender.build(), rleBlock); + + // append empty rle + positionsAppender.append(positions(), rleBlock(type, 0)); + assertEquals(positionsAppender.build().getPositionCount(), 0); + + // append null rle + Block nullRleBlock = nullRleBlock(type, 1); + positionsAppender.append(allPositions(1), nullRleBlock); + assertBlockEquals(type, positionsAppender.build(), nullRleBlock); + + // just build to confirm appender was reset + assertEquals(positionsAppender.build().getPositionCount(), 0); + } + @DataProvider(name = "nullRleTypes") public static Object[][] nullRleTypes() { @@ -225,6 +264,11 @@ private BlockView input(Block block, int... positions) return new BlockView(block, new IntArrayList(positions)); } + private static IntArrayList positions(int... positions) + { + return new IntArrayList(positions); + } + private DictionaryBlock dictionaryBlock(Block dictionary, int positionCount) { return createRandomDictionaryBlock(dictionary, positionCount);