Skip to content

Fix PositionsAppender#build for null Block #12670

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
sopel39 merged 1 commit into from
Jun 3, 2022
Merged

Fix PositionsAppender#build for null Block #12670

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
9 changes: 6 additions & 3 deletions core/trino-main/src/main/java/io/trino/operator/output/BytePositionsAppender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
9 changes: 6 additions & 3 deletions core/trino-main/src/main/java/io/trino/operator/output/Int128PositionsAppender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
9 changes: 6 additions & 3 deletions core/trino-main/src/main/java/io/trino/operator/output/Int96PositionsAppender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
9 changes: 6 additions & 3 deletions core/trino-main/src/main/java/io/trino/operator/output/IntPositionsAppender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
9 changes: 6 additions & 3 deletions core/trino-main/src/main/java/io/trino/operator/output/LongPositionsAppender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
9 changes: 6 additions & 3 deletions core/trino-main/src/main/java/io/trino/operator/output/ShortPositionsAppender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
17 changes: 10 additions & 7 deletions core/trino-main/src/main/java/io/trino/operator/output/SlicePositionsAppender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
44 changes: 44 additions & 0 deletions core/trino-main/src/test/java/io/trino/operator/output/TestPositionsAppender.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down Expand Up @@ -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);
Expand Down