Skip to content

Skip all-false nulls array in MapBlockBuilder #13710

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
raunaqmorarka merged 1 commit into from
Aug 17, 2022
Merged

Skip all-false nulls array in MapBlockBuilder #13710

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
15 changes: 13 additions & 2 deletions core/trino-main/src/test/java/io/trino/block/TestMapBlock.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ private static Map<String, Long>[] createTestMap(int... entryCounts)
private void testWith(Map<String, Long>[] expectedValues)
{
BlockBuilder blockBuilder = createBlockBuilderWithValues(expectedValues);
assertFalse(blockBuilder.mayHaveNull());

assertBlock(blockBuilder, () -> blockBuilder.newBlockBuilderLike(null), expectedValues);
assertBlock(blockBuilder.build(), () -> blockBuilder.newBlockBuilderLike(null), expectedValues);
Expand All @@ -214,13 +215,15 @@ private void testWith(Map<String, Long>[] expectedValues)
assertBlockFilteredPositions(expectedValues, blockBuilder.build(), () -> blockBuilder.newBlockBuilderLike(null), 2, 3, 5, 6);

Block block = createBlockWithValuesFromKeyValueBlock(expectedValues);
assertFalse(block.mayHaveNull());

assertBlock(block, () -> blockBuilder.newBlockBuilderLike(null), expectedValues);
assertBlockFilteredPositions(expectedValues, block, () -> blockBuilder.newBlockBuilderLike(null), 0, 1, 3, 4, 7);
assertBlockFilteredPositions(expectedValues, block, () -> blockBuilder.newBlockBuilderLike(null), 2, 3, 5, 6);

Map<String, Long>[] expectedValuesWithNull = alternatingNullValues(expectedValues);
BlockBuilder blockBuilderWithNull = createBlockBuilderWithValues(expectedValuesWithNull);
assertTrue(blockBuilderWithNull.mayHaveNull());

assertBlock(blockBuilderWithNull, () -> blockBuilder.newBlockBuilderLike(null), expectedValuesWithNull);
assertBlock(blockBuilderWithNull.build(), () -> blockBuilder.newBlockBuilderLike(null), expectedValuesWithNull);
Expand All @@ -230,6 +233,7 @@ private void testWith(Map<String, Long>[] expectedValues)
assertBlockFilteredPositions(expectedValuesWithNull, blockBuilderWithNull.build(), () -> blockBuilder.newBlockBuilderLike(null), 2, 3, 4, 9, 13, 14);

Block blockWithNull = createBlockWithValuesFromKeyValueBlock(expectedValuesWithNull);
assertTrue(blockWithNull.mayHaveNull());

assertBlock(blockWithNull, () -> blockBuilder.newBlockBuilderLike(null), expectedValuesWithNull);
assertBlockFilteredPositions(expectedValuesWithNull, blockWithNull, () -> blockBuilder.newBlockBuilderLike(null), 0, 1, 5, 6, 7, 10, 11, 12, 15);
Expand All @@ -252,9 +256,12 @@ private MapBlock createBlockWithValuesFromKeyValueBlock(Map<String, Long>[] maps
List<Long> values = new ArrayList<>();
int[] offsets = new int[maps.length + 1];
boolean[] mapIsNull = new boolean[maps.length];
boolean hasNullValue = false;
for (int i = 0; i < maps.length; i++) {
Map<String, Long> map = maps[i];
mapIsNull[i] = map == null;
boolean isNull = map == null;
mapIsNull[i] = isNull;
hasNullValue |= isNull;
if (map == null) {
offsets[i + 1] = offsets[i];
}
Expand All @@ -266,7 +273,11 @@ private MapBlock createBlockWithValuesFromKeyValueBlock(Map<String, Long>[] maps
offsets[i + 1] = offsets[i] + map.size();
}
}
return (MapBlock) mapType(VARCHAR, BIGINT).createBlockFromKeyValue(Optional.of(mapIsNull), offsets, createStringsBlock(keys), createLongsBlock(values));
return (MapBlock) mapType(VARCHAR, BIGINT).createBlockFromKeyValue(
hasNullValue ? Optional.of(mapIsNull) : Optional.empty(),
offsets,
createStringsBlock(keys),
createLongsBlock(values));
}

private void createBlockBuilderWithValues(Map<String, Long> map, BlockBuilder mapBlockBuilder)
Expand Down
7 changes: 7 additions & 0 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 @@ -37,6 +37,7 @@ public class MapBlock
private final int startOffset;
private final int positionCount;

@Nullable
private final boolean[] mapIsNull;
private final int[] offsets;
private final Block keyBlock;
Expand Down Expand Up @@ -201,6 +202,12 @@ protected boolean[] getMapIsNull()
return mapIsNull;
}

@Override
public boolean mayHaveNull()
{
return mapIsNull != null;
}

@Override
public int getPositionCount()
{
Expand Down
13 changes: 11 additions & 2 deletions core/trino-spi/src/main/java/io/trino/spi/block/MapBlockBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public class MapBlockBuilder
private int positionCount;
private int[] offsets;
private boolean[] mapIsNull;
private boolean hasNullValue;
private final BlockBuilder keyBlockBuilder;
private final BlockBuilder valueBlockBuilder;
private final MapHashTables hashTables;
Expand Down Expand Up @@ -120,10 +121,17 @@ protected int getOffsetBase()
return 0;
}

@Nullable
@Override
protected boolean[] getMapIsNull()
{
return mapIsNull;
return hasNullValue ? mapIsNull : null;
}

@Override
public boolean mayHaveNull()
{
return hasNullValue;
}

@Override
Expand Down Expand Up @@ -249,6 +257,7 @@ private void entryAdded(boolean isNull)
}
offsets[positionCount + 1] = keyBlockBuilder.getPositionCount();
mapIsNull[positionCount] = isNull;
hasNullValue |= isNull;
positionCount++;

if (blockBuilderStatus != null) {
Expand Down Expand Up @@ -279,7 +288,7 @@ public Block build()
getMapType(),
0,
positionCount,
Optional.of(mapIsNull),
hasNullValue ? Optional.of(mapIsNull) : Optional.empty(),
Comment thread
raunaqmorarka marked this conversation as resolved.
Outdated
offsets,
keyBlockBuilder.build(),
valueBlockBuilder.build(),
Expand Down