From 3761fb46386008663809cf678efaf49955dff9dd Mon Sep 17 00:00:00 2001 From: leventov Date: Mon, 18 Dec 2017 21:53:03 +0300 Subject: [PATCH 1/3] Reuse IndexedInts in DimensionSelector implementations --- .../ForwardingFilteredDimensionSelector.java | 9 ++- .../PredicateFilteredDimensionSelector.java | 9 ++- .../RowBasedColumnSelectorFactory.java | 5 +- ...ngStringGroupByColumnSelectorStrategy.java | 15 ++-- .../BaseObjectColumnValueSelector.java | 11 +++ .../io/druid/segment/ColumnValueSelector.java | 3 +- .../CompressedVSizeIndexedSupplier.java | 36 ++------- .../io/druid/segment/DimensionSelector.java | 12 ++- .../segment/SingleScanTimeDimSelector.java | 4 +- .../druid/segment/StringDimensionIndexer.java | 4 +- .../column/SimpleDictionaryEncodedColumn.java | 8 +- .../segment/data/ArrayBasedIndexedInts.java | 59 +++++++++------ .../druid/segment/data/RangeIndexedInts.java | 30 +++----- .../druid/segment/data/SingleIndexedInt.java | 28 +++---- .../druid/segment/data/SliceIndexedInts.java | 74 +++++++++++++++++++ .../SingleStringInputDimensionSelector.java | 7 +- .../aggregation/FilteredAggregatorTest.java | 6 +- .../dimension/TestDimensionSelector.java | 2 +- .../CompressedVSizeIndexedV3WriterTest.java | 2 +- .../druid/segment/data/IndexedIntsTest.java | 2 +- 20 files changed, 206 insertions(+), 120 deletions(-) create mode 100644 processing/src/main/java/io/druid/segment/data/SliceIndexedInts.java diff --git a/processing/src/main/java/io/druid/query/dimension/ForwardingFilteredDimensionSelector.java b/processing/src/main/java/io/druid/query/dimension/ForwardingFilteredDimensionSelector.java index 880616ef6a21..e64c10e0dc86 100644 --- a/processing/src/main/java/io/druid/query/dimension/ForwardingFilteredDimensionSelector.java +++ b/processing/src/main/java/io/druid/query/dimension/ForwardingFilteredDimensionSelector.java @@ -42,6 +42,7 @@ final class ForwardingFilteredDimensionSelector implements DimensionSelector, Id private final IdLookup baseIdLookup; private final Int2IntOpenHashMap forwardMapping; private final int[] reverseMapping; + private final ArrayBasedIndexedInts row = new ArrayBasedIndexedInts(); /** * @param selector must return true from {@link DimensionSelector#nameLookupPossibleInAdvance()} @@ -70,15 +71,17 @@ public IndexedInts getRow() { IndexedInts baseRow = selector.getRow(); int baseRowSize = baseRow.size(); - int[] result = new int[baseRowSize]; + row.ensureSize(baseRowSize); int resultSize = 0; for (int i = 0; i < baseRowSize; i++) { int forwardedValue = forwardMapping.get(baseRow.get(i)); if (forwardedValue >= 0) { - result[resultSize++] = forwardedValue; + row.setValue(resultSize, forwardedValue); + resultSize++; } } - return ArrayBasedIndexedInts.of(result, resultSize); + row.setSize(resultSize); + return row; } @Override diff --git a/processing/src/main/java/io/druid/query/dimension/PredicateFilteredDimensionSelector.java b/processing/src/main/java/io/druid/query/dimension/PredicateFilteredDimensionSelector.java index 0155ccd98d2f..32143a857230 100644 --- a/processing/src/main/java/io/druid/query/dimension/PredicateFilteredDimensionSelector.java +++ b/processing/src/main/java/io/druid/query/dimension/PredicateFilteredDimensionSelector.java @@ -34,6 +34,7 @@ final class PredicateFilteredDimensionSelector implements DimensionSelector { private final DimensionSelector selector; private final Predicate predicate; + private final ArrayBasedIndexedInts row = new ArrayBasedIndexedInts(); PredicateFilteredDimensionSelector(DimensionSelector selector, Predicate predicate) { @@ -46,14 +47,16 @@ public IndexedInts getRow() { IndexedInts baseRow = selector.getRow(); int baseRowSize = baseRow.size(); - int[] result = new int[baseRowSize]; + row.ensureSize(baseRowSize); int resultSize = 0; for (int i = 0; i < baseRowSize; i++) { if (predicate.apply(selector.lookupName(baseRow.get(i)))) { - result[resultSize++] = i; + row.setValue(resultSize, i); + resultSize++; } } - return ArrayBasedIndexedInts.of(result, resultSize); + row.setSize(resultSize); + return row; } @Override diff --git a/processing/src/main/java/io/druid/query/groupby/RowBasedColumnSelectorFactory.java b/processing/src/main/java/io/druid/query/groupby/RowBasedColumnSelectorFactory.java index fcc62546f479..70e53fa37d32 100644 --- a/processing/src/main/java/io/druid/query/groupby/RowBasedColumnSelectorFactory.java +++ b/processing/src/main/java/io/druid/query/groupby/RowBasedColumnSelectorFactory.java @@ -203,11 +203,14 @@ public void inspectRuntimeShape(RuntimeShapeInspector inspector) } else { return new DimensionSelector() { + private final RangeIndexedInts indexedInts = new RangeIndexedInts(); + @Override public IndexedInts getRow() { final List dimensionValues = row.get().getDimension(dimension); - return RangeIndexedInts.create(dimensionValues != null ? dimensionValues.size() : 0); + indexedInts.setSize(dimensionValues != null ? dimensionValues.size() : 0); + return indexedInts; } @Override diff --git a/processing/src/main/java/io/druid/query/groupby/epinephelinae/column/DictionaryBuildingStringGroupByColumnSelectorStrategy.java b/processing/src/main/java/io/druid/query/groupby/epinephelinae/column/DictionaryBuildingStringGroupByColumnSelectorStrategy.java index 503f80b6bd2e..db58c36e0579 100644 --- a/processing/src/main/java/io/druid/query/groupby/epinephelinae/column/DictionaryBuildingStringGroupByColumnSelectorStrategy.java +++ b/processing/src/main/java/io/druid/query/groupby/epinephelinae/column/DictionaryBuildingStringGroupByColumnSelectorStrategy.java @@ -68,21 +68,26 @@ public void initColumnValues(ColumnValueSelector selector, int columnIndex, Obje { final DimensionSelector dimSelector = (DimensionSelector) selector; final IndexedInts row = dimSelector.getRow(); - final int[] newIds = new int[row.size()]; - + ArrayBasedIndexedInts newRow = (ArrayBasedIndexedInts) valuess[columnIndex]; + if (newRow == null) { + newRow = new ArrayBasedIndexedInts(); + valuess[columnIndex] = newRow; + } + int rowSize = row.size(); + newRow.ensureSize(rowSize); for (int i = 0; i < row.size(); i++) { final String value = dimSelector.lookupName(row.get(i)); final int dictId = reverseDictionary.getInt(value); if (dictId < 0) { dictionary.add(value); reverseDictionary.put(value, nextId); - newIds[i] = nextId; + newRow.setValue(i, nextId); nextId++; } else { - newIds[i] = dictId; + newRow.setValue(i, dictId); } } - valuess[columnIndex] = ArrayBasedIndexedInts.of(newIds); + newRow.setSize(rowSize); } @Override diff --git a/processing/src/main/java/io/druid/segment/BaseObjectColumnValueSelector.java b/processing/src/main/java/io/druid/segment/BaseObjectColumnValueSelector.java index 619e9a23d2cc..4169e17e7cba 100644 --- a/processing/src/main/java/io/druid/segment/BaseObjectColumnValueSelector.java +++ b/processing/src/main/java/io/druid/segment/BaseObjectColumnValueSelector.java @@ -33,6 +33,17 @@ @PublicApi public interface BaseObjectColumnValueSelector { + /** + * Returns the column value at the current position. + * + * IMPORTANT. The returned object could generally be reused inside the implementation of + * BaseObjectColumnValueSelector, i. e. this method could always return the same object for the same selector. Users + * of this API, such as {@link io.druid.query.aggregation.Aggregator#aggregate()}, {@link + * io.druid.query.aggregation.BufferAggregator#aggregate}, {@link io.druid.query.aggregation.AggregateCombiner#reset}, + * {@link io.druid.query.aggregation.AggregateCombiner#fold} should be prepared for that and not storing the object + * returned from this method in their state, assuming that the object will remain unchanged even when the position of + * the selector changes. This may not be the case. + */ @Nullable T getObject(); diff --git a/processing/src/main/java/io/druid/segment/ColumnValueSelector.java b/processing/src/main/java/io/druid/segment/ColumnValueSelector.java index efb8c27caab8..c6a147d16c02 100644 --- a/processing/src/main/java/io/druid/segment/ColumnValueSelector.java +++ b/processing/src/main/java/io/druid/segment/ColumnValueSelector.java @@ -22,7 +22,8 @@ import io.druid.guice.annotations.PublicApi; /** - * Base type for interfaces that manage column value selection, e.g. DimensionSelector, LongColumnSelector + * Base type for interfaces that manage column value selection, e.g. {@link DimensionSelector}, {@link + * LongColumnSelector}. * * This interface has methods to get the value in all primitive types, that have corresponding basic aggregators in * Druid: Sum, Min, Max, etc: {@link #getFloat()}, {@link #getDouble()} and {@link #getLong()} to support "polymorphic" diff --git a/processing/src/main/java/io/druid/segment/CompressedVSizeIndexedSupplier.java b/processing/src/main/java/io/druid/segment/CompressedVSizeIndexedSupplier.java index 45043e18a47c..e3f75ff58ad8 100644 --- a/processing/src/main/java/io/druid/segment/CompressedVSizeIndexedSupplier.java +++ b/processing/src/main/java/io/druid/segment/CompressedVSizeIndexedSupplier.java @@ -30,6 +30,7 @@ import io.druid.segment.data.IndexedInts; import io.druid.segment.data.IndexedIterable; import io.druid.segment.data.IndexedMultivalue; +import io.druid.segment.data.SliceIndexedInts; import io.druid.segment.data.WritableSupplier; import it.unimi.dsi.fastutil.ints.IntArrayList; import it.unimi.dsi.fastutil.ints.IntList; @@ -47,7 +48,6 @@ * the last element in the offsets represents the total length of values column. * values - indexed integer representing values in each row */ - public class CompressedVSizeIndexedSupplier implements WritableSupplier> { private static final byte version = 0x2; @@ -154,11 +154,13 @@ public static class CompressedVSizeIndexed implements IndexedMultivalue= size) { - throw new IAE("Index[%d] >= size[%d]", index, size); - } - return values.get(index + offset); - } - - @Override - public void close() throws IOException - { - // no-op - } - - @Override - public void inspectRuntimeShape(RuntimeShapeInspector inspector) - { - inspector.visit("values", values); - } - }; + rowValues.setValues(offset, size); + return rowValues; } @Override diff --git a/processing/src/main/java/io/druid/segment/DimensionSelector.java b/processing/src/main/java/io/druid/segment/DimensionSelector.java index ae9c04f66f12..91e099070481 100644 --- a/processing/src/main/java/io/druid/segment/DimensionSelector.java +++ b/processing/src/main/java/io/druid/segment/DimensionSelector.java @@ -36,11 +36,15 @@ public interface DimensionSelector extends ColumnValueSelector, HotLoopCallee int CARDINALITY_UNKNOWN = -1; /** - * Gets all values for the row inside of an IntBuffer. I.e. one possible implementation could be + * Returns the indexed values at the current position in this DimensionSelector. * - * return IntBuffer.wrap(lookupExpansion(get()); - * - * @return all values for the row as an IntBuffer + * IMPORTANT. The returned {@link IndexedInts} object could generally be reused inside the implementation of + * DimensionSelector, i. e. this method could always return the same object for the same selector. Users + * of this API, such as {@link io.druid.query.aggregation.Aggregator#aggregate()}, {@link + * io.druid.query.aggregation.BufferAggregator#aggregate}, {@link io.druid.query.aggregation.AggregateCombiner#reset}, + * {@link io.druid.query.aggregation.AggregateCombiner#fold} should be prepared for that and not storing the object + * returned from this method in their state, assuming that the object will remain unchanged even when the position of + * the selector changes. This may not be the case. */ @CalledFromHotLoop IndexedInts getRow(); diff --git a/processing/src/main/java/io/druid/segment/SingleScanTimeDimSelector.java b/processing/src/main/java/io/druid/segment/SingleScanTimeDimSelector.java index 1d140304fcc0..c04fb0c8b5ce 100644 --- a/processing/src/main/java/io/druid/segment/SingleScanTimeDimSelector.java +++ b/processing/src/main/java/io/druid/segment/SingleScanTimeDimSelector.java @@ -38,6 +38,7 @@ public class SingleScanTimeDimSelector implements DimensionSelector private final boolean descending; private final List timeValues = new ArrayList<>(); + private final SingleIndexedInt row = new SingleIndexedInt(); private String currentValue = null; private long currentTimestamp = Long.MIN_VALUE; private int index = -1; @@ -61,7 +62,8 @@ public SingleScanTimeDimSelector(BaseLongColumnValueSelector selector, Extractio @Override public IndexedInts getRow() { - return SingleIndexedInt.of(getDimensionValueIndex()); + row.setValue(getDimensionValueIndex()); + return row; } @Override diff --git a/processing/src/main/java/io/druid/segment/StringDimensionIndexer.java b/processing/src/main/java/io/druid/segment/StringDimensionIndexer.java index 71e1c3443fe4..bbe89d603388 100644 --- a/processing/src/main/java/io/druid/segment/StringDimensionIndexer.java +++ b/processing/src/main/java/io/druid/segment/StringDimensionIndexer.java @@ -366,6 +366,7 @@ public DimensionSelector makeDimensionSelector( class IndexerDimensionSelector implements DimensionSelector, IdLookup { + private final ArrayBasedIndexedInts indexedInts = new ArrayBasedIndexedInts(); private int[] nullIdIntArray; @Override @@ -405,7 +406,8 @@ public IndexedInts getRow() rowSize = indices.length; } - return ArrayBasedIndexedInts.of(row, rowSize); + indexedInts.setValues(row, rowSize); + return indexedInts; } @Override diff --git a/processing/src/main/java/io/druid/segment/column/SimpleDictionaryEncodedColumn.java b/processing/src/main/java/io/druid/segment/column/SimpleDictionaryEncodedColumn.java index a2acc314ff14..1929c914b3ab 100644 --- a/processing/src/main/java/io/druid/segment/column/SimpleDictionaryEncodedColumn.java +++ b/processing/src/main/java/io/druid/segment/column/SimpleDictionaryEncodedColumn.java @@ -202,10 +202,13 @@ public void inspectRuntimeShape(RuntimeShapeInspector inspector) class SingleValueQueryableDimensionSelector extends QueryableDimensionSelector implements SingleValueHistoricalDimensionSelector { + private final SingleIndexedInt row = new SingleIndexedInt(); + @Override public IndexedInts getRow() { - return SingleIndexedInt.of(getRowValue()); + row.setValue(getRowValue()); + return row; } public int getRowValue() @@ -216,7 +219,8 @@ public int getRowValue() @Override public IndexedInts getRow(int offset) { - return SingleIndexedInt.of(getRowValue(offset)); + row.setValue(getRowValue(offset)); + return row; } @Override diff --git a/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java b/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java index 9caca269c29a..17e78525e15f 100644 --- a/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java @@ -23,42 +23,59 @@ import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; import it.unimi.dsi.fastutil.ints.IntArrays; -import java.io.IOException; - /** */ public final class ArrayBasedIndexedInts implements IndexedInts { - private static final ArrayBasedIndexedInts EMPTY = new ArrayBasedIndexedInts(IntArrays.EMPTY_ARRAY, 0); + private int[] expansion; + private int size; - public static ArrayBasedIndexedInts of(int[] expansion) + public ArrayBasedIndexedInts() { - if (expansion.length == 0) { - return EMPTY; - } - return new ArrayBasedIndexedInts(expansion, expansion.length); + expansion = IntArrays.EMPTY_ARRAY; + size = 0; + } + + public ArrayBasedIndexedInts(int[] expansion) + { + this.expansion = expansion; + this.size = expansion.length; } - public static ArrayBasedIndexedInts of(int[] expansion, int size) + public void ensureSize(int size) { - if (size == 0) { - return EMPTY; + if (expansion.length < size) { + expansion = new int[size]; } + } + + public void setSize(int size) + { if (size < 0 || size > expansion.length) { - throw new IAE("Size[%s] should be between 0 and %s", size, expansion.length); + throw new IAE("Size[%d] > expansion.length[%d] or < 0", size, expansion.length); } - return new ArrayBasedIndexedInts(expansion, size); + this.size = size; } - private final int[] expansion; - private final int size; - - private ArrayBasedIndexedInts(int[] expansion, int size) + /** + * Sets the values from the given array. The given values array is not reused and not prone to be muted later. + * Instead, the values from this array are copied into an array which is internal to ArrayBasedIndexedInts. + */ + public void setValues(int[] values, int size) { - this.expansion = expansion; + if (size < 0 || size > values.length) { + throw new IAE("Size[%d] should be between 0 and %d", size, values.length); + } + ensureSize(size); + System.arraycopy(values, 0, expansion, 0, size); this.size = size; } + public void setValue(int index, int value) + { + expansion[index] = value; + } + @Override public int size() { @@ -68,14 +85,14 @@ public int size() @Override public int get(int index) { - if (index >= size) { - throw new IndexOutOfBoundsException("index: " + index + ", size: " + size); + if (index < 0 || index >= size) { + throw new IAE("index[%d] >= size[%d] or < 0", index, size); } return expansion[index]; } @Override - public void close() throws IOException + public void close() { } diff --git a/processing/src/main/java/io/druid/segment/data/RangeIndexedInts.java b/processing/src/main/java/io/druid/segment/data/RangeIndexedInts.java index c02118afa807..e0398047b14b 100644 --- a/processing/src/main/java/io/druid/segment/data/RangeIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/RangeIndexedInts.java @@ -19,40 +19,28 @@ package io.druid.segment.data; -import com.google.common.base.Preconditions; +import io.druid.java.util.common.IAE; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; import java.io.IOException; /** - * An IndexedInts that always returns [0, 1, ..., N]. + * Reusable IndexedInts that returns sequences [0, 1, ..., N]. */ public class RangeIndexedInts implements IndexedInts { - private static final int CACHE_LIMIT = 8; - private static final RangeIndexedInts[] CACHE = new RangeIndexedInts[CACHE_LIMIT]; + private int size; - static { - for (int i = 0; i < CACHE_LIMIT; i++) { - CACHE[i] = new RangeIndexedInts(i); - } - } - - private final int size; - - private RangeIndexedInts(int size) + public RangeIndexedInts() { - this.size = size; } - public static RangeIndexedInts create(final int size) + public void setSize(int size) { - Preconditions.checkArgument(size >= 0, "size >= 0"); - if (size < CACHE_LIMIT) { - return CACHE[size]; - } else { - return new RangeIndexedInts(size); + if (size < 0) { + throw new IAE("Size[%d] must be non-negative", size); } + this.size = size; } @Override @@ -65,7 +53,7 @@ public int size() public int get(int index) { if (index < 0 || index >= size) { - throw new IndexOutOfBoundsException("index: " + index); + throw new IAE("index[%d] >= size[%d] or < 0", index, size); } return index; } diff --git a/processing/src/main/java/io/druid/segment/data/SingleIndexedInt.java b/processing/src/main/java/io/druid/segment/data/SingleIndexedInt.java index 3d3c11c8a9ec..fe8b21f2119a 100644 --- a/processing/src/main/java/io/druid/segment/data/SingleIndexedInt.java +++ b/processing/src/main/java/io/druid/segment/data/SingleIndexedInt.java @@ -19,35 +19,25 @@ package io.druid.segment.data; +import io.druid.java.util.common.IAE; import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; import java.io.IOException; +/** + * Reusable IndexedInts that represents a sequence of a solo value [X]. + */ public final class SingleIndexedInt implements IndexedInts { - private static final int CACHE_SIZE = 128; - private static final SingleIndexedInt[] CACHE = new SingleIndexedInt[CACHE_SIZE]; - - static { - for (int i = 0; i < CACHE_SIZE; i++) { - CACHE[i] = new SingleIndexedInt(i); - } - } + private int value; - private final int value; - - private SingleIndexedInt(int value) + public SingleIndexedInt() { - this.value = value; } - public static SingleIndexedInt of(int value) + public void setValue(int value) { - if (value >= 0 && value < CACHE_SIZE) { - return CACHE[value]; - } else { - return new SingleIndexedInt(value); - } + this.value = value; } @Override @@ -60,7 +50,7 @@ public int size() public int get(int i) { if (i != 0) { - throw new IllegalArgumentException(i + " != 0"); + throw new IAE("%d != 0", i); } return value; } diff --git a/processing/src/main/java/io/druid/segment/data/SliceIndexedInts.java b/processing/src/main/java/io/druid/segment/data/SliceIndexedInts.java new file mode 100644 index 000000000000..ac8c51f50531 --- /dev/null +++ b/processing/src/main/java/io/druid/segment/data/SliceIndexedInts.java @@ -0,0 +1,74 @@ +/* + * Licensed to Metamarkets Group Inc. (Metamarkets) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. Metamarkets licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package io.druid.segment.data; + +import io.druid.java.util.common.IAE; +import io.druid.query.monomorphicprocessing.RuntimeShapeInspector; + +/** + * Reusable IndexedInts, that could represent a sub-sequence ("slice") in a larger IndexedInts object. Used in + * {@link io.druid.segment.CompressedVSizeIndexedSupplier} implementation. + * + * Unsafe for concurrent use from multiple threads. + */ +public final class SliceIndexedInts implements IndexedInts +{ + private final IndexedInts base; + private int offset = -1; + private int size = -1; + + public SliceIndexedInts(IndexedInts base) + { + this.base = base; + } + + public void setValues(int offset, int size) + { + this.offset = offset; + this.size = size; + } + + @Override + public int size() + { + return size; + } + + @Override + public int get(int index) + { + if (index < 0 || index >= size) { + throw new IAE("Index[%d] >= size[%d] or < 0", index, size); + } + return base.get(offset + index); + } + + @Override + public void inspectRuntimeShape(RuntimeShapeInspector inspector) + { + inspector.visit("base", base); + } + + @Override + public void close() + { + // Do nothing + } +} diff --git a/processing/src/main/java/io/druid/segment/virtual/SingleStringInputDimensionSelector.java b/processing/src/main/java/io/druid/segment/virtual/SingleStringInputDimensionSelector.java index 360f9f2e3ee5..8f070435cac5 100644 --- a/processing/src/main/java/io/druid/segment/virtual/SingleStringInputDimensionSelector.java +++ b/processing/src/main/java/io/druid/segment/virtual/SingleStringInputDimensionSelector.java @@ -31,6 +31,7 @@ import io.druid.segment.IdLookup; import io.druid.segment.data.IndexedInts; import io.druid.segment.data.SingleIndexedInt; +import io.druid.segment.data.ZeroIndexedInts; import javax.annotation.Nullable; @@ -42,6 +43,7 @@ public class SingleStringInputDimensionSelector implements DimensionSelector private final DimensionSelector selector; private final Expr expression; private final SingleInputBindings bindings = new SingleInputBindings(); + private final SingleIndexedInt nullAdjustedRow = new SingleIndexedInt(); /** * 0 if selector has null as a value; 1 if it doesn't. @@ -90,12 +92,13 @@ public IndexedInts getRow() if (nullAdjustment == 0) { return row; } else { - return SingleIndexedInt.of(row.get(0) + nullAdjustment); + nullAdjustedRow.setValue(row.get(0) + nullAdjustment); + return nullAdjustedRow; } } else { // Can't handle non-singly-valued rows in expressions. // Treat them as nulls until we think of something better to do. - return SingleIndexedInt.of(0); + return ZeroIndexedInts.instance(); } } diff --git a/processing/src/test/java/io/druid/query/aggregation/FilteredAggregatorTest.java b/processing/src/test/java/io/druid/query/aggregation/FilteredAggregatorTest.java index 41f49e5909d2..584f08a2b810 100644 --- a/processing/src/test/java/io/druid/query/aggregation/FilteredAggregatorTest.java +++ b/processing/src/test/java/io/druid/query/aggregation/FilteredAggregatorTest.java @@ -100,11 +100,13 @@ public DimensionSelector makeDimensionSelector(DimensionSpec dimensionSpec) @Override public IndexedInts getRow() { + SingleIndexedInt row = new SingleIndexedInt(); if (selector.getIndex() % 3 == 2) { - return SingleIndexedInt.of(1); + row.setValue(1); } else { - return SingleIndexedInt.of(0); + row.setValue(0); } + return row; } @Override diff --git a/processing/src/test/java/io/druid/query/dimension/TestDimensionSelector.java b/processing/src/test/java/io/druid/query/dimension/TestDimensionSelector.java index 1613b005a858..6ef0b371b37b 100644 --- a/processing/src/test/java/io/druid/query/dimension/TestDimensionSelector.java +++ b/processing/src/test/java/io/druid/query/dimension/TestDimensionSelector.java @@ -47,7 +47,7 @@ private TestDimensionSelector() @Override public IndexedInts getRow() { - return ArrayBasedIndexedInts.of(new int[]{2, 4, 6}); + return new ArrayBasedIndexedInts(new int[]{2, 4, 6}); } @Override diff --git a/processing/src/test/java/io/druid/segment/data/CompressedVSizeIndexedV3WriterTest.java b/processing/src/test/java/io/druid/segment/data/CompressedVSizeIndexedV3WriterTest.java index b3f86aef9211..925c0072897b 100644 --- a/processing/src/test/java/io/druid/segment/data/CompressedVSizeIndexedV3WriterTest.java +++ b/processing/src/test/java/io/druid/segment/data/CompressedVSizeIndexedV3WriterTest.java @@ -126,7 +126,7 @@ private void checkSerializedSizeAndData(int offsetChunkFactor, int valueChunkFac ); CompressedVSizeIndexedV3Writer writer = new CompressedVSizeIndexedV3Writer(offsetWriter, valueWriter); CompressedVSizeIndexedV3Supplier supplierFromIterable = CompressedVSizeIndexedV3Supplier.fromIterable( - Iterables.transform(vals, ArrayBasedIndexedInts::of), + Iterables.transform(vals, ArrayBasedIndexedInts::new), offsetChunkFactor, maxValue, byteOrder, diff --git a/processing/src/test/java/io/druid/segment/data/IndexedIntsTest.java b/processing/src/test/java/io/druid/segment/data/IndexedIntsTest.java index ef284067997b..fff80d0ed431 100644 --- a/processing/src/test/java/io/druid/segment/data/IndexedIntsTest.java +++ b/processing/src/test/java/io/druid/segment/data/IndexedIntsTest.java @@ -43,7 +43,7 @@ public static Collection constructorFeeder() throws IOException return Arrays.asList( new Object[][]{ {VSizeIndexedInts.fromArray(array)}, - {ArrayBasedIndexedInts.of(array)} + {new ArrayBasedIndexedInts(array)} } ); } From bf2a09d5bde661f356176d389d56628aeec9e446 Mon Sep 17 00:00:00 2001 From: leventov Date: Tue, 9 Jan 2018 11:06:43 +0100 Subject: [PATCH 2/3] Remove BaseObjectColumnValueSelector.getObject() doc --- .../druid/segment/BaseObjectColumnValueSelector.java | 11 ----------- 1 file changed, 11 deletions(-) diff --git a/processing/src/main/java/io/druid/segment/BaseObjectColumnValueSelector.java b/processing/src/main/java/io/druid/segment/BaseObjectColumnValueSelector.java index 4169e17e7cba..619e9a23d2cc 100644 --- a/processing/src/main/java/io/druid/segment/BaseObjectColumnValueSelector.java +++ b/processing/src/main/java/io/druid/segment/BaseObjectColumnValueSelector.java @@ -33,17 +33,6 @@ @PublicApi public interface BaseObjectColumnValueSelector { - /** - * Returns the column value at the current position. - * - * IMPORTANT. The returned object could generally be reused inside the implementation of - * BaseObjectColumnValueSelector, i. e. this method could always return the same object for the same selector. Users - * of this API, such as {@link io.druid.query.aggregation.Aggregator#aggregate()}, {@link - * io.druid.query.aggregation.BufferAggregator#aggregate}, {@link io.druid.query.aggregation.AggregateCombiner#reset}, - * {@link io.druid.query.aggregation.AggregateCombiner#fold} should be prepared for that and not storing the object - * returned from this method in their state, assuming that the object will remain unchanged even when the position of - * the selector changes. This may not be the case. - */ @Nullable T getObject(); From 5e557c3d1e20054695a105b0b2ef85fd4cfa40ed Mon Sep 17 00:00:00 2001 From: leventov Date: Wed, 10 Jan 2018 17:21:35 +0100 Subject: [PATCH 3/3] typo --- .../main/java/io/druid/segment/data/ArrayBasedIndexedInts.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java b/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java index c317fc0b890f..608d6f968a71 100644 --- a/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java +++ b/processing/src/main/java/io/druid/segment/data/ArrayBasedIndexedInts.java @@ -58,7 +58,7 @@ public void setSize(int size) } /** - * Sets the values from the given array. The given values array is not reused and not prone to be muted later. + * Sets the values from the given array. The given values array is not reused and not prone to be mutated later. * Instead, the values from this array are copied into an array which is internal to ArrayBasedIndexedInts. */ public void setValues(int[] values, int size)