diff --git a/presto-orc/src/main/java/com/facebook/presto/orc/reader/ListSelectiveStreamReader.java b/presto-orc/src/main/java/com/facebook/presto/orc/reader/ListSelectiveStreamReader.java index d61648ad0d96..7ae01d4a6c59 100644 --- a/presto-orc/src/main/java/com/facebook/presto/orc/reader/ListSelectiveStreamReader.java +++ b/presto-orc/src/main/java/com/facebook/presto/orc/reader/ListSelectiveStreamReader.java @@ -29,6 +29,7 @@ import com.facebook.presto.spi.block.Block; import com.facebook.presto.spi.block.BlockLease; import com.facebook.presto.spi.block.RunLengthEncodedBlock; +import com.facebook.presto.spi.type.ArrayType; import com.facebook.presto.spi.type.Type; import com.google.common.collect.Iterables; import com.google.common.collect.Maps; @@ -71,7 +72,7 @@ public class ListSelectiveStreamReader private final boolean nonNullsAllowed; private final boolean outputRequired; @Nullable - private final Type outputType; + private final ArrayType outputType; // elementStreamReader is null if output is not required and filter is a simple IS [NOT] NULL @Nullable private final SelectiveStreamReader elementStreamReader; @@ -129,7 +130,7 @@ public ListSelectiveStreamReader( this.streamDescriptor = requireNonNull(streamDescriptor, "streamDescriptor is null"); this.outputRequired = requireNonNull(outputType, "outputType is null").isPresent(); - this.outputType = outputType.orElse(null); + this.outputType = (ArrayType) outputType.orElse(null); this.level = subfieldLevel; if (listFilter != null) { @@ -189,7 +190,7 @@ private static Optional getTopLevelFilter(Map 0, "positionCount must be greater than zero"); + checkArgument(positionCount > 0 || listFilter != null, "positionCount must be greater than zero"); checkState(!valuesInUse, "BlockLease hasn't been closed yet"); if (!rowGroupOpen) { @@ -267,17 +268,25 @@ private int readAllNulls(int[] positions, int positionCount) if (nullsFilter.testNull()) { outputPositionCount++; } + else { + outputPositionCount -= nullsFilter.getPrecedingPositionsToFail(); + i += nullsFilter.getSucceedingPositionsToFail(); + } } } else { outputPositionCount = positionCount; - allNulls = true; } } else { outputPositionCount = 0; } + if (listFilter != null) { + listFilter.populateElementFilters(0, null, null, 0); + } + + allNulls = true; return positions[positionCount - 1] + 1; } @@ -373,6 +382,9 @@ private int readNotAllNulls(int[] positions, int positionCount) if (elementStreamReader != null && elementPositionCount > 0) { elementStreamReader.read(elementReadOffset, elementPositions, elementPositionCount); } + else if (listFilter != null && listFilter.getChild() != null) { + elementStreamReader.read(elementReadOffset, elementPositions, elementPositionCount); + } elementReadOffset += elementPositionCount + skippedElements; if (listFilter == null || level > 0) { @@ -470,11 +482,15 @@ public Block getBlock(int[] positions, int positionCount) boolean mayHaveNulls = nullsAllowed && presentStream != null; if (positionCount == outputPositionCount) { + Block elementBlock; if (elementOutputPositionCount == 0) { - return new RunLengthEncodedBlock(outputType.createBlockBuilder(null, 1).appendNull().build(), outputPositionCount); + elementBlock = outputType.getElementType().createBlockBuilder(null, 0).build(); + } + else { + elementBlock = elementStreamReader.getBlock(elementOutputPositions, elementOutputPositionCount); } - Block block = ArrayBlock.fromElementBlock(positionCount, Optional.ofNullable(mayHaveNulls ? nulls : null), offsets, elementStreamReader.getBlock(elementOutputPositions, elementOutputPositionCount)); + Block block = ArrayBlock.fromElementBlock(positionCount, Optional.ofNullable(mayHaveNulls ? nulls : null), offsets, elementBlock); nulls = null; offsets = null; return block; @@ -537,12 +553,15 @@ public BlockLease getBlockView(int[] positions, int positionCount) compactValues(positions, positionCount, includeNulls); } + BlockLease elementBlockLease; if (elementOutputPositionCount == 0) { - return newLease(new RunLengthEncodedBlock(outputType.createBlockBuilder(null, 1).appendNull().build(), positionCount)); + elementBlockLease = newLease(outputType.getElementType().createBlockBuilder(null, 0).build()); + } + else { + elementBlockLease = elementStreamReader.getBlockView(elementOutputPositions, elementOutputPositionCount); } valuesInUse = true; - BlockLease elementBlockLease = elementStreamReader.getBlockView(elementOutputPositions, elementOutputPositionCount); Block block = ArrayBlock.fromElementBlock(positionCount, Optional.ofNullable(includeNulls ? nulls : null), offsets, elementBlockLease.get()); return newLease(block, () -> closeBlockLease(elementBlockLease)); } diff --git a/presto-orc/src/test/java/com/facebook/presto/orc/OrcTester.java b/presto-orc/src/test/java/com/facebook/presto/orc/OrcTester.java index 7d301785b6a3..cb9f4e1a96af 100644 --- a/presto-orc/src/test/java/com/facebook/presto/orc/OrcTester.java +++ b/presto-orc/src/test/java/com/facebook/presto/orc/OrcTester.java @@ -745,9 +745,12 @@ private static boolean testSubfieldValue(Type type, Object value, Subfield subfi if (nestedValue == null) { return filter == IS_NULL; } - int index = toIntExact(((Subfield.LongSubscript) pathElement).getIndex()); + int index = toIntExact(((Subfield.LongSubscript) pathElement).getIndex()) - 1; nestedType = ((ArrayType) nestedType).getElementType(); - nestedValue = ((List) nestedValue).get(index - 1); + if (index >= ((List) nestedValue).size()) { + return true; + } + nestedValue = ((List) nestedValue).get(index); } else { fail("Unsupported type: " + type); @@ -808,6 +811,7 @@ private static void assertColumnValueEquals(Type type, Object actual, Object exp if (StandardTypes.ARRAY.equals(baseType)) { List actualArray = (List) actual; List expectedArray = (List) expected; + assertEquals(actualArray == null, expectedArray == null); assertEquals(actualArray.size(), expectedArray.size()); Type elementType = type.getTypeParameters().get(0); diff --git a/presto-orc/src/test/java/com/facebook/presto/orc/TestSelectiveOrcReader.java b/presto-orc/src/test/java/com/facebook/presto/orc/TestSelectiveOrcReader.java index 7c05bda5c335..f6fa2bbff3e7 100644 --- a/presto-orc/src/test/java/com/facebook/presto/orc/TestSelectiveOrcReader.java +++ b/presto-orc/src/test/java/com/facebook/presto/orc/TestSelectiveOrcReader.java @@ -18,6 +18,7 @@ import com.facebook.presto.orc.TupleDomainFilter.BooleanValue; import com.facebook.presto.orc.TupleDomainFilter.DoubleRange; import com.facebook.presto.orc.TupleDomainFilter.FloatRange; +import com.facebook.presto.spi.PrestoException; import com.facebook.presto.spi.Subfield; import com.facebook.presto.spi.type.SqlDate; import com.facebook.presto.spi.type.SqlTimestamp; @@ -70,6 +71,8 @@ import static java.util.Collections.nCopies; import static java.util.stream.Collectors.toList; import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertTrue; +import static org.testng.Assert.fail; public class TestSelectiveOrcReader { @@ -370,13 +373,64 @@ public void testArrays() 1, ImmutableMap.of(new Subfield("c[1]"), IS_NULL)))); } + @Test + public void testArrayIndexOutOfBounds() + throws Exception + { + Random random = new Random(0); + + // non-null arrays of varying sizes + try { + tester.testRoundTrip(arrayType(INTEGER), + IntStream.range(0, 30_000).map(i -> random.nextInt(10)).mapToObj(size -> makeArray(size, random)).collect(toImmutableList()), + ImmutableList.of(ImmutableMap.of(new Subfield("c[2]"), IS_NULL))); + fail("Expected 'Array subscript out of bounds' exception"); + } + catch (PrestoException e) { + assertTrue(e.getMessage().contains("Array subscript out of bounds")); + } + + // non-null nested arrays of varying sizes + try { + tester.testRoundTrip(arrayType(arrayType(INTEGER)), + IntStream.range(0, 30_000).mapToObj(i -> ImmutableList.of(makeArray(random.nextInt(5), random), makeArray(random.nextInt(5), random))).collect(toImmutableList()), + ImmutableList.of(ImmutableMap.of(new Subfield("c[2][3]"), IS_NULL))); + fail("Expected 'Array subscript out of bounds' exception"); + } + catch (PrestoException e) { + assertTrue(e.getMessage().contains("Array subscript out of bounds")); + } + + // empty arrays + try { + tester.testRoundTrip(arrayType(INTEGER), + nCopies(30_000, ImmutableList.of()), + ImmutableList.of(ImmutableMap.of(new Subfield("c[2]"), IS_NULL))); + fail("Expected 'Array subscript out of bounds' exception"); + } + catch (PrestoException e) { + assertTrue(e.getMessage().contains("Array subscript out of bounds")); + } + + // empty nested arrays + try { + tester.testRoundTrip(arrayType(arrayType(INTEGER)), + nCopies(30_000, ImmutableList.of()), + ImmutableList.of(ImmutableMap.of(new Subfield("c[2][3]"), IS_NULL))); + fail("Expected 'Array subscript out of bounds' exception"); + } + catch (PrestoException e) { + assertTrue(e.getMessage().contains("Array subscript out of bounds")); + } + } + @Test public void testArraysOfNulls() throws Exception { - for (Type type : ImmutableList.of(BOOLEAN, BIGINT, INTEGER, SMALLINT, TINYINT, DOUBLE, REAL, TIMESTAMP)) { + for (Type type : ImmutableList.of(BOOLEAN, BIGINT, INTEGER, SMALLINT, TINYINT, DOUBLE, REAL, TIMESTAMP, arrayType(INTEGER))) { tester.testRoundTrip(arrayType(type), - IntStream.range(0, 30_000).mapToObj(i -> Collections.nCopies(5, null)).collect(toImmutableList()), + nCopies(30_000, nCopies(5, null)), ImmutableList.of( ImmutableMap.of(new Subfield("c[2]"), IS_NULL), ImmutableMap.of(new Subfield("c[2]"), IS_NOT_NULL))); @@ -418,7 +472,7 @@ public void testMaps() ImmutableList.of( IntStream.range(0, 30_000).mapToObj(i -> random.nextInt()).collect(toImmutableList()), IntStream.range(0, 30_000).boxed().map(i -> createMap(i)).collect(toImmutableList())), - ImmutableList.of( + toSubfieldFilters( ImmutableMap.of(0, BigintRange.of(0, Integer.MAX_VALUE, false)), ImmutableMap.of(1, IS_NOT_NULL), ImmutableMap.of(1, IS_NULL))); @@ -429,7 +483,7 @@ public void testMaps() ImmutableList.of( IntStream.range(0, 30_000).mapToObj(i -> random.nextInt()).collect(toImmutableList()), IntStream.range(0, 30_000).boxed().map(i -> i % 5 == 0 ? null : createMap(i)).collect(toList())), - ImmutableList.of( + toSubfieldFilters( ImmutableMap.of(0, BigintRange.of(0, Integer.MAX_VALUE, false)), ImmutableMap.of(1, IS_NOT_NULL), ImmutableMap.of(1, IS_NULL), @@ -442,7 +496,7 @@ public void testMaps() ImmutableList.of( IntStream.range(0, 30_000).boxed().map(i -> i % 5 == 0 ? null : createMap(i)).collect(toList()), IntStream.range(0, 30_000).mapToObj(i -> random.nextInt()).collect(toImmutableList())), - ImmutableList.of( + toSubfieldFilters( ImmutableMap.of(0, IS_NULL, 1, BigintRange.of(0, Integer.MAX_VALUE, false)), ImmutableMap.of(0, IS_NOT_NULL, 1, BigintRange.of(0, Integer.MAX_VALUE, false)))); }