Skip all-false nulls array in MapBlockBuilder

trinodb · Aug 17, 2022 · 5bd6b78 · 5bd6b78
1 parent f45ff16
commit 5bd6b78
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 4 deletions.
diff --git a/core/trino-main/src/test/java/io/trino/block/TestMapBlock.java b/core/trino-main/src/test/java/io/trino/block/TestMapBlock.java
@@ -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);
@@ -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);
@@ -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);
@@ -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];
             }
@@ -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)

diff --git a/core/trino-spi/src/main/java/io/trino/spi/block/MapBlock.java b/core/trino-spi/src/main/java/io/trino/spi/block/MapBlock.java
@@ -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;
@@ -201,6 +202,12 @@ protected boolean[] getMapIsNull()
         return mapIsNull;
     }
 
+    @Override
+    public boolean mayHaveNull()
+    {
+        return mapIsNull != null;
+    }
+
     @Override
     public int getPositionCount()
     {

diff --git a/core/trino-spi/src/main/java/io/trino/spi/block/MapBlockBuilder.java b/core/trino-spi/src/main/java/io/trino/spi/block/MapBlockBuilder.java
@@ -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;
@@ -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
@@ -249,6 +257,7 @@ private void entryAdded(boolean isNull)
         }
         offsets[positionCount + 1] = keyBlockBuilder.getPositionCount();
         mapIsNull[positionCount] = isNull;
+        hasNullValue |= isNull;
         positionCount++;
 
         if (blockBuilderStatus != null) {
@@ -279,7 +288,7 @@ public Block build()
                 getMapType(),
                 0,
                 positionCount,
-                Optional.of(mapIsNull),
+                hasNullValue ? Optional.of(mapIsNull) : Optional.empty(),
                 offsets,
                 keyBlockBuilder.build(),
                 valueBlockBuilder.build(),