From 581ca1a475a1d520556344a6fe2fca0606c38cfc Mon Sep 17 00:00:00 2001 From: Martin Traverso Date: Wed, 22 Aug 2012 21:24:31 -0700 Subject: [PATCH] Get rid of empty blocks Filter methods now return Optional<> instead. Operators never produce an empty block. --- src/main/java/com/facebook/presto/Block.java | 1 - .../com/facebook/presto/BlockBuilder.java | 5 +- .../java/com/facebook/presto/DataScan1.java | 7 +- .../java/com/facebook/presto/DataScan2.java | 7 +- .../java/com/facebook/presto/DataScan3.java | 7 +- .../facebook/presto/EmptyPositionBlock.java | 69 ------------- .../com/facebook/presto/EmptyValueBlock.java | 98 ------------------- .../com/facebook/presto/MaskedValueBlock.java | 43 ++++---- .../com/facebook/presto/PositionBlock.java | 3 +- .../facebook/presto/RangePositionBlock.java | 22 ++--- .../presto/RunLengthEncodedBlock.java | 17 ++-- .../presto/UncompressedPositionBlock.java | 17 +--- .../presto/UncompressedValueBlock.java | 17 ++-- .../java/com/facebook/presto/ValueBlock.java | 7 +- .../aggregations/AverageAggregation.java | 11 ++- .../presto/aggregations/SumAggregation.java | 9 +- .../facebook/presto/TestMaskedValueBlock.java | 17 ++-- 17 files changed, 81 insertions(+), 276 deletions(-) delete mode 100644 src/main/java/com/facebook/presto/EmptyPositionBlock.java delete mode 100644 src/main/java/com/facebook/presto/EmptyValueBlock.java diff --git a/src/main/java/com/facebook/presto/Block.java b/src/main/java/com/facebook/presto/Block.java index dc9aae6b2fe17..29226e8ded87d 100644 --- a/src/main/java/com/facebook/presto/Block.java +++ b/src/main/java/com/facebook/presto/Block.java @@ -4,7 +4,6 @@ public interface Block { - boolean isEmpty(); int getCount(); boolean isSorted(); diff --git a/src/main/java/com/facebook/presto/BlockBuilder.java b/src/main/java/com/facebook/presto/BlockBuilder.java index 4bbc1a6e155bc..af2f6317d0965 100644 --- a/src/main/java/com/facebook/presto/BlockBuilder.java +++ b/src/main/java/com/facebook/presto/BlockBuilder.java @@ -1,7 +1,6 @@ package com.facebook.presto; import com.google.common.base.Preconditions; - import io.airlift.units.DataSize; import io.airlift.units.DataSize.Unit; @@ -83,9 +82,7 @@ public ValueBlock build() { flushTupleIfNecessary(); - if (count == 0) { - return EmptyValueBlock.INSTANCE; - } + Preconditions.checkState(count > 0, "Cannot build an empty block"); return new UncompressedValueBlock(Range.create(startPosition, startPosition + count - 1), tupleInfo, sliceOutput.slice()); } diff --git a/src/main/java/com/facebook/presto/DataScan1.java b/src/main/java/com/facebook/presto/DataScan1.java index 0c6bd7e0d7979..ffb57c9c8d698 100644 --- a/src/main/java/com/facebook/presto/DataScan1.java +++ b/src/main/java/com/facebook/presto/DataScan1.java @@ -1,5 +1,6 @@ package com.facebook.presto; +import com.google.common.base.Optional; import com.google.common.base.Predicate; import com.google.common.collect.AbstractIterator; @@ -21,9 +22,9 @@ public DataScan1(Iterator source, Predicate predicate) protected PositionBlock computeNext() { while (source.hasNext()) { - PositionBlock block = source.next().selectPositions(predicate); - if (!block.isEmpty()) { - return block; + Optional block = source.next().selectPositions(predicate); + if (block.isPresent()) { + return block.get(); } } diff --git a/src/main/java/com/facebook/presto/DataScan2.java b/src/main/java/com/facebook/presto/DataScan2.java index 68e90db3a78e9..1d909928d3ec8 100644 --- a/src/main/java/com/facebook/presto/DataScan2.java +++ b/src/main/java/com/facebook/presto/DataScan2.java @@ -1,5 +1,6 @@ package com.facebook.presto; +import com.google.common.base.Optional; import com.google.common.base.Predicate; import com.google.common.collect.AbstractIterator; @@ -21,9 +22,9 @@ public DataScan2(Iterator source, Predicate predicate) protected ValueBlock computeNext() { while (source.hasNext()) { - ValueBlock block = source.next().selectPairs(predicate); - if (!block.isEmpty()) { - return block; + Optional block = source.next().selectPairs(predicate); + if (block.isPresent()) { + return block.get(); } } diff --git a/src/main/java/com/facebook/presto/DataScan3.java b/src/main/java/com/facebook/presto/DataScan3.java index d670e4d21b2c6..04a77ba7e87bf 100644 --- a/src/main/java/com/facebook/presto/DataScan3.java +++ b/src/main/java/com/facebook/presto/DataScan3.java @@ -1,5 +1,6 @@ package com.facebook.presto; +import com.google.common.base.Optional; import com.google.common.collect.AbstractIterator; import java.util.Iterator; @@ -24,9 +25,9 @@ protected ValueBlock computeNext() { while (advance()) { if (valueBlock.getRange().overlaps(positionBlock.getRange())) { - ValueBlock result = valueBlock.filter(positionBlock); - if (!result.isEmpty()) { - return result; + Optional result = valueBlock.filter(positionBlock); + if (result.isPresent()) { + return result.get(); } } } diff --git a/src/main/java/com/facebook/presto/EmptyPositionBlock.java b/src/main/java/com/facebook/presto/EmptyPositionBlock.java deleted file mode 100644 index 14e7e16ce40e3..0000000000000 --- a/src/main/java/com/facebook/presto/EmptyPositionBlock.java +++ /dev/null @@ -1,69 +0,0 @@ -package com.facebook.presto; - -import com.google.common.collect.ImmutableList; - - - -import javax.annotation.Nullable; - -public final class EmptyPositionBlock - implements PositionBlock -{ - public static final EmptyPositionBlock INSTANCE = new EmptyPositionBlock(); - - private EmptyPositionBlock() - { - } - - public PositionBlock filter(PositionBlock positionBlock) { - return this; - } - - @Override - public boolean isEmpty() - { - return true; - } - - @Override - public int getCount() - { - return 0; - } - - @Override - public boolean isSorted() - { - return false; - } - - @Override - public boolean isSingleValue() - { - return false; - } - - @Override - public boolean isPositionsContiguous() - { - return false; - } - - @Override - public Iterable getPositions() - { - return ImmutableList.of(); - } - - @Override - public Range getRange() - { - throw new UnsupportedOperationException("Empty block doesn't have a range"); - } - - @Override - public boolean apply(@Nullable Long input) - { - return false; - } -} diff --git a/src/main/java/com/facebook/presto/EmptyValueBlock.java b/src/main/java/com/facebook/presto/EmptyValueBlock.java deleted file mode 100644 index d5427e840856d..0000000000000 --- a/src/main/java/com/facebook/presto/EmptyValueBlock.java +++ /dev/null @@ -1,98 +0,0 @@ -package com.facebook.presto; - -import com.google.common.base.Predicate; -import com.google.common.collect.ImmutableList; -import com.google.common.collect.Iterators; -import com.google.common.collect.PeekingIterator; - - - -import java.util.Iterator; - -public final class EmptyValueBlock - implements ValueBlock -{ - public final static EmptyValueBlock INSTANCE = new EmptyValueBlock(); - - private EmptyValueBlock() - { - } - - @Override - public PositionBlock selectPositions(Predicate predicate) - { - return EmptyPositionBlock.INSTANCE; - } - - @Override - public ValueBlock selectPairs(Predicate predicate) - { - return this; - } - - @Override - public PositionBlock toPositionBlock() - { - return EmptyPositionBlock.INSTANCE; - } - - @Override - public ValueBlock filter(PositionBlock positions) - { - return this; - } - - @Override - public PeekingIterator pairIterator() - { - return Iterators.peekingIterator(Iterators.emptyIterator()); - } - - @Override - public boolean isEmpty() - { - return true; - } - - @Override - public int getCount() - { - return 0; - } - - @Override - public boolean isSorted() - { - return false; - } - - @Override - public boolean isSingleValue() - { - return false; - } - - @Override - public boolean isPositionsContiguous() - { - return false; - } - - @Override - public Iterable getPositions() - { - return ImmutableList.of(); - } - - @Override - public Range getRange() - { - throw new UnsupportedOperationException("Empty block doesn't have a range"); - } - - @Override - public Iterator iterator() - { - return Iterators.emptyIterator(); - } -} diff --git a/src/main/java/com/facebook/presto/MaskedValueBlock.java b/src/main/java/com/facebook/presto/MaskedValueBlock.java index 05573a5faccbe..e920e35d5b2c6 100644 --- a/src/main/java/com/facebook/presto/MaskedValueBlock.java +++ b/src/main/java/com/facebook/presto/MaskedValueBlock.java @@ -3,6 +3,7 @@ */ package com.facebook.presto; +import com.google.common.base.Optional; import com.google.common.base.Predicate; import com.google.common.collect.Iterators; import com.google.common.collect.PeekingIterator; @@ -14,24 +15,20 @@ public class MaskedValueBlock implements ValueBlock { - public static ValueBlock maskBlock(ValueBlock valueBlock, PositionBlock positions) + public static Optional maskBlock(ValueBlock valueBlock, PositionBlock positions) { - if (positions.isEmpty()) { - return EmptyValueBlock.INSTANCE; - } - if (!valueBlock.getRange().overlaps(positions.getRange())) { - return EmptyValueBlock.INSTANCE; + return Optional.absent(); } Range intersection = valueBlock.getRange().intersect(positions.getRange()); if (valueBlock.isSingleValue() && positions.isPositionsContiguous()) { Tuple value = valueBlock.iterator().next(); - return new RunLengthEncodedBlock(value, intersection); + return Optional.of(new RunLengthEncodedBlock(value, intersection)); } - PositionBlock newPositionBlock; + Optional newPositionBlock; if (valueBlock.isPositionsContiguous()) { newPositionBlock = positions.filter(new RangePositionBlock(valueBlock.getRange())); } @@ -39,7 +36,11 @@ public static ValueBlock maskBlock(ValueBlock valueBlock, PositionBlock position newPositionBlock = positions.filter(valueBlock.toPositionBlock()); } - return new MaskedValueBlock(valueBlock, newPositionBlock); + if (newPositionBlock.isPresent()) { + return Optional.of(new MaskedValueBlock(valueBlock, newPositionBlock.get())); + } + + return Optional.absent(); } private final ValueBlock valueBlock; @@ -52,13 +53,13 @@ private MaskedValueBlock(ValueBlock valueBlock, PositionBlock positionBlock) } @Override - public PositionBlock selectPositions(Predicate predicate) + public Optional selectPositions(Predicate predicate) { throw new UnsupportedOperationException(); } @Override - public ValueBlock selectPairs(Predicate predicate) + public Optional selectPairs(Predicate predicate) { throw new UnsupportedOperationException(); } @@ -70,17 +71,13 @@ public PositionBlock toPositionBlock() } @Override - public ValueBlock filter(PositionBlock positions) + public Optional filter(PositionBlock positions) { - if (positions.isEmpty()) { - return EmptyValueBlock.INSTANCE; - } - - PositionBlock activePositions = positionBlock.filter(positions); - if (activePositions.isEmpty()) { - return EmptyValueBlock.INSTANCE; + Optional activePositions = positionBlock.filter(positions); + if (!activePositions.isPresent()) { + return Optional.absent(); } - return new MaskedValueBlock(valueBlock, activePositions); + return Optional.of(new MaskedValueBlock(valueBlock, activePositions.get())); } @Override @@ -96,12 +93,6 @@ public boolean apply(Pair pair) })); } - @Override - public boolean isEmpty() - { - return false; - } - @Override public int getCount() { diff --git a/src/main/java/com/facebook/presto/PositionBlock.java b/src/main/java/com/facebook/presto/PositionBlock.java index 2688bf01b9bdd..e891dbb597b10 100644 --- a/src/main/java/com/facebook/presto/PositionBlock.java +++ b/src/main/java/com/facebook/presto/PositionBlock.java @@ -1,9 +1,10 @@ package com.facebook.presto; +import com.google.common.base.Optional; import com.google.common.base.Predicate; public interface PositionBlock extends Block, Predicate { - PositionBlock filter(PositionBlock positionBlock); + Optional filter(PositionBlock positionBlock); } diff --git a/src/main/java/com/facebook/presto/RangePositionBlock.java b/src/main/java/com/facebook/presto/RangePositionBlock.java index c40ed13bbb3fc..f7995758a46a3 100644 --- a/src/main/java/com/facebook/presto/RangePositionBlock.java +++ b/src/main/java/com/facebook/presto/RangePositionBlock.java @@ -1,6 +1,7 @@ package com.facebook.presto; import com.google.common.base.Function; +import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; public class RangePositionBlock @@ -14,17 +15,14 @@ public RangePositionBlock(Range range) } @Override - public PositionBlock filter(PositionBlock positionBlock) { - if (positionBlock.isEmpty()) { - return positionBlock; - } - + public Optional filter(PositionBlock positionBlock) + { if (positionBlock.isPositionsContiguous()) { if (!range.overlaps(positionBlock.getRange())) { - return EmptyPositionBlock.INSTANCE; + return Optional.absent(); } - return new RangePositionBlock(range.intersect(positionBlock.getRange())); + return Optional.of(new RangePositionBlock(range.intersect(positionBlock.getRange()))); } ImmutableList.Builder builder = ImmutableList.builder(); @@ -34,16 +32,8 @@ public PositionBlock filter(PositionBlock positionBlock) { builder.add(position); } } - if (positionBlock.isEmpty()) { - return EmptyPositionBlock.INSTANCE; - } - return new UncompressedPositionBlock(builder.build()); - } - @Override - public boolean isEmpty() - { - return false; + return Optional.of(new UncompressedPositionBlock(builder.build())); } @Override diff --git a/src/main/java/com/facebook/presto/RunLengthEncodedBlock.java b/src/main/java/com/facebook/presto/RunLengthEncodedBlock.java index 74a4b76015189..0b2c486d44e87 100644 --- a/src/main/java/com/facebook/presto/RunLengthEncodedBlock.java +++ b/src/main/java/com/facebook/presto/RunLengthEncodedBlock.java @@ -1,6 +1,7 @@ package com.facebook.presto; import com.google.common.base.Function; +import com.google.common.base.Optional; import com.google.common.base.Predicate; import com.google.common.collect.Iterators; import com.google.common.collect.PeekingIterator; @@ -26,15 +27,15 @@ public Tuple getValue() } @Override - public PositionBlock selectPositions(Predicate predicate) + public Optional selectPositions(Predicate predicate) { - return null; + throw new UnsupportedOperationException(); } @Override - public ValueBlock selectPairs(Predicate predicate) + public Optional selectPairs(Predicate predicate) { - return null; + throw new UnsupportedOperationException(); } @Override @@ -44,7 +45,7 @@ public PositionBlock toPositionBlock() } @Override - public ValueBlock filter(PositionBlock positions) + public Optional filter(PositionBlock positions) { return MaskedValueBlock.maskBlock(this, positions); } @@ -68,12 +69,6 @@ public Iterator iterator() return Iterators.peekingIterator(Collections.nCopies(getCount(), value).iterator()); } - @Override - public boolean isEmpty() - { - return false; - } - @Override public int getCount() { diff --git a/src/main/java/com/facebook/presto/UncompressedPositionBlock.java b/src/main/java/com/facebook/presto/UncompressedPositionBlock.java index 8d423c04aa750..224a437617f8e 100644 --- a/src/main/java/com/facebook/presto/UncompressedPositionBlock.java +++ b/src/main/java/com/facebook/presto/UncompressedPositionBlock.java @@ -1,5 +1,6 @@ package com.facebook.presto; +import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; @@ -33,11 +34,7 @@ public UncompressedPositionBlock(List positions) this.range = Range.create(positions.get(0), positions.get(positions.size() - 1)); } - public PositionBlock filter(PositionBlock positionBlock) { - if (positionBlock.isEmpty()) { - return EmptyPositionBlock.INSTANCE; - } - + public Optional filter(PositionBlock positionBlock) { ImmutableList.Builder builder = ImmutableList.builder(); for (Long position : positions) { if (positionBlock.apply(position)) { @@ -45,16 +42,8 @@ public PositionBlock filter(PositionBlock positionBlock) { } } ImmutableList newPositions = builder.build(); - if (newPositions.isEmpty()) { - return EmptyPositionBlock.INSTANCE; - } - return new UncompressedPositionBlock(newPositions); - } - @Override - public boolean isEmpty() - { - return false; + return Optional.of(new UncompressedPositionBlock(newPositions)); } @Override diff --git a/src/main/java/com/facebook/presto/UncompressedValueBlock.java b/src/main/java/com/facebook/presto/UncompressedValueBlock.java index 33b3f6c17e619..dcdd4ea579f23 100644 --- a/src/main/java/com/facebook/presto/UncompressedValueBlock.java +++ b/src/main/java/com/facebook/presto/UncompressedValueBlock.java @@ -1,5 +1,6 @@ package com.facebook.presto; +import com.google.common.base.Optional; import com.google.common.base.Preconditions; import com.google.common.base.Predicate; import com.google.common.collect.AbstractIterator; @@ -28,15 +29,15 @@ public UncompressedValueBlock(Range range, TupleInfo tupleInfo, Slice slice) } @Override - public PositionBlock selectPositions(Predicate predicate) + public Optional selectPositions(Predicate predicate) { - return null; + throw new UnsupportedOperationException(); } @Override - public ValueBlock selectPairs(Predicate predicate) + public Optional selectPairs(Predicate predicate) { - return null; + throw new UnsupportedOperationException(); } @Override @@ -49,7 +50,7 @@ public PositionBlock toPositionBlock() * Build a new block with only the selected value positions */ @Override - public ValueBlock filter(PositionBlock positions) + public Optional filter(PositionBlock positions) { return MaskedValueBlock.maskBlock(this, positions); } @@ -112,12 +113,6 @@ protected Pair computeNext() }); } - @Override - public boolean isEmpty() - { - return false; - } - @Override public int getCount() { diff --git a/src/main/java/com/facebook/presto/ValueBlock.java b/src/main/java/com/facebook/presto/ValueBlock.java index 46e9c7933e315..7b8b27865a624 100644 --- a/src/main/java/com/facebook/presto/ValueBlock.java +++ b/src/main/java/com/facebook/presto/ValueBlock.java @@ -1,17 +1,18 @@ package com.facebook.presto; +import com.google.common.base.Optional; import com.google.common.base.Predicate; import com.google.common.collect.PeekingIterator; public interface ValueBlock extends Block, Iterable { - PositionBlock selectPositions(Predicate predicate); + Optional selectPositions(Predicate predicate); PositionBlock toPositionBlock(); - ValueBlock selectPairs(Predicate predicate); + Optional selectPairs(Predicate predicate); - ValueBlock filter(PositionBlock positions); + Optional filter(PositionBlock positions); PeekingIterator pairIterator(); } diff --git a/src/main/java/com/facebook/presto/aggregations/AverageAggregation.java b/src/main/java/com/facebook/presto/aggregations/AverageAggregation.java index 66b33f827a695..297a5e437ac80 100644 --- a/src/main/java/com/facebook/presto/aggregations/AverageAggregation.java +++ b/src/main/java/com/facebook/presto/aggregations/AverageAggregation.java @@ -4,6 +4,7 @@ import com.facebook.presto.Tuple; import com.facebook.presto.TupleInfo; import com.facebook.presto.ValueBlock; +import com.google.common.base.Optional; public class AverageAggregation implements AggregationFunction @@ -23,11 +24,13 @@ public void add(ValueBlock values, PositionBlock relevantPositions) { // TODO: deal with overflow // TODO: optimize RLE blocks - ValueBlock filtered = values.filter(relevantPositions); + Optional filtered = values.filter(relevantPositions); - for (Tuple value : filtered) { - sum += value.getLong(0); - ++count; + if (filtered.isPresent()) { + for (Tuple value : filtered.get()) { + sum += value.getLong(0); + ++count; + } } } diff --git a/src/main/java/com/facebook/presto/aggregations/SumAggregation.java b/src/main/java/com/facebook/presto/aggregations/SumAggregation.java index 023619a0bfc82..bd4184d1618c7 100644 --- a/src/main/java/com/facebook/presto/aggregations/SumAggregation.java +++ b/src/main/java/com/facebook/presto/aggregations/SumAggregation.java @@ -4,6 +4,7 @@ import com.facebook.presto.Tuple; import com.facebook.presto.TupleInfo; import com.facebook.presto.ValueBlock; +import com.google.common.base.Optional; public class SumAggregation implements AggregationFunction @@ -19,10 +20,12 @@ public TupleInfo getTupleInfo() @Override public void add(ValueBlock values, PositionBlock relevantPositions) { - ValueBlock filtered = values.filter(relevantPositions); + Optional filtered = values.filter(relevantPositions); - for (Tuple value : filtered) { - sum += value.getLong(0); + if (filtered.isPresent()) { + for (Tuple value : filtered.get()) { + sum += value.getLong(0); + } } } diff --git a/src/test/java/com/facebook/presto/TestMaskedValueBlock.java b/src/test/java/com/facebook/presto/TestMaskedValueBlock.java index d8a7673699399..1357cb3a58cf7 100644 --- a/src/test/java/com/facebook/presto/TestMaskedValueBlock.java +++ b/src/test/java/com/facebook/presto/TestMaskedValueBlock.java @@ -3,6 +3,7 @@ */ package com.facebook.presto; +import com.google.common.base.Optional; import com.google.common.collect.ImmutableList; import org.testng.annotations.Test; @@ -20,9 +21,11 @@ public void testMaskUncompressedBlock() throws Exception { ValueBlock valueBlock = createBlock(10, "alice", "bob", "charlie", "david", "eric", "frank", "greg", "hank", "ian", "jenny"); - ValueBlock tuples = MaskedValueBlock.maskBlock(valueBlock, new UncompressedPositionBlock(8, 10, 12, 14, 16, 100)); + Optional masked = MaskedValueBlock.maskBlock(valueBlock, new UncompressedPositionBlock(8, 10, 12, 14, 16, 100)); + + assertTrue(masked.isPresent()); + ValueBlock tuples = masked.get(); - assertFalse(tuples.isEmpty()); assertEquals(tuples.getCount(), 4); assertEquals(ImmutableList.copyOf(tuples.getPositions()), ImmutableList.of(10L, 12L, 14L, 16L)); assertEquals(tuples.getRange(), Range.create(10L, 16L)); @@ -39,7 +42,7 @@ public void testMaskUncompressedBlock() new Pair(14, createTuple("eric")), new Pair(16, createTuple("greg")))); - assertEquals(ImmutableList.copyOf(tuples.filter(new UncompressedPositionBlock(12, 16)).pairIterator()), ImmutableList.of( + assertEquals(ImmutableList.copyOf(tuples.filter(new UncompressedPositionBlock(12, 16)).get().pairIterator()), ImmutableList.of( new Pair(12, createTuple("charlie")), new Pair(16, createTuple("greg")))); } @@ -49,9 +52,11 @@ public void testMaskRunLengthEncodedBlock() throws Exception { ValueBlock valueBlock = new RunLengthEncodedBlock(createTuple("run"), Range.create(10L, 19L)); - ValueBlock tuples = MaskedValueBlock.maskBlock(valueBlock, new RangePositionBlock(Range.create(12L, 15L))); + Optional masked = MaskedValueBlock.maskBlock(valueBlock, new RangePositionBlock(Range.create(12L, 15L))); + + assertTrue(masked.isPresent()); - assertFalse(tuples.isEmpty()); + ValueBlock tuples = masked.get(); assertEquals(tuples.getCount(), 4); assertEquals(ImmutableList.copyOf(tuples.getPositions()), ImmutableList.of(12L, 13L, 14L, 15L)); assertEquals(tuples.getRange(), Range.create(12L, 15L)); @@ -68,7 +73,7 @@ public void testMaskRunLengthEncodedBlock() new Pair(14, createTuple("run")), new Pair(15, createTuple("run")))); - ValueBlock secondFilter = tuples.filter(new UncompressedPositionBlock(13, 15)); + ValueBlock secondFilter = tuples.filter(new UncompressedPositionBlock(13, 15)).get(); assertFalse(secondFilter.isPositionsContiguous()); assertTrue(secondFilter.isSingleValue()); assertTrue(secondFilter.isSorted());