Optimize null checking in Dictionary Blocks

trinodb · Feb 25, 2022 · 3345adb · 3345adb
1 parent a585a57
commit 3345adb
Show file tree

Hide file tree

Showing 3 changed files with 16 additions and 10 deletions.
diff --git a/core/trino-main/src/test/java/io/trino/block/AbstractTestBlock.java b/core/trino-main/src/test/java/io/trino/block/AbstractTestBlock.java
@@ -73,13 +73,15 @@ protected <T> void assertBlock(Block block, Supplier<BlockBuilder> newBlockBuild
         assertBlockSize(block);
         assertRetainedSize(block);
 
-        assertThatThrownBy(() -> block.isNull(-1))
-                .isInstanceOf(IllegalArgumentException.class)
-                .hasMessageMatching(format("(position is not valid|Invalid position -1 in block with %d positions)", block.getPositionCount()));
-
-        assertThatThrownBy(() -> block.isNull(block.getPositionCount()))
-                .isInstanceOf(IllegalArgumentException.class)
-                .hasMessageMatching(format("(position is not valid|Invalid position %d in block with %d positions)", block.getPositionCount(), block.getPositionCount()));
+        if (block.mayHaveNull()) {
+            assertThatThrownBy(() -> block.isNull(-1))
+                    .isInstanceOf(IllegalArgumentException.class)
+                    .hasMessageMatching(format("(position is not valid|Invalid position -1 in block with %d positions)", block.getPositionCount()));
+
+            assertThatThrownBy(() -> block.isNull(block.getPositionCount()))
+                    .isInstanceOf(IllegalArgumentException.class)
+                    .hasMessageMatching(format("(position is not valid|Invalid position %d in block with %d positions)", block.getPositionCount(), block.getPositionCount()));
+        }
     }
 
     private void assertRetainedSize(Block block)

diff --git a/core/trino-spi/src/main/java/io/trino/spi/block/Block.java b/core/trino-spi/src/main/java/io/trino/spi/block/Block.java
@@ -289,7 +289,8 @@ default boolean mayHaveNull()
     /**
      * Is the specified position null?
      *
-     * @throws IllegalArgumentException if this position is not valid
+     * @throws IllegalArgumentException if this position is not valid. The method may return false
+     * without throwing exception when there are no nulls in the block, even if the position is invalid
      */
     boolean isNull(int position);
 

diff --git a/core/trino-spi/src/main/java/io/trino/spi/block/DictionaryBlock.java b/core/trino-spi/src/main/java/io/trino/spi/block/DictionaryBlock.java
@@ -462,14 +462,17 @@ public Block copyRegion(int position, int length)
     @Override
     public boolean mayHaveNull()
     {
-        return positionCount > 0 && dictionary.mayHaveNull();
+        return mayHaveNull && dictionary.mayHaveNull();
     }
 
     @Override
     public boolean isNull(int position)
     {
+        if (!mayHaveNull) {
+            return false;
+        }
         checkValidPosition(position, positionCount);
-        return mayHaveNull && dictionary.isNull(getIdUnchecked(position));
+        return dictionary.isNull(getIdUnchecked(position));
     }
 
     @Override