From df5131c0c1e52d63a71541885bbe3d289e9b585c Mon Sep 17 00:00:00 2001 From: Ying Su Date: Wed, 5 Sep 2018 15:59:54 -0700 Subject: [PATCH] Remove null vector from ArrayBlock, MapBlock, and RowBlock when all values are not null This reverts commit d7256c59dede509e84ae127ecb20810f09de7014. --- .../facebook/presto/hive/HivePageSource.java | 6 ++--- .../hive/parquet/reader/ParquetReader.java | 7 ++--- .../facebook/presto/block/TestArrayBlock.java | 8 +++--- .../facebook/presto/block/TestMapBlock.java | 8 +++--- .../facebook/presto/block/TestRowBlock.java | 8 +++--- .../scalar/BenchmarkArraySubscript.java | 2 +- .../operator/scalar/BenchmarkMapConcat.java | 2 +- .../scalar/BenchmarkMapSubscript.java | 2 +- .../scalar/BenchmarkMapToMapCast.java | 2 +- .../scalar/BenchmarkRowToRowCast.java | 2 +- .../presto/orc/reader/ListStreamReader.java | 6 +++-- .../presto/orc/reader/MapStreamReader.java | 6 +++-- .../presto/orc/reader/StructStreamReader.java | 5 ++-- .../presto/orc/TestStructStreamReader.java | 2 +- .../presto/spi/block/AbstractArrayBlock.java | 8 +++--- .../presto/spi/block/AbstractMapBlock.java | 17 +++++++----- .../presto/spi/block/AbstractRowBlock.java | 8 +++--- .../facebook/presto/spi/block/ArrayBlock.java | 21 ++++++++------- .../facebook/presto/spi/block/MapBlock.java | 27 ++++++++++--------- .../presto/spi/block/MapBlockBuilder.java | 3 ++- .../presto/spi/block/MapBlockEncoding.java | 4 ++- .../facebook/presto/spi/block/RowBlock.java | 26 +++++++++--------- .../com/facebook/presto/spi/type/MapType.java | 5 ++-- .../datatypes/PrestoThriftBigintArray.java | 2 +- 24 files changed, 106 insertions(+), 81 deletions(-) diff --git a/presto-hive/src/main/java/com/facebook/presto/hive/HivePageSource.java b/presto-hive/src/main/java/com/facebook/presto/hive/HivePageSource.java index 8094f886b523..80912e2dc83d 100644 --- a/presto-hive/src/main/java/com/facebook/presto/hive/HivePageSource.java +++ b/presto-hive/src/main/java/com/facebook/presto/hive/HivePageSource.java @@ -514,7 +514,7 @@ public Block apply(Block block) valueIsNull[i] = arrayBlock.isNull(i); offsets[i + 1] = offsets[i] + arrayBlock.getLength(i); } - return ArrayBlock.fromElementBlock(arrayBlock.getPositionCount(), valueIsNull, offsets, elementsBlock); + return ArrayBlock.fromElementBlock(arrayBlock.getPositionCount(), Optional.of(valueIsNull), offsets, elementsBlock); } } @@ -550,7 +550,7 @@ public Block apply(Block block) valueIsNull[i] = mapBlock.isNull(i); offsets[i + 1] = offsets[i] + mapBlock.getEntryCount(i); } - return ((MapType) toType).createBlockFromKeyValue(valueIsNull, offsets, keysBlock, valuesBlock); + return ((MapType) toType).createBlockFromKeyValue(Optional.of(valueIsNull), offsets, keysBlock, valuesBlock); } } @@ -600,7 +600,7 @@ else if (i < rowBlock.getFieldCount()) { for (int i = 0; i < rowBlock.getPositionCount(); i++) { valueIsNull[i] = rowBlock.isNull(i); } - return RowBlock.fromFieldBlocks(valueIsNull, fields); + return RowBlock.fromFieldBlocks(valueIsNull.length, Optional.of(valueIsNull), fields); } } diff --git a/presto-hive/src/main/java/com/facebook/presto/hive/parquet/reader/ParquetReader.java b/presto-hive/src/main/java/com/facebook/presto/hive/parquet/reader/ParquetReader.java index cf6b544fadf2..684e60f37f89 100644 --- a/presto-hive/src/main/java/com/facebook/presto/hive/parquet/reader/ParquetReader.java +++ b/presto-hive/src/main/java/com/facebook/presto/hive/parquet/reader/ParquetReader.java @@ -144,7 +144,7 @@ private ColumnChunk readArray(GroupField field) BooleanList valueIsNull = new BooleanArrayList(); calculateCollectionOffsets(field, offsets, valueIsNull, columnChunk.getDefinitionLevels(), columnChunk.getRepetitionLevels()); - Block arrayBlock = ArrayBlock.fromElementBlock(valueIsNull.size(), valueIsNull.toBooleanArray(), offsets.toIntArray(), columnChunk.getBlock()); + Block arrayBlock = ArrayBlock.fromElementBlock(valueIsNull.size(), Optional.of(valueIsNull.toBooleanArray()), offsets.toIntArray(), columnChunk.getBlock()); return new ColumnChunk(arrayBlock, columnChunk.getDefinitionLevels(), columnChunk.getRepetitionLevels()); } @@ -161,7 +161,7 @@ private ColumnChunk readMap(GroupField field) IntList offsets = new IntArrayList(); BooleanList valueIsNull = new BooleanArrayList(); calculateCollectionOffsets(field, offsets, valueIsNull, columnChunk.getDefinitionLevels(), columnChunk.getRepetitionLevels()); - Block mapBlock = ((MapType) field.getType()).createBlockFromKeyValue(valueIsNull.toBooleanArray(), offsets.toIntArray(), blocks[0], blocks[1]); + Block mapBlock = ((MapType) field.getType()).createBlockFromKeyValue(Optional.of(valueIsNull.toBooleanArray()), offsets.toIntArray(), blocks[0], blocks[1]); return new ColumnChunk(mapBlock, columnChunk.getDefinitionLevels(), columnChunk.getRepetitionLevels()); } @@ -185,7 +185,8 @@ private ColumnChunk readStruct(GroupField field) } } BooleanList structIsNull = ParquetStructColumnReader.calculateStructOffsets(field, columnChunk.getDefinitionLevels(), columnChunk.getRepetitionLevels()); - Block rowBlock = RowBlock.fromFieldBlocks(structIsNull.toBooleanArray(), blocks); + boolean[] structIsNullVector = structIsNull.toBooleanArray(); + Block rowBlock = RowBlock.fromFieldBlocks(structIsNullVector.length, Optional.of(structIsNullVector), blocks); return new ColumnChunk(rowBlock, columnChunk.getDefinitionLevels(), columnChunk.getRepetitionLevels()); } diff --git a/presto-main/src/test/java/com/facebook/presto/block/TestArrayBlock.java b/presto-main/src/test/java/com/facebook/presto/block/TestArrayBlock.java index c42b8a60e644..b9c4b49b5292 100644 --- a/presto-main/src/test/java/com/facebook/presto/block/TestArrayBlock.java +++ b/presto-main/src/test/java/com/facebook/presto/block/TestArrayBlock.java @@ -180,11 +180,11 @@ public void testCompactBlock() int[] offsets = {0, 1, 1, 2, 4, 8, 16}; boolean[] valueIsNull = {false, true, false, false, false, false}; - testCompactBlock(fromElementBlock(0, new boolean[0], new int[1], emptyValueBlock)); - testCompactBlock(fromElementBlock(valueIsNull.length, valueIsNull, offsets, compactValueBlock)); - testIncompactBlock(fromElementBlock(valueIsNull.length - 1, valueIsNull, offsets, compactValueBlock)); + testCompactBlock(fromElementBlock(0, Optional.empty(), new int[1], emptyValueBlock)); + testCompactBlock(fromElementBlock(valueIsNull.length, Optional.of(valueIsNull), offsets, compactValueBlock)); + testIncompactBlock(fromElementBlock(valueIsNull.length - 1, Optional.of(valueIsNull), offsets, compactValueBlock)); // underlying value block is not compact - testIncompactBlock(fromElementBlock(valueIsNull.length, valueIsNull, offsets, inCompactValueBlock)); + testIncompactBlock(fromElementBlock(valueIsNull.length, Optional.of(valueIsNull), offsets, inCompactValueBlock)); } private static BlockBuilder createBlockBuilderWithValues(long[][][] expectedValues) diff --git a/presto-main/src/test/java/com/facebook/presto/block/TestMapBlock.java b/presto-main/src/test/java/com/facebook/presto/block/TestMapBlock.java index ac0d91c71194..9d9eddd67f99 100644 --- a/presto-main/src/test/java/com/facebook/presto/block/TestMapBlock.java +++ b/presto-main/src/test/java/com/facebook/presto/block/TestMapBlock.java @@ -72,12 +72,12 @@ public void testCompactBlock() int[] offsets = {0, 1, 1, 2, 4, 8, 16}; boolean[] mapIsNull = {false, true, false, false, false, false}; - testCompactBlock(mapType(TINYINT, TINYINT).createBlockFromKeyValue(new boolean[0], new int[1], emptyBlock, emptyBlock)); - testCompactBlock(mapType(TINYINT, TINYINT).createBlockFromKeyValue(mapIsNull, offsets, compactKeyBlock, compactValueBlock)); + testCompactBlock(mapType(TINYINT, TINYINT).createBlockFromKeyValue(Optional.empty(), new int[1], emptyBlock, emptyBlock)); + testCompactBlock(mapType(TINYINT, TINYINT).createBlockFromKeyValue(Optional.of(mapIsNull), offsets, compactKeyBlock, compactValueBlock)); // TODO: Add test case for a sliced MapBlock // underlying key/value block is not compact - testIncompactBlock(mapType(TINYINT, TINYINT).createBlockFromKeyValue(mapIsNull, offsets, inCompactKeyBlock, inCompactValueBlock)); + testIncompactBlock(mapType(TINYINT, TINYINT).createBlockFromKeyValue(Optional.of(mapIsNull), offsets, inCompactKeyBlock, inCompactValueBlock)); } private Map[] createTestMap(int... entryCounts) @@ -158,7 +158,7 @@ private Block createBlockWithValuesFromKeyValueBlock(Map[] maps) offsets[i + 1] = offsets[i] + map.size(); } } - return mapType(VARCHAR, BIGINT).createBlockFromKeyValue(mapIsNull, offsets, createStringsBlock(keys), createLongsBlock(values)); + return mapType(VARCHAR, BIGINT).createBlockFromKeyValue(Optional.of(mapIsNull), offsets, createStringsBlock(keys), createLongsBlock(values)); } private void createBlockBuilderWithValues(Map map, BlockBuilder mapBlockBuilder) diff --git a/presto-main/src/test/java/com/facebook/presto/block/TestRowBlock.java b/presto-main/src/test/java/com/facebook/presto/block/TestRowBlock.java index d23ad77873c5..952bb9dea3e9 100644 --- a/presto-main/src/test/java/com/facebook/presto/block/TestRowBlock.java +++ b/presto-main/src/test/java/com/facebook/presto/block/TestRowBlock.java @@ -88,13 +88,13 @@ public void testCompactBlock() Block incompactFiledBlock2 = new ByteArrayBlock(5, Optional.empty(), createExpectedValue(6).getBytes()); boolean[] rowIsNull = {false, true, false, false, false, false}; - assertCompact(fromFieldBlocks(new boolean[0], new Block[] {emptyBlock, emptyBlock})); - assertCompact(fromFieldBlocks(rowIsNull, new Block[] {compactFieldBlock1, compactFieldBlock2})); + assertCompact(fromFieldBlocks(0, Optional.empty(), new Block[] {emptyBlock, emptyBlock})); + assertCompact(fromFieldBlocks(rowIsNull.length, Optional.of(rowIsNull), new Block[] {compactFieldBlock1, compactFieldBlock2})); // TODO: add test case for a sliced RowBlock // underlying field blocks are not compact - testIncompactBlock(fromFieldBlocks(rowIsNull, new Block[] {incompactFiledBlock1, incompactFiledBlock2})); - testIncompactBlock(fromFieldBlocks(rowIsNull, new Block[] {incompactFiledBlock1, incompactFiledBlock2})); + testIncompactBlock(fromFieldBlocks(rowIsNull.length, Optional.of(rowIsNull), new Block[] {incompactFiledBlock1, incompactFiledBlock2})); + testIncompactBlock(fromFieldBlocks(rowIsNull.length, Optional.of(rowIsNull), new Block[] {incompactFiledBlock1, incompactFiledBlock2})); } private void testWith(List fieldTypes, List[] expectedValues) diff --git a/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkArraySubscript.java b/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkArraySubscript.java index 65e8816ebb03..1b44bca04c3b 100644 --- a/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkArraySubscript.java +++ b/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkArraySubscript.java @@ -164,7 +164,7 @@ private static Block createArrayBlock(int positionCount, Block elementsBlock) for (int i = 0; i < offsets.length; i++) { offsets[i] = arraySize * i; } - return ArrayBlock.fromElementBlock(positionCount, new boolean[positionCount], offsets, elementsBlock); + return ArrayBlock.fromElementBlock(positionCount, Optional.empty(), offsets, elementsBlock); } private static Block createFixWidthValueBlock(int positionCount, int mapSize) diff --git a/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapConcat.java b/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapConcat.java index e2c58b258023..b2ac87884e2d 100644 --- a/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapConcat.java +++ b/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapConcat.java @@ -164,7 +164,7 @@ private static Block createMapBlock(MapType mapType, int positionCount, Block ke for (int i = 0; i < offsets.length; i++) { offsets[i] = mapSize * i; } - return mapType.createBlockFromKeyValue(new boolean[positionCount], offsets, keyBlock, valueBlock); + return mapType.createBlockFromKeyValue(Optional.empty(), offsets, keyBlock, valueBlock); } private static Block createKeyBlock(int positionCount, List keys) diff --git a/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapSubscript.java b/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapSubscript.java index e4e0dd6f4907..53e5047e5e52 100644 --- a/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapSubscript.java +++ b/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapSubscript.java @@ -175,7 +175,7 @@ private static Block createMapBlock(MapType mapType, int positionCount, Block ke for (int i = 0; i < offsets.length; i++) { offsets[i] = mapSize * i; } - return mapType.createBlockFromKeyValue(new boolean[positionCount], offsets, keyBlock, valueBlock); + return mapType.createBlockFromKeyValue(Optional.empty(), offsets, keyBlock, valueBlock); } private static Block createKeyBlock(int positionCount, List keys) diff --git a/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapToMapCast.java b/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapToMapCast.java index c35650599807..d5b5b5505093 100644 --- a/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapToMapCast.java +++ b/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkMapToMapCast.java @@ -108,7 +108,7 @@ private static Block createMapBlock(MapType mapType, int positionCount, Block ke for (int i = 0; i < offsets.length; i++) { offsets[i] = mapSize * i; } - return mapType.createBlockFromKeyValue(new boolean[positionCount], offsets, keyBlock, valueBlock); + return mapType.createBlockFromKeyValue(Optional.empty(), offsets, keyBlock, valueBlock); } private static Block createKeyBlock(int positionCount, int mapSize) diff --git a/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkRowToRowCast.java b/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkRowToRowCast.java index f5aab8a17c7d..bbbb8b4961f4 100644 --- a/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkRowToRowCast.java +++ b/presto-main/src/test/java/com/facebook/presto/operator/scalar/BenchmarkRowToRowCast.java @@ -99,7 +99,7 @@ public void setup() Block[] fieldBlocks = fromFieldTypes.stream() .map(type -> createBlock(POSITION_COUNT, type)) .toArray(Block[]::new); - Block rowBlock = fromFieldBlocks(new boolean[POSITION_COUNT], fieldBlocks); + Block rowBlock = fromFieldBlocks(POSITION_COUNT, Optional.empty(), fieldBlocks); page = new Page(rowBlock); } diff --git a/presto-orc/src/main/java/com/facebook/presto/orc/reader/ListStreamReader.java b/presto-orc/src/main/java/com/facebook/presto/orc/reader/ListStreamReader.java index b941b2077d0b..5c2b90fd3913 100644 --- a/presto-orc/src/main/java/com/facebook/presto/orc/reader/ListStreamReader.java +++ b/presto-orc/src/main/java/com/facebook/presto/orc/reader/ListStreamReader.java @@ -34,6 +34,7 @@ import java.io.IOException; import java.io.UncheckedIOException; import java.util.List; +import java.util.Optional; import static com.facebook.presto.orc.metadata.Stream.StreamKind.LENGTH; import static com.facebook.presto.orc.metadata.Stream.StreamKind.PRESENT; @@ -106,7 +107,7 @@ public Block readBlock(Type type) // We will use the offsetVector as the buffer to read the length values from lengthStream, // and the length values will be converted in-place to an offset vector. int[] offsetVector = new int[nextBatchSize + 1]; - boolean[] nullVector = new boolean[nextBatchSize]; + boolean[] nullVector = null; if (presentStream == null) { if (lengthStream == null) { throw new OrcCorruptionException(streamDescriptor.getOrcDataSourceId(), "Value is not null but data stream is not present"); @@ -114,6 +115,7 @@ public Block readBlock(Type type) lengthStream.nextIntVector(nextBatchSize, offsetVector, 0); } else { + nullVector = new boolean[nextBatchSize]; int nullValues = presentStream.getUnsetBits(nextBatchSize, nullVector); if (nullValues != nextBatchSize) { if (lengthStream == null) { @@ -143,7 +145,7 @@ public Block readBlock(Type type) else { elements = elementType.createBlockBuilder(null, 0).build(); } - Block arrayBlock = ArrayBlock.fromElementBlock(nextBatchSize, nullVector, offsetVector, elements); + Block arrayBlock = ArrayBlock.fromElementBlock(nextBatchSize, Optional.ofNullable(nullVector), offsetVector, elements); readOffset = 0; nextBatchSize = 0; diff --git a/presto-orc/src/main/java/com/facebook/presto/orc/reader/MapStreamReader.java b/presto-orc/src/main/java/com/facebook/presto/orc/reader/MapStreamReader.java index b7f262eef0e5..6ec24933a2b9 100644 --- a/presto-orc/src/main/java/com/facebook/presto/orc/reader/MapStreamReader.java +++ b/presto-orc/src/main/java/com/facebook/presto/orc/reader/MapStreamReader.java @@ -35,6 +35,7 @@ import java.io.IOException; import java.io.UncheckedIOException; import java.util.List; +import java.util.Optional; import static com.facebook.presto.orc.metadata.Stream.StreamKind.LENGTH; import static com.facebook.presto.orc.metadata.Stream.StreamKind.PRESENT; @@ -110,7 +111,7 @@ public Block readBlock(Type type) // We will use the offsetVector as the buffer to read the length values from lengthStream, // and the length values will be converted in-place to an offset vector. int[] offsetVector = new int[nextBatchSize + 1]; - boolean[] nullVector = new boolean[nextBatchSize]; + boolean[] nullVector = null; if (presentStream == null) { if (lengthStream == null) { @@ -119,6 +120,7 @@ public Block readBlock(Type type) lengthStream.nextIntVector(nextBatchSize, offsetVector, 0); } else { + nullVector = new boolean[nextBatchSize]; int nullValues = presentStream.getUnsetBits(nextBatchSize, nullVector); if (nullValues != nextBatchSize) { if (lengthStream == null) { @@ -165,7 +167,7 @@ public Block readBlock(Type type) readOffset = 0; nextBatchSize = 0; - return mapType.createBlockFromKeyValue(nullVector, offsetVector, keyValueBlock[0], keyValueBlock[1]); + return mapType.createBlockFromKeyValue(Optional.ofNullable(nullVector), offsetVector, keyValueBlock[0], keyValueBlock[1]); } private static Block[] createKeyValueBlock(int positionCount, Block keys, Block values, int[] lengths) diff --git a/presto-orc/src/main/java/com/facebook/presto/orc/reader/StructStreamReader.java b/presto-orc/src/main/java/com/facebook/presto/orc/reader/StructStreamReader.java index 26f17e25f483..9c8fe5ace52f 100644 --- a/presto-orc/src/main/java/com/facebook/presto/orc/reader/StructStreamReader.java +++ b/presto-orc/src/main/java/com/facebook/presto/orc/reader/StructStreamReader.java @@ -99,13 +99,14 @@ public Block readBlock(Type type) } } - boolean[] nullVector = new boolean[nextBatchSize]; + boolean[] nullVector = null; Block[] blocks; if (presentStream == null) { blocks = getBlocksForType(type, nextBatchSize); } else { + nullVector = new boolean[nextBatchSize]; int nullValues = presentStream.getUnsetBits(nextBatchSize, nullVector); if (nullValues != nextBatchSize) { blocks = getBlocksForType(type, nextBatchSize - nullValues); @@ -125,7 +126,7 @@ public Block readBlock(Type type) .count() == 1); // Struct is represented as a row block - Block rowBlock = RowBlock.fromFieldBlocks(nullVector, blocks); + Block rowBlock = RowBlock.fromFieldBlocks(nextBatchSize, Optional.ofNullable(nullVector), blocks); readOffset = 0; nextBatchSize = 0; diff --git a/presto-orc/src/test/java/com/facebook/presto/orc/TestStructStreamReader.java b/presto-orc/src/test/java/com/facebook/presto/orc/TestStructStreamReader.java index de84f7f65a90..a95cd8683b59 100644 --- a/presto-orc/src/test/java/com/facebook/presto/orc/TestStructStreamReader.java +++ b/presto-orc/src/test/java/com/facebook/presto/orc/TestStructStreamReader.java @@ -234,7 +234,7 @@ private void write(TempFile tempFile, Type writerType, List data) throws fieldBlocks[i] = blockBuilder.build(); blockBuilder = blockBuilder.newBlockBuilderLike(null); } - Block rowBlock = RowBlock.fromFieldBlocks(rowIsNull, fieldBlocks); + Block rowBlock = RowBlock.fromFieldBlocks(rowIsNull.length, Optional.of(rowIsNull), fieldBlocks); writer.write(new Page(rowBlock)); writer.close(); } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractArrayBlock.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractArrayBlock.java index 647a4464eb95..105f6f19b647 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractArrayBlock.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractArrayBlock.java @@ -111,9 +111,10 @@ public Block copyRegion(int position, int length) Block newValues = getRawElementBlock().copyRegion(startValueOffset, endValueOffset - startValueOffset); int[] newOffsets = compactOffsets(getOffsets(), position + getOffsetBase(), length); - boolean[] newValueIsNull = compactArray(getValueIsNull(), position + getOffsetBase(), length); + boolean[] valueIsNull = getValueIsNull(); + boolean[] newValueIsNull = valueIsNull == null ? null : compactArray(valueIsNull, position + getOffsetBase(), length); - if (newValues == getRawElementBlock() && newOffsets == getOffsets() && newValueIsNull == getValueIsNull()) { + if (newValues == getRawElementBlock() && newOffsets == getOffsets() && newValueIsNull == valueIsNull) { return this; } return createArrayBlockInternal(0, length, newValueIsNull, newOffsets, newValues); @@ -180,7 +181,8 @@ public long getEstimatedDataSizeForStats(int position) public boolean isNull(int position) { checkReadablePosition(position); - return getValueIsNull()[position + getOffsetBase()]; + boolean[] valueIsNull = getValueIsNull(); + return valueIsNull == null ? false : valueIsNull[position + getOffsetBase()]; } public T apply(ArrayBlockFunction function, int position) diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractMapBlock.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractMapBlock.java index c280df09422d..87acaf4c59f5 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractMapBlock.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractMapBlock.java @@ -18,6 +18,7 @@ import java.lang.invoke.MethodHandle; import java.util.Arrays; +import java.util.Optional; import static com.facebook.presto.spi.block.BlockUtil.checkArrayRange; import static com.facebook.presto.spi.block.BlockUtil.checkValidRegion; @@ -121,7 +122,7 @@ public Block copyPositions(int[] positions, int offset, int length) Block newKeys = getRawKeyBlock().copyPositions(entriesPositions.elements(), 0, entriesPositions.size()); Block newValues = getRawValueBlock().copyPositions(entriesPositions.elements(), 0, entriesPositions.size()); - return createMapBlockInternal(0, length, newMapIsNull, newOffsets, newKeys, newValues, newHashTable, keyType, keyBlockNativeEquals, keyNativeHashCode); + return createMapBlockInternal(0, length, Optional.of(newMapIsNull), newOffsets, newKeys, newValues, newHashTable, keyType, keyBlockNativeEquals, keyNativeHashCode); } @Override @@ -133,7 +134,7 @@ public Block getRegion(int position, int length) return createMapBlockInternal( position + getOffsetBase(), length, - getMapIsNull(), + Optional.ofNullable(getMapIsNull()), getOffsets(), getRawKeyBlock(), getRawValueBlock(), @@ -171,16 +172,17 @@ public Block copyRegion(int position, int length) Block newValues = getRawValueBlock().copyRegion(startValueOffset, endValueOffset - startValueOffset); int[] newOffsets = compactOffsets(getOffsets(), position + getOffsetBase(), length); - boolean[] newMapIsNull = compactArray(getMapIsNull(), position + getOffsetBase(), length); + boolean[] mapIsNull = getMapIsNull(); + boolean[] newMapIsNull = mapIsNull == null ? null : compactArray(mapIsNull, position + getOffsetBase(), length); int[] newHashTable = compactArray(getHashTables(), startValueOffset * HASH_MULTIPLIER, (endValueOffset - startValueOffset) * HASH_MULTIPLIER); - if (newKeys == getRawKeyBlock() && newValues == getRawValueBlock() && newOffsets == getOffsets() && newMapIsNull == getMapIsNull() && newHashTable == getHashTables()) { + if (newKeys == getRawKeyBlock() && newValues == getRawValueBlock() && newOffsets == getOffsets() && newMapIsNull == mapIsNull && newHashTable == getHashTables()) { return this; } return createMapBlockInternal( 0, length, - newMapIsNull, + Optional.ofNullable(newMapIsNull), newOffsets, newKeys, newValues, @@ -233,7 +235,7 @@ public Block getSingleValueBlock(int position) return createMapBlockInternal( 0, 1, - new boolean[] {isNull(position)}, + Optional.of(new boolean[] {isNull(position)}), new int[] {0, valueLength}, newKeys, newValues, @@ -269,7 +271,8 @@ public long getEstimatedDataSizeForStats(int position) public boolean isNull(int position) { checkReadablePosition(position); - return getMapIsNull()[position + getOffsetBase()]; + boolean[] mapIsNull = getMapIsNull(); + return mapIsNull != null && mapIsNull[position + getOffsetBase()]; } private void checkReadablePosition(int position) diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractRowBlock.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractRowBlock.java index c856d5321fb7..67dcad0c4e8f 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractRowBlock.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/AbstractRowBlock.java @@ -122,9 +122,10 @@ public Block copyRegion(int position, int length) } int[] newOffsets = compactOffsets(getFieldBlockOffsets(), position + getOffsetBase(), length); - boolean[] newRowIsNull = compactArray(getRowIsNull(), position + getOffsetBase(), length); + boolean[] rowIsNull = getRowIsNull(); + boolean[] newRowIsNull = rowIsNull == null ? null : compactArray(rowIsNull, position + getOffsetBase(), length); - if (arraySame(newBlocks, getRawFieldBlocks()) && newOffsets == getFieldBlockOffsets() && newRowIsNull == getRowIsNull()) { + if (arraySame(newBlocks, getRawFieldBlocks()) && newOffsets == getFieldBlockOffsets() && newRowIsNull == rowIsNull) { return this; } return createRowBlockInternal(0, length, newRowIsNull, newOffsets, newBlocks); @@ -187,7 +188,8 @@ public long getEstimatedDataSizeForStats(int position) public boolean isNull(int position) { checkReadablePosition(position); - return getRowIsNull()[position + getOffsetBase()]; + boolean[] rowIsNull = getRowIsNull(); + return rowIsNull != null && rowIsNull[position + getOffsetBase()]; } private void checkReadablePosition(int position) diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/ArrayBlock.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/ArrayBlock.java index fcd7a33c3ab6..79803e84dad4 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/block/ArrayBlock.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/ArrayBlock.java @@ -15,6 +15,9 @@ import org.openjdk.jol.info.ClassLayout; +import javax.annotation.Nullable; + +import java.util.Optional; import java.util.function.BiConsumer; import static io.airlift.slice.SizeOf.sizeOf; @@ -39,9 +42,9 @@ public class ArrayBlock * Create an array block directly from columnar nulls, values, and offsets into the values. * A null array must have no entries. */ - public static Block fromElementBlock(int positionCount, boolean[] valueIsNull, int[] arrayOffset, Block values) + public static Block fromElementBlock(int positionCount, Optional valueIsNull, int[] arrayOffset, Block values) { - validateConstructorArguments(0, positionCount, valueIsNull, arrayOffset, values); + validateConstructorArguments(0, positionCount, valueIsNull.orElse(null), arrayOffset, values); // for performance reasons per element checks are only performed on the public construction for (int i = 0; i < positionCount; i++) { int offset = arrayOffset[i]; @@ -49,23 +52,23 @@ public static Block fromElementBlock(int positionCount, boolean[] valueIsNull, i if (length < 0) { throw new IllegalArgumentException(format("Offset is not monotonically ascending. offsets[%s]=%s, offsets[%s]=%s", i, arrayOffset[i], i + 1, arrayOffset[i + 1])); } - if (valueIsNull[i] && length != 0) { + if (valueIsNull.isPresent() && valueIsNull.get()[i] && length != 0) { throw new IllegalArgumentException("A null array must have zero entries"); } } - return new ArrayBlock(0, positionCount, valueIsNull, arrayOffset, values); + return new ArrayBlock(0, positionCount, valueIsNull.orElse(null), arrayOffset, values); } /** * Create an array block directly without per element validations. */ - static ArrayBlock createArrayBlockInternal(int arrayOffset, int positionCount, boolean[] valueIsNull, int[] offsets, Block values) + static ArrayBlock createArrayBlockInternal(int arrayOffset, int positionCount, @Nullable boolean[] valueIsNull, int[] offsets, Block values) { validateConstructorArguments(arrayOffset, positionCount, valueIsNull, offsets, values); return new ArrayBlock(arrayOffset, positionCount, valueIsNull, offsets, values); } - private static void validateConstructorArguments(int arrayOffset, int positionCount, boolean[] valueIsNull, int[] offsets, Block values) + private static void validateConstructorArguments(int arrayOffset, int positionCount, @Nullable boolean[] valueIsNull, int[] offsets, Block values) { if (arrayOffset < 0) { throw new IllegalArgumentException("arrayOffset is negative"); @@ -75,8 +78,7 @@ private static void validateConstructorArguments(int arrayOffset, int positionCo throw new IllegalArgumentException("positionCount is negative"); } - requireNonNull(valueIsNull, "valueIsNull is null"); - if (valueIsNull.length - arrayOffset < positionCount) { + if (valueIsNull != null && valueIsNull.length - arrayOffset < positionCount) { throw new IllegalArgumentException("isNull length is less than positionCount"); } @@ -92,7 +94,7 @@ private static void validateConstructorArguments(int arrayOffset, int positionCo * Use createArrayBlockInternal or fromElementBlock instead of this method. The caller of this method is assumed to have * validated the arguments with validateConstructorArguments. */ - private ArrayBlock(int arrayOffset, int positionCount, boolean[] valueIsNull, int[] offsets, Block values) + private ArrayBlock(int arrayOffset, int positionCount, @Nullable boolean[] valueIsNull, int[] offsets, Block values) { // caller must check arguments with validateConstructorArguments this.arrayOffset = arrayOffset; @@ -161,6 +163,7 @@ protected int getOffsetBase() } @Override + @Nullable protected boolean[] getValueIsNull() { return valueIsNull; diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlock.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlock.java index 17a03277cd1b..516fc1830337 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlock.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlock.java @@ -18,8 +18,11 @@ import com.facebook.presto.spi.type.Type; import org.openjdk.jol.info.ClassLayout; +import javax.annotation.Nullable; + import java.lang.invoke.MethodHandle; import java.util.Arrays; +import java.util.Optional; import java.util.function.BiConsumer; import static com.facebook.presto.spi.block.MapBlockBuilder.buildHashTable; @@ -53,7 +56,7 @@ public class MapBlock * @param keyNativeHashCode hash of a key stack type; signature is (K)long */ public static MapBlock fromKeyValueBlock( - boolean[] mapIsNull, + Optional mapIsNull, int[] offsets, Block keyBlock, Block valueBlock, @@ -62,9 +65,9 @@ public static MapBlock fromKeyValueBlock( MethodHandle keyNativeHashCode, MethodHandle keyBlockHashCode) { - validateConstructorArguments(0, mapIsNull.length, mapIsNull, offsets, keyBlock, valueBlock, mapType.getKeyType(), keyBlockNativeEquals, keyNativeHashCode); + validateConstructorArguments(0, offsets.length - 1, mapIsNull.orElse(null), offsets, keyBlock, valueBlock, mapType.getKeyType(), keyBlockNativeEquals, keyNativeHashCode); - int mapCount = mapIsNull.length; + int mapCount = offsets.length - 1; int elementCount = keyBlock.getPositionCount(); int[] hashTables = new int[elementCount * HASH_MULTIPLIER]; Arrays.fill(hashTables, -1); @@ -74,7 +77,7 @@ public static MapBlock fromKeyValueBlock( if (keyCount < 0) { throw new IllegalArgumentException(format("Offset is not monotonically ascending. offsets[%s]=%s, offsets[%s]=%s", i, offsets[i], i + 1, offsets[i + 1])); } - if (mapIsNull[i] && keyCount != 0) { + if (mapIsNull.isPresent() && mapIsNull.get()[i] && keyCount != 0) { throw new IllegalArgumentException("A null map must have zero entries"); } buildHashTable(keyBlock, keyOffset, keyCount, keyBlockHashCode, hashTables, keyOffset * HASH_MULTIPLIER, keyCount * HASH_MULTIPLIER); @@ -105,7 +108,7 @@ public static MapBlock fromKeyValueBlock( public static MapBlock createMapBlockInternal( int startOffset, int positionCount, - boolean[] mapIsNull, + Optional mapIsNull, int[] offsets, Block keyBlock, Block valueBlock, @@ -114,14 +117,14 @@ public static MapBlock createMapBlockInternal( MethodHandle keyBlockNativeEquals, MethodHandle keyNativeHashCode) { - validateConstructorArguments(startOffset, positionCount, mapIsNull, offsets, keyBlock, valueBlock, keyType, keyBlockNativeEquals, keyNativeHashCode); - return new MapBlock(startOffset, positionCount, mapIsNull, offsets, keyBlock, valueBlock, hashTables, keyType, keyBlockNativeEquals, keyNativeHashCode); + validateConstructorArguments(startOffset, positionCount, mapIsNull.orElse(null), offsets, keyBlock, valueBlock, keyType, keyBlockNativeEquals, keyNativeHashCode); + return new MapBlock(startOffset, positionCount, mapIsNull.orElse(null), offsets, keyBlock, valueBlock, hashTables, keyType, keyBlockNativeEquals, keyNativeHashCode); } private static void validateConstructorArguments( int startOffset, int positionCount, - boolean[] mapIsNull, + @Nullable boolean[] mapIsNull, int[] offsets, Block keyBlock, Block valueBlock, @@ -137,8 +140,7 @@ private static void validateConstructorArguments( throw new IllegalArgumentException("positionCount is negative"); } - requireNonNull(mapIsNull, "mapIsNull is null"); - if (mapIsNull.length - startOffset < positionCount) { + if (mapIsNull != null && mapIsNull.length - startOffset < positionCount) { throw new IllegalArgumentException("isNull length is less than positionCount"); } @@ -165,7 +167,7 @@ private static void validateConstructorArguments( private MapBlock( int startOffset, int positionCount, - boolean[] mapIsNull, + @Nullable boolean[] mapIsNull, int[] offsets, Block keyBlock, Block valueBlock, @@ -224,6 +226,7 @@ protected int getOffsetBase() } @Override + @Nullable protected boolean[] getMapIsNull() { return mapIsNull; @@ -296,7 +299,7 @@ public Block getLoadedBlock() return createMapBlockInternal( startOffset, positionCount, - mapIsNull, + Optional.ofNullable(mapIsNull), offsets, keyBlock, loadedValueBlock, diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlockBuilder.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlockBuilder.java index cb10c0e11e50..5b6ffcfda318 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlockBuilder.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlockBuilder.java @@ -22,6 +22,7 @@ import java.lang.invoke.MethodHandle; import java.util.Arrays; +import java.util.Optional; import java.util.function.BiConsumer; import static com.facebook.presto.spi.StandardErrorCode.NOT_SUPPORTED; @@ -306,7 +307,7 @@ public Block build() return createMapBlockInternal( 0, positionCount, - mapIsNull, + Optional.of(mapIsNull), offsets, keyBlockBuilder.build(), valueBlockBuilder.build(), diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlockEncoding.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlockEncoding.java index f876b70e4271..cc0b43537b65 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlockEncoding.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/MapBlockEncoding.java @@ -21,6 +21,8 @@ import io.airlift.slice.SliceInput; import io.airlift.slice.SliceOutput; +import java.util.Optional; + import static com.facebook.presto.spi.block.AbstractMapBlock.HASH_MULTIPLIER; import static io.airlift.slice.Slices.wrappedIntArray; import static java.lang.String.format; @@ -91,7 +93,7 @@ public Block readBlock(BlockEncodingSerde blockEncodingSerde, SliceInput sliceIn int positionCount = sliceInput.readInt(); int[] offsets = new int[positionCount + 1]; sliceInput.readBytes(wrappedIntArray(offsets)); - boolean[] mapIsNull = EncoderUtil.decodeNullBits(sliceInput, positionCount).orElseGet(() -> new boolean[positionCount]); + Optional mapIsNull = EncoderUtil.decodeNullBits(sliceInput, positionCount); return MapType.createMapBlockInternal(typeManager, keyType, 0, positionCount, mapIsNull, offsets, keyBlock, valueBlock, hashTable); } } diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/block/RowBlock.java b/presto-spi/src/main/java/com/facebook/presto/spi/block/RowBlock.java index a86e67aabea6..52a7ceeaf622 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/block/RowBlock.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/block/RowBlock.java @@ -15,6 +15,9 @@ import org.openjdk.jol.info.ClassLayout; +import javax.annotation.Nullable; + +import java.util.Optional; import java.util.function.BiConsumer; import static io.airlift.slice.SizeOf.sizeOf; @@ -39,27 +42,26 @@ public class RowBlock /** * Create a row block directly from columnar nulls and field blocks. */ - public static Block fromFieldBlocks(boolean[] rowIsNull, Block[] fieldBlocks) + public static Block fromFieldBlocks(int positionCount, Optional rowIsNull, Block[] fieldBlocks) { - requireNonNull(rowIsNull, "rowIsNull is null"); - int[] fieldBlockOffsets = new int[rowIsNull.length + 1]; - for (int position = 0; position < rowIsNull.length; position++) { - fieldBlockOffsets[position + 1] = fieldBlockOffsets[position] + (rowIsNull[position] ? 0 : 1); + int[] fieldBlockOffsets = new int[positionCount + 1]; + for (int position = 0; position < positionCount; position++) { + fieldBlockOffsets[position + 1] = fieldBlockOffsets[position] + (rowIsNull.isPresent() && rowIsNull.get()[position] ? 0 : 1); } - validateConstructorArguments(0, rowIsNull.length, rowIsNull, fieldBlockOffsets, fieldBlocks); - return new RowBlock(0, rowIsNull.length, rowIsNull, fieldBlockOffsets, fieldBlocks); + validateConstructorArguments(0, positionCount, rowIsNull.orElse(null), fieldBlockOffsets, fieldBlocks); + return new RowBlock(0, positionCount, rowIsNull.orElse(null), fieldBlockOffsets, fieldBlocks); } /** * Create a row block directly without per element validations. */ - static RowBlock createRowBlockInternal(int startOffset, int positionCount, boolean[] rowIsNull, int[] fieldBlockOffsets, Block[] fieldBlocks) + static RowBlock createRowBlockInternal(int startOffset, int positionCount, @Nullable boolean[] rowIsNull, int[] fieldBlockOffsets, Block[] fieldBlocks) { validateConstructorArguments(startOffset, positionCount, rowIsNull, fieldBlockOffsets, fieldBlocks); return new RowBlock(startOffset, positionCount, rowIsNull, fieldBlockOffsets, fieldBlocks); } - private static void validateConstructorArguments(int startOffset, int positionCount, boolean[] rowIsNull, int[] fieldBlockOffsets, Block[] fieldBlocks) + private static void validateConstructorArguments(int startOffset, int positionCount, @Nullable boolean[] rowIsNull, int[] fieldBlockOffsets, Block[] fieldBlocks) { if (startOffset < 0) { throw new IllegalArgumentException("arrayOffset is negative"); @@ -69,8 +71,7 @@ private static void validateConstructorArguments(int startOffset, int positionCo throw new IllegalArgumentException("positionCount is negative"); } - requireNonNull(rowIsNull, "rowIsNull is null"); - if (rowIsNull.length - startOffset < positionCount) { + if (rowIsNull != null && rowIsNull.length - startOffset < positionCount) { throw new IllegalArgumentException("rowIsNull length is less than positionCount"); } @@ -97,7 +98,7 @@ private static void validateConstructorArguments(int startOffset, int positionCo * Use createRowBlockInternal or fromFieldBlocks instead of this method. The caller of this method is assumed to have * validated the arguments with validateConstructorArguments. */ - private RowBlock(int startOffset, int positionCount, boolean[] rowIsNull, int[] fieldBlockOffsets, Block[] fieldBlocks) + private RowBlock(int startOffset, int positionCount, @Nullable boolean[] rowIsNull, int[] fieldBlockOffsets, Block[] fieldBlocks) { super(fieldBlocks.length); @@ -134,6 +135,7 @@ protected int getOffsetBase() } @Override + @Nullable protected boolean[] getRowIsNull() { return rowIsNull; diff --git a/presto-spi/src/main/java/com/facebook/presto/spi/type/MapType.java b/presto-spi/src/main/java/com/facebook/presto/spi/type/MapType.java index cf2e06021172..80e9ab8221e8 100644 --- a/presto-spi/src/main/java/com/facebook/presto/spi/type/MapType.java +++ b/presto-spi/src/main/java/com/facebook/presto/spi/type/MapType.java @@ -26,6 +26,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.Optional; import static com.facebook.presto.spi.type.BigintType.BIGINT; import static com.facebook.presto.spi.type.TypeUtils.checkElementNotNull; @@ -250,7 +251,7 @@ public String getDisplayName() return "map(" + keyType.getDisplayName() + ", " + valueType.getDisplayName() + ")"; } - public Block createBlockFromKeyValue(boolean[] mapIsNull, int[] offsets, Block keyBlock, Block valueBlock) + public Block createBlockFromKeyValue(Optional mapIsNull, int[] offsets, Block keyBlock, Block valueBlock) { return MapBlock.fromKeyValueBlock( mapIsNull, @@ -273,7 +274,7 @@ public static Block createMapBlockInternal( Type keyType, int startOffset, int positionCount, - boolean[] mapIsNull, + Optional mapIsNull, int[] offsets, Block keyBlock, Block valueBlock, diff --git a/presto-thrift-connector-api/src/main/java/com/facebook/presto/connector/thrift/api/datatypes/PrestoThriftBigintArray.java b/presto-thrift-connector-api/src/main/java/com/facebook/presto/connector/thrift/api/datatypes/PrestoThriftBigintArray.java index fb0103ff5636..69ea9f55ffdd 100644 --- a/presto-thrift-connector-api/src/main/java/com/facebook/presto/connector/thrift/api/datatypes/PrestoThriftBigintArray.java +++ b/presto-thrift-connector-api/src/main/java/com/facebook/presto/connector/thrift/api/datatypes/PrestoThriftBigintArray.java @@ -96,7 +96,7 @@ public Block toBlock(Type desiredType) int numberOfRecords = numberOfRecords(); return ArrayBlock.fromElementBlock( numberOfRecords, - nulls == null ? new boolean[numberOfRecords] : nulls, + Optional.of(nulls == null ? new boolean[numberOfRecords] : nulls), calculateOffsets(sizes, nulls, numberOfRecords), values != null ? values.toBlock(BIGINT) : new LongArrayBlock(0, Optional.empty(), new long[] {})); }