From 21a35071ac44373d00a30f73ba6afd1cd4ad12a3 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Mon, 7 Mar 2022 13:37:01 -0800 Subject: [PATCH 01/11] push value range and set index get operations into BitmapIndex --- .../ExpressionVectorSelectorBenchmark.java | 14 +- .../benchmark/MockBitmapIndexSelector.java | 11 +- .../query/filter/BitmapIndexSelector.java | 6 +- .../druid/query/filter/InDimFilter.java | 29 +-- .../druid/query/metadata/SegmentAnalyzer.java | 2 +- .../apache/druid/segment/ColumnSelector.java | 14 +- .../ColumnSelectorBitmapIndexSelector.java | 92 ++++++++- .../QueryableIndexColumnSelectorFactory.java | 7 +- .../QueryableIndexIndexableAdapter.java | 2 +- .../segment/QueryableIndexStorageAdapter.java | 17 +- .../apache/druid/segment/VirtualColumns.java | 6 +- .../druid/segment/column/BitmapIndex.java | 24 +++ .../druid/segment/data/GenericIndexed.java | 2 +- .../apache/druid/segment/data/Indexed.java | 2 +- .../druid/segment/filter/BoundFilter.java | 66 ++----- .../segment/filter/ExpressionFilter.java | 4 +- .../druid/segment/filter/LikeFilter.java | 77 +------- .../StringBitmapIndexColumnPartSupplier.java | 180 ++++++++++++++++++ ...yableIndexVectorColumnSelectorFactory.java | 8 +- .../segment/virtual/ExpressionSelectors.java | 7 +- .../virtual/ListFilteredVirtualColumn.java | 166 +++++++++++++++- ...ColumnSelectorBitmapIndexSelectorTest.java | 1 + .../segment/IndexMergerNullHandlingTest.java | 2 +- .../druid/segment/filter/BaseFilterTest.java | 8 +- .../druid/segment/filter/BoundFilterTest.java | 39 ++++ .../filter/ExtractionDimFilterTest.java | 9 + .../druid/segment/filter/FiltersTest.java | 38 ++++ .../segment/filter/SelectorFilterTest.java | 14 ++ .../virtual/ExpressionSelectorsTest.java | 31 ++- .../ExpressionVectorSelectorsTest.java | 13 +- 30 files changed, 653 insertions(+), 238 deletions(-) diff --git a/benchmarks/src/test/java/org/apache/druid/benchmark/ExpressionVectorSelectorBenchmark.java b/benchmarks/src/test/java/org/apache/druid/benchmark/ExpressionVectorSelectorBenchmark.java index a0e06d019fd5..a262aea7edaa 100644 --- a/benchmarks/src/test/java/org/apache/druid/benchmark/ExpressionVectorSelectorBenchmark.java +++ b/benchmarks/src/test/java/org/apache/druid/benchmark/ExpressionVectorSelectorBenchmark.java @@ -30,13 +30,11 @@ import org.apache.druid.math.expr.ExpressionType; import org.apache.druid.math.expr.Parser; import org.apache.druid.query.expression.TestExprMacroTable; -import org.apache.druid.segment.ColumnInspector; import org.apache.druid.segment.ColumnValueSelector; import org.apache.druid.segment.Cursor; import org.apache.druid.segment.QueryableIndex; import org.apache.druid.segment.QueryableIndexStorageAdapter; import org.apache.druid.segment.VirtualColumns; -import org.apache.druid.segment.column.ColumnCapabilities; import org.apache.druid.segment.generator.GeneratorBasicSchemas; import org.apache.druid.segment.generator.GeneratorSchemaInfo; import org.apache.druid.segment.generator.SegmentGenerator; @@ -126,17 +124,7 @@ public void setup() ); Expr parsed = Parser.parse(expression, ExprMacroTable.nil()); - outputType = parsed.getOutputType( - new ColumnInspector() - { - @Nullable - @Override - public ColumnCapabilities getColumnCapabilities(String column) - { - return QueryableIndexStorageAdapter.getColumnCapabilities(index, column); - } - } - ); + outputType = parsed.getOutputType(index); checkSanity(); } diff --git a/benchmarks/src/test/java/org/apache/druid/benchmark/MockBitmapIndexSelector.java b/benchmarks/src/test/java/org/apache/druid/benchmark/MockBitmapIndexSelector.java index 55934fa67ff9..5d6dd23228e4 100644 --- a/benchmarks/src/test/java/org/apache/druid/benchmark/MockBitmapIndexSelector.java +++ b/benchmarks/src/test/java/org/apache/druid/benchmark/MockBitmapIndexSelector.java @@ -28,6 +28,8 @@ import org.apache.druid.segment.data.CloseableIndexed; import org.apache.druid.segment.data.GenericIndexed; +import javax.annotation.Nullable; + public class MockBitmapIndexSelector implements BitmapIndexSelector { private final GenericIndexed dictionary; @@ -71,7 +73,7 @@ public BitmapFactory getBitmapFactory() @Override public ImmutableBitmap getBitmapIndex(String dimension, String value) { - return bitmapIndex.getBitmap(bitmapIndex.getIndex(value)); + return bitmapIndex.getBitmapForValue(value); } @Override @@ -85,4 +87,11 @@ public ImmutableRTree getSpatialIndex(String dimension) { throw new UnsupportedOperationException(); } + + @Nullable + @Override + public ColumnCapabilities getColumnCapabilities(String column) + { + return null; + } } diff --git a/processing/src/main/java/org/apache/druid/query/filter/BitmapIndexSelector.java b/processing/src/main/java/org/apache/druid/query/filter/BitmapIndexSelector.java index fd90e7412b68..6a55aa34efec 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/BitmapIndexSelector.java +++ b/processing/src/main/java/org/apache/druid/query/filter/BitmapIndexSelector.java @@ -23,6 +23,7 @@ import org.apache.druid.collections.bitmap.BitmapFactory; import org.apache.druid.collections.bitmap.ImmutableBitmap; import org.apache.druid.collections.spatial.ImmutableRTree; +import org.apache.druid.segment.ColumnInspector; import org.apache.druid.segment.column.BitmapIndex; import org.apache.druid.segment.column.ColumnCapabilities; import org.apache.druid.segment.data.CloseableIndexed; @@ -31,12 +32,15 @@ /** */ -public interface BitmapIndexSelector +public interface BitmapIndexSelector extends ColumnInspector { @MustBeClosed @Nullable CloseableIndexed getDimensionValues(String dimension); + + @Deprecated ColumnCapabilities.Capable hasMultipleValues(String dimension); + int getNumRows(); BitmapFactory getBitmapFactory(); @Nullable diff --git a/processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java b/processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java index 0cae9809a8c6..839a7dfe8cd6 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java +++ b/processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java @@ -38,8 +38,6 @@ import com.google.common.hash.Hasher; import com.google.common.hash.Hashing; import it.unimi.dsi.fastutil.ints.IntArrayList; -import it.unimi.dsi.fastutil.ints.IntIterable; -import it.unimi.dsi.fastutil.ints.IntIterator; import it.unimi.dsi.fastutil.ints.IntOpenHashSet; import it.unimi.dsi.fastutil.longs.LongArrayList; import it.unimi.dsi.fastutil.longs.LongOpenHashSet; @@ -60,7 +58,6 @@ import org.apache.druid.segment.ColumnSelector; import org.apache.druid.segment.ColumnSelectorFactory; import org.apache.druid.segment.DimensionHandlerUtils; -import org.apache.druid.segment.IntIteratorUtils; import org.apache.druid.segment.column.BitmapIndex; import org.apache.druid.segment.filter.Filters; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; @@ -71,7 +68,6 @@ import java.util.Collection; import java.util.Comparator; import java.util.HashSet; -import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; @@ -305,8 +301,7 @@ public double estimateSelectivity(BitmapIndexSelector indexSelector) if (extractionFn == null) { final BitmapIndex bitmapIndex = indexSelector.getBitmapIndex(dimension); return Filters.estimateSelectivity( - bitmapIndex, - IntIteratorUtils.toIntList(getBitmapIndexIterable(values, bitmapIndex).iterator()), + bitmapIndex.getBitmapsForValues(values).iterator(), indexSelector.getNumRows() ); } else { @@ -503,27 +498,7 @@ private static boolean isNaturalOrder(@Nullable final Comparator comparat private static Iterable getBitmapIterable(final Set values, final BitmapIndex bitmapIndex) { - return Filters.bitmapsFromIndexes(getBitmapIndexIterable(values, bitmapIndex), bitmapIndex); - } - - private static IntIterable getBitmapIndexIterable(final Set values, final BitmapIndex bitmapIndex) - { - return () -> new IntIterator() - { - final Iterator iterator = values.iterator(); - - @Override - public boolean hasNext() - { - return iterator.hasNext(); - } - - @Override - public int nextInt() - { - return bitmapIndex.getIndex(iterator.next()); - } - }; + return bitmapIndex.getBitmapsForValues(values); } @SuppressWarnings("ReturnValueIgnored") diff --git a/processing/src/main/java/org/apache/druid/query/metadata/SegmentAnalyzer.java b/processing/src/main/java/org/apache/druid/query/metadata/SegmentAnalyzer.java index 29b05f3e0e59..00f244d5ec7c 100644 --- a/processing/src/main/java/org/apache/druid/query/metadata/SegmentAnalyzer.java +++ b/processing/src/main/java/org/apache/druid/query/metadata/SegmentAnalyzer.java @@ -209,7 +209,7 @@ private ColumnAnalysis analyzeStringColumn( String value = bitmapIndex.getValue(i); if (value != null) { size += StringUtils.estimatedBinaryLengthAsUTF8(value) * - ((long) bitmapIndex.getBitmap(bitmapIndex.getIndex(value)).size()); + ((long) bitmapIndex.getBitmapForValue(value).size()); } } } diff --git a/processing/src/main/java/org/apache/druid/segment/ColumnSelector.java b/processing/src/main/java/org/apache/druid/segment/ColumnSelector.java index d73b2dacc48d..38bfc0da60f8 100644 --- a/processing/src/main/java/org/apache/druid/segment/ColumnSelector.java +++ b/processing/src/main/java/org/apache/druid/segment/ColumnSelector.java @@ -19,6 +19,7 @@ package org.apache.druid.segment; +import org.apache.druid.segment.column.ColumnCapabilities; import org.apache.druid.segment.column.ColumnHolder; import javax.annotation.Nullable; @@ -26,10 +27,21 @@ /** */ -public interface ColumnSelector +public interface ColumnSelector extends ColumnInspector { List getColumnNames(); @Nullable ColumnHolder getColumnHolder(String columnName); + + @Nullable + @Override + default ColumnCapabilities getColumnCapabilities(String column) + { + final ColumnHolder columnHolder = getColumnHolder(column); + if (columnHolder == null) { + return null; + } + return columnHolder.getCapabilities(); + } } diff --git a/processing/src/main/java/org/apache/druid/segment/ColumnSelectorBitmapIndexSelector.java b/processing/src/main/java/org/apache/druid/segment/ColumnSelectorBitmapIndexSelector.java index fb7a2baf7633..29e988da661c 100644 --- a/processing/src/main/java/org/apache/druid/segment/ColumnSelectorBitmapIndexSelector.java +++ b/processing/src/main/java/org/apache/druid/segment/ColumnSelectorBitmapIndexSelector.java @@ -19,6 +19,8 @@ package org.apache.druid.segment; +import com.google.common.collect.ImmutableList; +import it.unimi.dsi.fastutil.ints.IntIterator; import org.apache.druid.collections.bitmap.BitmapFactory; import org.apache.druid.collections.bitmap.ImmutableBitmap; import org.apache.druid.collections.spatial.ImmutableRTree; @@ -39,6 +41,8 @@ import javax.annotation.Nullable; import java.io.IOException; import java.util.Iterator; +import java.util.Set; +import java.util.function.IntPredicate; /** */ @@ -171,10 +175,7 @@ public ColumnCapabilities.Capable hasMultipleValues(final String dimension) VirtualColumn virtualColumn = virtualColumns.getVirtualColumn(dimension); ColumnCapabilities virtualCapabilities = null; if (virtualColumn != null) { - virtualCapabilities = virtualColumn.capabilities( - QueryableIndexStorageAdapter.getColumnInspectorForIndex(index), - dimension - ); + virtualCapabilities = virtualColumn.capabilities(index, dimension); } return virtualCapabilities != null ? virtualCapabilities.hasMultipleValues() : ColumnCapabilities.Capable.FALSE; } @@ -264,6 +265,78 @@ public ImmutableBitmap getBitmap(int idx) return bitmapFactory.makeEmptyImmutableBitmap(); } } + + @Override + public ImmutableBitmap getBitmapForValue(@Nullable String value) + { + if (NullHandling.isNullOrEquivalent(value)) { + return bitmapFactory.complement(bitmapFactory.makeEmptyImmutableBitmap(), getNumRows()); + } else { + return bitmapFactory.makeEmptyImmutableBitmap(); + } + } + + @Override + public Iterable getBitmapsInRange( + @Nullable String startValue, + boolean startStrict, + @Nullable String endValue, + boolean endStrict, + IntPredicate matcher + ) + { + final int startIndex; // inclusive + int endIndex; // exclusive + + if (startValue == null) { + startIndex = 0; + } else { + if (NullHandling.isNullOrEquivalent(startValue)) { + startIndex = startStrict ? 1 : 0; + } else { + startIndex = 1; + } + } + + if (endValue == null) { + endIndex = 1; + } else { + if (NullHandling.isNullOrEquivalent(endValue)) { + endIndex = endStrict ? 0 : 1; + } else { + endIndex = 1; + } + } + + endIndex = startIndex > endIndex || !matcher.test(endIndex) ? startIndex : endIndex; + if (matcher.test(startIndex)) { + final IntIterator rangeIterator = IntListUtils.fromTo(startIndex, endIndex).iterator(); + return () -> new Iterator() + { + @Override + public boolean hasNext() + { + return rangeIterator.hasNext(); + } + + @Override + public ImmutableBitmap next() + { + return getBitmap(rangeIterator.nextInt()); + } + }; + } + return ImmutableList.of(bitmapFactory.makeEmptyImmutableBitmap()); + } + + @Override + public Iterable getBitmapsForValues(Set values) + { + if (values.contains(null) || (NullHandling.replaceWithDefault() && values.contains(""))) { + return ImmutableList.of(getBitmap(0)); + } + return ImmutableList.of(bitmapFactory.makeEmptyImmutableBitmap()); + } }; } else if (columnHolder.getCapabilities().hasBitmapIndexes() && columnHolder.getCapabilities().is(ValueType.STRING)) { // currently BitmapIndex are reliant on STRING dictionaries to operate correctly, and will fail when used with @@ -282,7 +355,7 @@ public ImmutableBitmap getBitmapIndex(String dimension, String value) if (idx == null) { return null; } - return idx.getBitmap(idx.getIndex(value)); + return idx.getBitmapForValue(value); } final ColumnHolder columnHolder = index.getColumnHolder(dimension); @@ -301,7 +374,7 @@ public ImmutableBitmap getBitmapIndex(String dimension, String value) } final BitmapIndex bitmapIndex = columnHolder.getBitmapIndex(); - return bitmapIndex.getBitmap(bitmapIndex.getIndex(value)); + return bitmapIndex.getBitmapForValue(value); } @Override @@ -323,4 +396,11 @@ private boolean isVirtualColumn(final String columnName) { return virtualColumns.getVirtualColumn(columnName) != null; } + + @Nullable + @Override + public ColumnCapabilities getColumnCapabilities(String column) + { + return virtualColumns.getColumnCapabilities(index, column); + } } diff --git a/processing/src/main/java/org/apache/druid/segment/QueryableIndexColumnSelectorFactory.java b/processing/src/main/java/org/apache/druid/segment/QueryableIndexColumnSelectorFactory.java index 28c8b6a0c8fb..72859024ec6e 100644 --- a/processing/src/main/java/org/apache/druid/segment/QueryableIndexColumnSelectorFactory.java +++ b/processing/src/main/java/org/apache/druid/segment/QueryableIndexColumnSelectorFactory.java @@ -198,12 +198,9 @@ private T getCachedColumn(final String columnName, final public ColumnCapabilities getColumnCapabilities(String columnName) { if (virtualColumns.exists(columnName)) { - return virtualColumns.getColumnCapabilities( - QueryableIndexStorageAdapter.getColumnInspectorForIndex(index), - columnName - ); + return virtualColumns.getColumnCapabilities(index, columnName); } - return QueryableIndexStorageAdapter.getColumnCapabilities(index, columnName); + return index.getColumnCapabilities(columnName); } } diff --git a/processing/src/main/java/org/apache/druid/segment/QueryableIndexIndexableAdapter.java b/processing/src/main/java/org/apache/druid/segment/QueryableIndexIndexableAdapter.java index 801e19a0d092..b6b6cfd49510 100644 --- a/processing/src/main/java/org/apache/druid/segment/QueryableIndexIndexableAdapter.java +++ b/processing/src/main/java/org/apache/druid/segment/QueryableIndexIndexableAdapter.java @@ -397,7 +397,7 @@ BitmapValues getBitmapIndex(String dimension, String value) return BitmapValues.EMPTY; } - return new ImmutableBitmapValues(bitmaps.getBitmap(bitmaps.getIndex(value))); + return new ImmutableBitmapValues(bitmaps.getBitmapForValue(value)); } @Override diff --git a/processing/src/main/java/org/apache/druid/segment/QueryableIndexStorageAdapter.java b/processing/src/main/java/org/apache/druid/segment/QueryableIndexStorageAdapter.java index cb070e1e4927..5025ef693b04 100644 --- a/processing/src/main/java/org/apache/druid/segment/QueryableIndexStorageAdapter.java +++ b/processing/src/main/java/org/apache/druid/segment/QueryableIndexStorageAdapter.java @@ -168,7 +168,7 @@ public Comparable getMaxValue(String dimension) @Nullable public ColumnCapabilities getColumnCapabilities(String column) { - return getColumnCapabilities(index, column); + return index.getColumnCapabilities(column); } @Override @@ -280,21 +280,6 @@ public Sequence makeCursors( ); } - @Nullable - public static ColumnCapabilities getColumnCapabilities(ColumnSelector index, String columnName) - { - final ColumnHolder columnHolder = index.getColumnHolder(columnName); - if (columnHolder == null) { - return null; - } - return columnHolder.getCapabilities(); - } - - public static ColumnInspector getColumnInspectorForIndex(ColumnSelector index) - { - return column -> getColumnCapabilities(index, column); - } - @Override public Metadata getMetadata() { diff --git a/processing/src/main/java/org/apache/druid/segment/VirtualColumns.java b/processing/src/main/java/org/apache/druid/segment/VirtualColumns.java index 5a0b0d939844..dc6441c3375a 100644 --- a/processing/src/main/java/org/apache/druid/segment/VirtualColumns.java +++ b/processing/src/main/java/org/apache/druid/segment/VirtualColumns.java @@ -387,11 +387,7 @@ public ColumnCapabilities getColumnCapabilities(ColumnInspector inspector, Strin { final VirtualColumn virtualColumn = getVirtualColumn(columnName); if (virtualColumn != null) { - return Preconditions.checkNotNull( - virtualColumn.capabilities(column -> getColumnCapabilitiesWithFallback(inspector, column), columnName), - "capabilities for column[%s]", - columnName - ); + return virtualColumn.capabilities(column -> getColumnCapabilitiesWithFallback(inspector, column), columnName); } else { return null; } diff --git a/processing/src/main/java/org/apache/druid/segment/column/BitmapIndex.java b/processing/src/main/java/org/apache/druid/segment/column/BitmapIndex.java index 0d6821605606..b08916146337 100644 --- a/processing/src/main/java/org/apache/druid/segment/column/BitmapIndex.java +++ b/processing/src/main/java/org/apache/druid/segment/column/BitmapIndex.java @@ -23,6 +23,8 @@ import org.apache.druid.collections.bitmap.ImmutableBitmap; import javax.annotation.Nullable; +import java.util.Set; +import java.util.function.IntPredicate; /** */ @@ -47,4 +49,26 @@ public interface BitmapIndex int getIndex(@Nullable String value); ImmutableBitmap getBitmap(int idx); + + ImmutableBitmap getBitmapForValue(@Nullable String value); + + default Iterable getBitmapsInRange( + @Nullable String startValue, + boolean startStrict, + @Nullable String endValue, + boolean endStrict + ) + { + return getBitmapsInRange(startValue, startStrict, endValue, endStrict, (index) -> true); + } + + Iterable getBitmapsInRange( + @Nullable String startValue, + boolean startStrict, + @Nullable String endValue, + boolean endStrict, + IntPredicate matcher + ); + + Iterable getBitmapsForValues(Set values); } diff --git a/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java b/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java index be1ed1cb4055..2c6ea106157b 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java +++ b/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java @@ -355,7 +355,7 @@ private int indexOf(Indexed indexed, @Nullable T value) if (!allowReverseLookup) { throw new UnsupportedOperationException("Reverse lookup not allowed."); } - return Indexed.indexOf(indexed::get, size, strategy, value); + return Indexed.indexOf(indexed, size, strategy, value); } @Override diff --git a/processing/src/main/java/org/apache/druid/segment/data/Indexed.java b/processing/src/main/java/org/apache/druid/segment/data/Indexed.java index aae7c3777612..e4b5b772b955 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/Indexed.java +++ b/processing/src/main/java/org/apache/druid/segment/data/Indexed.java @@ -65,7 +65,7 @@ public interface Indexed extends Iterable, HotLoopCallee * * @return index of value, or negative number equal to (-(insertion point) - 1). */ - static int indexOf(IndexedGetter indexed, int size, Comparator comparator, @Nullable T value) + static int indexOf(Indexed indexed, int size, Comparator comparator, @Nullable T value) { int minIndex = 0; int maxIndex = size - 1; diff --git a/processing/src/main/java/org/apache/druid/segment/filter/BoundFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/BoundFilter.java index 73c10e71a18e..115b8d36d4b2 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/BoundFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/BoundFilter.java @@ -22,11 +22,9 @@ import com.google.common.annotations.VisibleForTesting; import com.google.common.base.Predicate; import com.google.common.base.Supplier; -import it.unimi.dsi.fastutil.ints.IntList; import org.apache.druid.collections.bitmap.ImmutableBitmap; import org.apache.druid.common.config.NullHandling; import org.apache.druid.java.util.common.IAE; -import org.apache.druid.java.util.common.Pair; import org.apache.druid.query.BitmapResultFactory; import org.apache.druid.query.extraction.ExtractionFn; import org.apache.druid.query.filter.BitmapIndexSelector; @@ -45,7 +43,6 @@ import org.apache.druid.segment.ColumnProcessors; import org.apache.druid.segment.ColumnSelector; import org.apache.druid.segment.ColumnSelectorFactory; -import org.apache.druid.segment.IntListUtils; import org.apache.druid.segment.column.BitmapIndex; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; @@ -102,8 +99,12 @@ public double estimateSelectivity(BitmapIndexSelector indexSelector) } return Filters.estimateSelectivity( - bitmapIndex, - getBitmapIndexList(boundDimFilter, bitmapIndex), + bitmapIndex.getBitmapsInRange( + boundDimFilter.getLower(), + boundDimFilter.isLowerStrict(), + boundDimFilter.getUpper(), + boundDimFilter.isUpperStrict() + ).iterator(), indexSelector.getNumRows() ); } else { @@ -200,60 +201,17 @@ public Filter rewriteRequiredColumns(Map columnRewrites) ); } - private static Pair getStartEndIndexes( - final BoundDimFilter boundDimFilter, - final BitmapIndex bitmapIndex - ) - { - final int startIndex; // inclusive - int endIndex; // exclusive - - if (!boundDimFilter.hasLowerBound()) { - startIndex = 0; - } else { - final int found = bitmapIndex.getIndex(NullHandling.emptyToNullIfNeeded(boundDimFilter.getLower())); - if (found >= 0) { - startIndex = boundDimFilter.isLowerStrict() ? found + 1 : found; - } else { - startIndex = -(found + 1); - } - } - - if (!boundDimFilter.hasUpperBound()) { - endIndex = bitmapIndex.getCardinality(); - } else { - final int found = bitmapIndex.getIndex(NullHandling.emptyToNullIfNeeded(boundDimFilter.getUpper())); - if (found >= 0) { - endIndex = boundDimFilter.isUpperStrict() ? found : found + 1; - } else { - endIndex = -(found + 1); - } - } - - endIndex = startIndex > endIndex ? startIndex : endIndex; - - return new Pair<>(startIndex, endIndex); - } - private static Iterable getBitmapIterator( final BoundDimFilter boundDimFilter, final BitmapIndex bitmapIndex ) { - return Filters.bitmapsFromIndexes(getBitmapIndexList(boundDimFilter, bitmapIndex), bitmapIndex); - } - - private static IntList getBitmapIndexList( - final BoundDimFilter boundDimFilter, - final BitmapIndex bitmapIndex - ) - { - // search for start, end indexes in the bitmaps; then include all bitmaps between those points - final Pair indexes = getStartEndIndexes(boundDimFilter, bitmapIndex); - final int startIndex = indexes.lhs; - final int endIndex = indexes.rhs; - - return IntListUtils.fromTo(startIndex, endIndex); + return bitmapIndex.getBitmapsInRange( + boundDimFilter.getLower(), + boundDimFilter.isLowerStrict(), + boundDimFilter.getUpper(), + boundDimFilter.isUpperStrict() + ); } private DruidPredicateFactory getPredicateFactory() diff --git a/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java index 4ec947a23d51..fe45a86425bd 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java @@ -47,6 +47,7 @@ import org.apache.druid.segment.ColumnSelector; import org.apache.druid.segment.ColumnSelectorFactory; import org.apache.druid.segment.ColumnValueSelector; +import org.apache.druid.segment.column.ColumnCapabilities; import org.apache.druid.segment.column.ColumnCapabilitiesImpl; import org.apache.druid.segment.column.ColumnType; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; @@ -229,8 +230,9 @@ public boolean supportsBitmapIndex(final BitmapIndexSelector selector) // Single-column expression. We can use bitmap indexes if this column has an index and the expression can // map over the values of the index. final String column = Iterables.getOnlyElement(details.getRequiredBindings()); + final ColumnCapabilities capabilities = selector.getColumnCapabilities(column); return selector.getBitmapIndex(column) != null - && ExpressionSelectors.canMapOverDictionary(details, selector.hasMultipleValues(column)); + && ExpressionSelectors.canMapOverDictionary(details, capabilities); } else { // Multi-column expression. return false; diff --git a/processing/src/main/java/org/apache/druid/segment/filter/LikeFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/LikeFilter.java index a0dac33a456e..3676a4b23b77 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/LikeFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/LikeFilter.java @@ -21,8 +21,6 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; -import it.unimi.dsi.fastutil.ints.IntIterable; -import it.unimi.dsi.fastutil.ints.IntIterator; import org.apache.druid.collections.bitmap.ImmutableBitmap; import org.apache.druid.common.config.NullHandling; import org.apache.druid.java.util.common.IAE; @@ -47,7 +45,6 @@ import java.io.IOException; import java.io.UncheckedIOException; import java.util.Map; -import java.util.NoSuchElementException; import java.util.Objects; import java.util.Set; @@ -180,7 +177,7 @@ private Iterable getBitmapIterable(final BitmapIndexSelector se // Union bitmaps for all matching dimension values in range. // Use lazy iterator to allow unioning bitmaps one by one and avoid materializing all of them at once. - return Filters.bitmapsFromIndexes(getDimValueIndexIterableForPrefixMatch(bitmapIndex, dimValues), bitmapIndex); + return getDimValueIndexIterableForPrefixMatch(bitmapIndex, dimValues); } catch (IOException e) { throw new UncheckedIOException(e); @@ -211,7 +208,7 @@ private boolean isSimplePrefix() return extractionFn == null && !likeMatcher.getPrefix().isEmpty(); } - private IntIterable getDimValueIndexIterableForPrefixMatch( + private Iterable getDimValueIndexIterableForPrefixMatch( final BitmapIndex bitmapIndex, final Indexed dimValues ) @@ -220,69 +217,13 @@ private IntIterable getDimValueIndexIterableForPrefixMatch( final String lower = NullHandling.nullToEmptyIfNeeded(likeMatcher.getPrefix()); final String upper = NullHandling.nullToEmptyIfNeeded(likeMatcher.getPrefix()) + Character.MAX_VALUE; - final int startIndex; // inclusive - final int endIndex; // exclusive - - if (lower == null) { - // For Null values - startIndex = bitmapIndex.getIndex(null); - endIndex = startIndex + 1; - } else { - final int lowerFound = bitmapIndex.getIndex(lower); - startIndex = lowerFound >= 0 ? lowerFound : -(lowerFound + 1); - - final int upperFound = bitmapIndex.getIndex(upper); - endIndex = upperFound >= 0 ? upperFound + 1 : -(upperFound + 1); - } - - return new IntIterable() - { - @Override - public IntIterator iterator() - { - return new IntIterator() - { - int currIndex = startIndex; - int found; - - { - found = findNext(); - } - - private int findNext() - { - while (currIndex < endIndex && !likeMatcher.matchesSuffixOnly(dimValues, currIndex)) { - currIndex++; - } - - if (currIndex < endIndex) { - return currIndex++; - } else { - return -1; - } - } - - @Override - public boolean hasNext() - { - return found != -1; - } - - @Override - public int nextInt() - { - int cur = found; - - if (cur == -1) { - throw new NoSuchElementException(); - } - - found = findNext(); - return cur; - } - }; - } - }; + return bitmapIndex.getBitmapsInRange( + lower, + false, + upper, + false, + (index) -> likeMatcher.matchesSuffixOnly(dimValues, index) + ); } @Override diff --git a/processing/src/main/java/org/apache/druid/segment/serde/StringBitmapIndexColumnPartSupplier.java b/processing/src/main/java/org/apache/druid/segment/serde/StringBitmapIndexColumnPartSupplier.java index 0a771f670006..286df7202dcf 100644 --- a/processing/src/main/java/org/apache/druid/segment/serde/StringBitmapIndexColumnPartSupplier.java +++ b/processing/src/main/java/org/apache/druid/segment/serde/StringBitmapIndexColumnPartSupplier.java @@ -20,12 +20,19 @@ package org.apache.druid.segment.serde; import com.google.common.base.Supplier; +import it.unimi.dsi.fastutil.ints.IntIterator; import org.apache.druid.collections.bitmap.BitmapFactory; import org.apache.druid.collections.bitmap.ImmutableBitmap; +import org.apache.druid.common.config.NullHandling; +import org.apache.druid.segment.IntListUtils; import org.apache.druid.segment.column.BitmapIndex; import org.apache.druid.segment.data.GenericIndexed; import javax.annotation.Nullable; +import java.util.Iterator; +import java.util.NoSuchElementException; +import java.util.Set; +import java.util.function.IntPredicate; /** * Provides {@link BitmapIndex} for some dictionary encoded column, where the dictionary and bitmaps are stored in some @@ -94,6 +101,179 @@ public ImmutableBitmap getBitmap(int idx) final ImmutableBitmap bitmap = bitmaps.get(idx); return bitmap == null ? bitmapFactory.makeEmptyImmutableBitmap() : bitmap; } + + @Override + public ImmutableBitmap getBitmapForValue(@Nullable String value) + { + final int idx = dictionary.indexOf(value); + return getBitmap(idx); + } + + @Override + public Iterable getBitmapsInRange( + @Nullable String startValue, + boolean startStrict, + @Nullable String endValue, + boolean endStrict + ) + { + int startIndex, endIndex; + if (startValue == null) { + startIndex = 0; + } else { + final int found = dictionary.indexOf(NullHandling.emptyToNullIfNeeded(startValue)); + if (found >= 0) { + startIndex = startStrict ? found + 1 : found; + } else { + startIndex = -(found + 1); + } + } + + if (endValue == null) { + endIndex = dictionary.size(); + } else { + final int found = dictionary.indexOf(NullHandling.emptyToNullIfNeeded(endValue)); + if (found >= 0) { + endIndex = endStrict ? found : found + 1; + } else { + endIndex = -(found + 1); + } + } + + endIndex = startIndex > endIndex ? startIndex : endIndex; + final IntIterator rangeIterator = IntListUtils.fromTo(startIndex, endIndex).iterator(); + return () -> new Iterator() + { + @Override + public boolean hasNext() + { + return rangeIterator.hasNext(); + } + + @Override + public ImmutableBitmap next() + { + return getBitmap(rangeIterator.nextInt()); + } + }; + } + + @Override + public Iterable getBitmapsInRange( + @Nullable String startValue, + boolean startStrict, + @Nullable String endValue, + boolean endStrict, + IntPredicate indexMatcher + ) + { + int startIndex, endIndex; + if (startValue == null) { + startIndex = 0; + } else { + final int found = dictionary.indexOf(NullHandling.emptyToNullIfNeeded(startValue)); + if (found >= 0) { + startIndex = startStrict ? found + 1 : found; + } else { + startIndex = -(found + 1); + } + } + + if (endValue == null) { + endIndex = dictionary.size(); + } else { + final int found = dictionary.indexOf(NullHandling.emptyToNullIfNeeded(endValue)); + if (found >= 0) { + endIndex = endStrict ? found : found + 1; + } else { + endIndex = -(found + 1); + } + } + + endIndex = startIndex > endIndex ? startIndex : endIndex; + final int start = startIndex, end = endIndex; + return () -> new Iterator() + { + int currIndex = start; + int found; + { + found = findNext(); + } + + private int findNext() + { + while (currIndex < end && !indexMatcher.test(currIndex)) { + currIndex++; + } + + if (currIndex < end) { + return currIndex++; + } else { + return -1; + } + } + + @Override + public boolean hasNext() + { + return found != -1; + } + + @Override + public ImmutableBitmap next() + { + int cur = found; + + if (cur == -1) { + throw new NoSuchElementException(); + } + + found = findNext(); + return getBitmap(cur); + } + }; + } + + @Override + public Iterable getBitmapsForValues(Set values) + { + return () -> new Iterator() + { + final Iterator iterator = values.iterator(); + int next = -1; + + @Override + public boolean hasNext() + { + if (next < 0) { + findNext(); + } + return next >= 0; + } + + @Override + public ImmutableBitmap next() + { + if (next < 0) { + findNext(); + if (next < 0) { + throw new NoSuchElementException(); + } + } + final int swap = next; + next = -1; + return getBitmap(swap); + } + + private void findNext() + { + while (next < 0 && iterator.hasNext()) { + String nextValue = iterator.next(); + next = dictionary.indexOf(nextValue); + } + } + }; + } }; } } diff --git a/processing/src/main/java/org/apache/druid/segment/vector/QueryableIndexVectorColumnSelectorFactory.java b/processing/src/main/java/org/apache/druid/segment/vector/QueryableIndexVectorColumnSelectorFactory.java index af9e2832cb08..e300674cc5fa 100644 --- a/processing/src/main/java/org/apache/druid/segment/vector/QueryableIndexVectorColumnSelectorFactory.java +++ b/processing/src/main/java/org/apache/druid/segment/vector/QueryableIndexVectorColumnSelectorFactory.java @@ -23,7 +23,6 @@ import org.apache.druid.java.util.common.io.Closer; import org.apache.druid.query.dimension.DimensionSpec; import org.apache.druid.segment.QueryableIndex; -import org.apache.druid.segment.QueryableIndexStorageAdapter; import org.apache.druid.segment.VirtualColumns; import org.apache.druid.segment.column.BaseColumn; import org.apache.druid.segment.column.ColumnCapabilities; @@ -266,11 +265,8 @@ private BaseColumn getCachedColumn(final String columnName) public ColumnCapabilities getColumnCapabilities(final String columnName) { if (virtualColumns.exists(columnName)) { - return virtualColumns.getColumnCapabilities( - QueryableIndexStorageAdapter.getColumnInspectorForIndex(index), - columnName - ); + return virtualColumns.getColumnCapabilities(index, columnName); } - return QueryableIndexStorageAdapter.getColumnCapabilities(index, columnName); + return index.getColumnCapabilities(columnName); } } diff --git a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java index 670fcf41cf46..efcdc7fb66ed 100644 --- a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java +++ b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java @@ -237,11 +237,14 @@ public static DimensionSelector makeDimensionSelector( */ public static boolean canMapOverDictionary( final Expr.BindingAnalysis bindingAnalysis, - final ColumnCapabilities.Capable hasMultipleValues + final ColumnCapabilities columnCapabilities ) { Preconditions.checkState(bindingAnalysis.getRequiredBindings().size() == 1, "requiredBindings.size == 1"); - return !hasMultipleValues.isUnknown() && !bindingAnalysis.hasInputArrays() && !bindingAnalysis.isOutputArray(); + return columnCapabilities != null && + !columnCapabilities.hasMultipleValues().isUnknown() && + !bindingAnalysis.hasInputArrays() && + !bindingAnalysis.isOutputArray(); } /** diff --git a/processing/src/main/java/org/apache/druid/segment/virtual/ListFilteredVirtualColumn.java b/processing/src/main/java/org/apache/druid/segment/virtual/ListFilteredVirtualColumn.java index 91ffe4da0d1d..e87db1b5aac6 100644 --- a/processing/src/main/java/org/apache/druid/segment/virtual/ListFilteredVirtualColumn.java +++ b/processing/src/main/java/org/apache/druid/segment/virtual/ListFilteredVirtualColumn.java @@ -24,11 +24,12 @@ import com.google.common.base.Preconditions; import org.apache.druid.collections.bitmap.BitmapFactory; import org.apache.druid.collections.bitmap.ImmutableBitmap; -import org.apache.druid.java.util.common.guava.Comparators; +import org.apache.druid.common.config.NullHandling; import org.apache.druid.query.cache.CacheKeyBuilder; import org.apache.druid.query.dimension.DefaultDimensionSpec; import org.apache.druid.query.dimension.DimensionSpec; import org.apache.druid.query.dimension.ListFilteredDimensionSpec; +import org.apache.druid.query.ordering.StringComparators; import org.apache.druid.segment.ColumnInspector; import org.apache.druid.segment.ColumnSelector; import org.apache.druid.segment.ColumnSelectorFactory; @@ -40,13 +41,16 @@ import org.apache.druid.segment.column.ColumnCapabilities; import org.apache.druid.segment.column.ColumnCapabilitiesImpl; import org.apache.druid.segment.column.ColumnHolder; -import org.apache.druid.segment.data.Indexed; import javax.annotation.Nullable; import java.util.Collections; +import java.util.Comparator; +import java.util.Iterator; import java.util.List; +import java.util.NoSuchElementException; import java.util.Objects; import java.util.Set; +import java.util.function.IntPredicate; /** * {@link VirtualColumn} form of {@link ListFilteredDimensionSpec}, powered by @@ -252,6 +256,18 @@ public BitmapFactory getBitmapFactory() return delegate.getBitmapFactory(); } + @Override + public int getCardinality() + { + return idMapping.getValueCardinality(); + } + + @Override + public int getIndex(@Nullable String value) + { + return getReverseIndex(value); + } + @Override public ImmutableBitmap getBitmap(int idx) { @@ -259,15 +275,153 @@ public ImmutableBitmap getBitmap(int idx) } @Override - public int getCardinality() + public ImmutableBitmap getBitmapForValue(@Nullable String value) { - return idMapping.getValueCardinality(); + if (getReverseIndex(value) < 0) { + return delegate.getBitmap(-1); + } + return delegate.getBitmapForValue(value); } @Override - public int getIndex(@Nullable String value) + public Iterable getBitmapsInRange( + @Nullable String startValue, + boolean startStrict, + @Nullable String endValue, + boolean endStrict, + IntPredicate matcher + ) { - return Indexed.indexOf(this::getValue, getCardinality(), Comparators.naturalNullsFirst(), value); + int startIndex, endIndex; + if (startValue == null) { + startIndex = 0; + } else { + final int found = getReverseIndex(NullHandling.emptyToNullIfNeeded(startValue)); + if (found >= 0) { + startIndex = startStrict ? found + 1 : found; + } else { + startIndex = -(found + 1); + } + } + + if (endValue == null) { + endIndex = idMapping.getValueCardinality(); + } else { + final int found = getReverseIndex(NullHandling.emptyToNullIfNeeded(endValue)); + if (found >= 0) { + endIndex = endStrict ? found : found + 1; + } else { + endIndex = -(found + 1); + } + } + + endIndex = startIndex > endIndex ? startIndex : endIndex; + final int start = startIndex, end = endIndex; + return () -> new Iterator() + { + int currIndex = start; + int found; + { + found = findNext(); + } + + private int findNext() + { + while (currIndex < end && !matcher.test(currIndex)) { + currIndex++; + } + + if (currIndex < end) { + return currIndex++; + } else { + return -1; + } + } + + @Override + public boolean hasNext() + { + return found != -1; + } + + @Override + public ImmutableBitmap next() + { + int cur = found; + + if (cur == -1) { + throw new NoSuchElementException(); + } + + found = findNext(); + return getBitmap(cur); + } + }; + } + + @Override + public Iterable getBitmapsForValues(Set values) + { + return () -> new Iterator() + { + final Iterator iterator = values.iterator(); + int next = -1; + + @Override + public boolean hasNext() + { + if (next < 0) { + findNext(); + } + return next >= 0; + } + + @Override + public ImmutableBitmap next() + { + if (next < 0) { + findNext(); + if (next < 0) { + throw new NoSuchElementException(); + } + } + final int swap = next; + next = -1; + return getBitmap(swap); + } + + private void findNext() + { + while (next < 0 && iterator.hasNext()) { + String nextValue = iterator.next(); + next = getReverseIndex(nextValue); + } + } + }; + } + + private int getReverseIndex(@Nullable String value) + { + int minIndex = 0; + int maxIndex = idMapping.getValueCardinality() - 1; + final Comparator comparator = StringComparators.LEXICOGRAPHIC; + while (minIndex <= maxIndex) { + int currIndex = (minIndex + maxIndex) >>> 1; + + String currValue = delegate.getValue(idMapping.getReverseId(currIndex)); + int comparison = comparator.compare(currValue, value); + if (comparison == 0) { + return currIndex; + } + + if (comparison < 0) { + minIndex = currIndex + 1; + } else { + maxIndex = currIndex - 1; + } + } + + return -(minIndex + 1); } } } diff --git a/processing/src/test/java/org/apache/druid/segment/ColumnSelectorBitmapIndexSelectorTest.java b/processing/src/test/java/org/apache/druid/segment/ColumnSelectorBitmapIndexSelectorTest.java index 125f8ab4281b..92d51b542683 100644 --- a/processing/src/test/java/org/apache/druid/segment/ColumnSelectorBitmapIndexSelectorTest.java +++ b/processing/src/test/java/org/apache/druid/segment/ColumnSelectorBitmapIndexSelectorTest.java @@ -71,6 +71,7 @@ public void setup() ImmutableBitmap someBitmap = EasyMock.createMock(ImmutableBitmap.class); EasyMock.expect(someIndex.getIndex("foo")).andReturn(0).anyTimes(); EasyMock.expect(someIndex.getBitmap(0)).andReturn(someBitmap).anyTimes(); + EasyMock.expect(someIndex.getBitmapForValue("foo")).andReturn(someBitmap).anyTimes(); ColumnHolder nonStringHolder = EasyMock.createMock(ColumnHolder.class); diff --git a/processing/src/test/java/org/apache/druid/segment/IndexMergerNullHandlingTest.java b/processing/src/test/java/org/apache/druid/segment/IndexMergerNullHandlingTest.java index a9b63b477ab3..13f344b795e3 100644 --- a/processing/src/test/java/org/apache/druid/segment/IndexMergerNullHandlingTest.java +++ b/processing/src/test/java/org/apache/druid/segment/IndexMergerNullHandlingTest.java @@ -196,7 +196,7 @@ public void testStringColumnNullHandling() throws Exception if (expectedNullRows.size() > 0) { Assert.assertEquals(subsetList.toString(), 0, bitmapIndex.getIndex(null)); - final ImmutableBitmap nullBitmap = bitmapIndex.getBitmap(bitmapIndex.getIndex(null)); + final ImmutableBitmap nullBitmap = bitmapIndex.getBitmapForValue(null); final List actualNullRows = new ArrayList<>(); final IntIterator iterator = nullBitmap.iterator(); while (iterator.hasNext()) { diff --git a/processing/src/test/java/org/apache/druid/segment/filter/BaseFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/BaseFilterTest.java index c74c6eff2f92..c91c80604c0b 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/BaseFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/BaseFilterTest.java @@ -23,6 +23,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.common.collect.Maps; import org.apache.druid.common.config.NullHandling; @@ -90,6 +91,7 @@ import org.apache.druid.segment.vector.VectorObjectSelector; import org.apache.druid.segment.vector.VectorValueSelector; import org.apache.druid.segment.virtual.ExpressionVirtualColumn; +import org.apache.druid.segment.virtual.ListFilteredVirtualColumn; import org.apache.druid.segment.writeout.OffHeapMemorySegmentWriteOutMediumFactory; import org.apache.druid.segment.writeout.SegmentWriteOutMediumFactory; import org.apache.druid.segment.writeout.TmpFileSegmentWriteOutMediumFactory; @@ -125,7 +127,11 @@ public abstract class BaseFilterTest extends InitializedNullHandlingTest new ExpressionVirtualColumn("vdim1", "dim1", ColumnType.STRING, TestExprMacroTable.INSTANCE), new ExpressionVirtualColumn("vd0", "d0", ColumnType.DOUBLE, TestExprMacroTable.INSTANCE), new ExpressionVirtualColumn("vf0", "f0", ColumnType.FLOAT, TestExprMacroTable.INSTANCE), - new ExpressionVirtualColumn("vl0", "l0", ColumnType.LONG, TestExprMacroTable.INSTANCE) + new ExpressionVirtualColumn("vl0", "l0", ColumnType.LONG, TestExprMacroTable.INSTANCE), + new ListFilteredVirtualColumn("allow-dim0", DefaultDimensionSpec.of("dim0"), ImmutableSet.of("3", "4"), true), + new ListFilteredVirtualColumn("deny-dim0", DefaultDimensionSpec.of("dim0"), ImmutableSet.of("3", "4"), false), + new ListFilteredVirtualColumn("allow-dim2", DefaultDimensionSpec.of("dim2"), ImmutableSet.of("a"), true), + new ListFilteredVirtualColumn("deny-dim2", DefaultDimensionSpec.of("dim2"), ImmutableSet.of("a"), false) ) ); diff --git a/processing/src/test/java/org/apache/druid/segment/filter/BoundFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/BoundFilterTest.java index 1e6d188e4095..033b5bd5dbea 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/BoundFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/BoundFilterTest.java @@ -439,6 +439,45 @@ public void testNumericMatchVirtualColumn() ); } + @Test + public void testListFilteredVirtualColumn() + { + assertFilterMatchesSkipVectorize( + new BoundDimFilter("allow-dim0", "0", "2", false, false, false, null, StringComparators.LEXICOGRAPHIC), + ImmutableList.of() + ); + assertFilterMatchesSkipVectorize( + new BoundDimFilter("allow-dim0", "0", "6", false, false, false, null, StringComparators.LEXICOGRAPHIC), + ImmutableList.of("3", "4") + ); + assertFilterMatchesSkipVectorize( + new BoundDimFilter("deny-dim0", "0", "6", false, false, false, null, StringComparators.LEXICOGRAPHIC), + ImmutableList.of("0", "1", "2", "5", "6") + ); + assertFilterMatchesSkipVectorize( + new BoundDimFilter("deny-dim0", "3", "4", false, false, false, null, StringComparators.LEXICOGRAPHIC), + ImmutableList.of() + ); + + assertFilterMatchesSkipVectorize( + new BoundDimFilter("allow-dim2", "a", "c", false, false, false, null, StringComparators.LEXICOGRAPHIC), + ImmutableList.of("0", "3", "6") + ); + assertFilterMatchesSkipVectorize( + new BoundDimFilter("allow-dim2", "c", "z", false, false, false, null, StringComparators.LEXICOGRAPHIC), + ImmutableList.of() + ); + + assertFilterMatchesSkipVectorize( + new BoundDimFilter("deny-dim2", "a", "b", false, true, false, null, StringComparators.LEXICOGRAPHIC), + ImmutableList.of() + ); + assertFilterMatchesSkipVectorize( + new BoundDimFilter("deny-dim2", "c", "z", false, false, false, null, StringComparators.LEXICOGRAPHIC), + ImmutableList.of("4", "7") + ); + } + @Test public void testNumericMatchExactlySingleValue() { diff --git a/processing/src/test/java/org/apache/druid/segment/filter/ExtractionDimFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/ExtractionDimFilterTest.java index a6c0aec0176d..c37efe024fc5 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/ExtractionDimFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/ExtractionDimFilterTest.java @@ -37,6 +37,8 @@ import org.apache.druid.query.monomorphicprocessing.RuntimeShapeInspector; import org.apache.druid.segment.column.BitmapIndex; import org.apache.druid.segment.column.ColumnCapabilities; +import org.apache.druid.segment.column.ColumnCapabilitiesImpl; +import org.apache.druid.segment.column.ColumnType; import org.apache.druid.segment.data.BitmapSerdeFactory; import org.apache.druid.segment.data.CloseableIndexed; import org.apache.druid.segment.data.ConciseBitmapSerdeFactory; @@ -95,6 +97,13 @@ public ExtractionDimFilterTest(BitmapFactory bitmapFactory, BitmapSerdeFactory b private final BitmapIndexSelector BITMAP_INDEX_SELECTOR = new BitmapIndexSelector() { + @Nullable + @Override + public ColumnCapabilities getColumnCapabilities(String column) + { + return ColumnCapabilitiesImpl.createDefault().setType(ColumnType.STRING).setHasMultipleValues(true); + } + @Override public CloseableIndexed getDimensionValues(String dimension) { diff --git a/processing/src/test/java/org/apache/druid/segment/filter/FiltersTest.java b/processing/src/test/java/org/apache/druid/segment/filter/FiltersTest.java index f9d493a167f7..b31dc60b5eb0 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/FiltersTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/FiltersTest.java @@ -31,7 +31,10 @@ import org.junit.Assert; import org.junit.Test; +import javax.annotation.Nullable; import java.util.List; +import java.util.Set; +import java.util.function.IntPredicate; public class FiltersTest extends InitializedNullHandlingTest { @@ -85,6 +88,41 @@ public int getIndex(String value) throw new UnsupportedOperationException(); } + @Override + public ImmutableBitmap getBitmapForValue(@Nullable String value) + { + throw new UnsupportedOperationException(); + } + + @Override + public Iterable getBitmapsInRange( + @Nullable String startValue, + boolean startStrict, + @Nullable String endValue, + boolean endStrict + ) + { + throw new UnsupportedOperationException(); + } + + @Override + public Iterable getBitmapsInRange( + @Nullable String startValue, + boolean startStrict, + @Nullable String endValue, + boolean endStrict, + IntPredicate matcher + ) + { + throw new UnsupportedOperationException(); + } + + @Override + public Iterable getBitmapsForValues(Set values) + { + throw new UnsupportedOperationException(); + } + @Override public ImmutableBitmap getBitmap(int idx) { diff --git a/processing/src/test/java/org/apache/druid/segment/filter/SelectorFilterTest.java b/processing/src/test/java/org/apache/druid/segment/filter/SelectorFilterTest.java index 81aa2efa4d87..3939cf8af62a 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/SelectorFilterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/SelectorFilterTest.java @@ -115,6 +115,20 @@ public void testSingleValueVirtualStringColumnWithoutNulls() assertFilterMatches(new SelectorDimFilter("vdim0", "1", null), ImmutableList.of("1")); } + @Test + public void testListFilteredVirtualColumn() + { + assertFilterMatchesSkipVectorize(new SelectorDimFilter("allow-dim0", "1", null), ImmutableList.of()); + assertFilterMatchesSkipVectorize(new SelectorDimFilter("allow-dim0", "4", null), ImmutableList.of("4")); + assertFilterMatchesSkipVectorize(new SelectorDimFilter("deny-dim0", "0", null), ImmutableList.of("0")); + assertFilterMatchesSkipVectorize(new SelectorDimFilter("deny-dim0", "4", null), ImmutableList.of()); + + assertFilterMatchesSkipVectorize(new SelectorDimFilter("allow-dim2", "b", null), ImmutableList.of()); + assertFilterMatchesSkipVectorize(new SelectorDimFilter("allow-dim2", "a", null), ImmutableList.of("0", "3")); + assertFilterMatchesSkipVectorize(new SelectorDimFilter("deny-dim2", "b", null), ImmutableList.of("0")); + assertFilterMatchesSkipVectorize(new SelectorDimFilter("deny-dim2", "a", null), ImmutableList.of()); + } + @Test public void testSingleValueStringColumnWithNulls() { diff --git a/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionSelectorsTest.java b/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionSelectorsTest.java index 93e7b10727fa..67620a1d27cf 100644 --- a/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionSelectorsTest.java +++ b/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionSelectorsTest.java @@ -52,6 +52,8 @@ import org.apache.druid.segment.TestObjectColumnSelector; import org.apache.druid.segment.VirtualColumns; import org.apache.druid.segment.column.ColumnCapabilities; +import org.apache.druid.segment.column.ColumnCapabilitiesImpl; +import org.apache.druid.segment.column.ColumnType; import org.apache.druid.segment.generator.GeneratorBasicSchemas; import org.apache.druid.segment.generator.GeneratorSchemaInfo; import org.apache.druid.segment.generator.SegmentGenerator; @@ -82,6 +84,19 @@ public class ExpressionSelectorsTest extends InitializedNullHandlingTest private static IncrementalIndexStorageAdapter INCREMENTAL_INDEX_STORAGE_ADAPTER; private static List ADAPTERS; + private static final ColumnCapabilities SINGLE_VALUE = new ColumnCapabilitiesImpl().setType(ColumnType.STRING) + .setDictionaryEncoded(true) + .setDictionaryValuesUnique(true) + .setDictionaryValuesSorted(true) + .setHasMultipleValues(false) + .setHasNulls(true); + private static final ColumnCapabilities MULTI_VAL = new ColumnCapabilitiesImpl().setType(ColumnType.STRING) + .setDictionaryEncoded(true) + .setDictionaryValuesUnique(true) + .setDictionaryValuesSorted(true) + .setHasMultipleValues(true) + .setHasNulls(true); + @BeforeClass public static void setup() { @@ -394,7 +409,7 @@ public void test_canMapOverDictionary_oneSingleValueInput() Assert.assertTrue( ExpressionSelectors.canMapOverDictionary( Parser.parse("dim1 == 2", ExprMacroTable.nil()).analyzeInputs(), - ColumnCapabilities.Capable.FALSE + SINGLE_VALUE ) ); } @@ -405,7 +420,7 @@ public void test_canMapOverDictionary_oneSingleValueInputSpecifiedTwice() Assert.assertTrue( ExpressionSelectors.canMapOverDictionary( Parser.parse("concat(dim1, dim1) == 2", ExprMacroTable.nil()).analyzeInputs(), - ColumnCapabilities.Capable.FALSE + SINGLE_VALUE ) ); } @@ -416,7 +431,7 @@ public void test_canMapOverDictionary_oneMultiValueInput() Assert.assertTrue( ExpressionSelectors.canMapOverDictionary( Parser.parse("dim1 == 2", ExprMacroTable.nil()).analyzeInputs(), - ColumnCapabilities.Capable.TRUE + MULTI_VAL ) ); } @@ -427,7 +442,7 @@ public void test_canMapOverDictionary_oneUnknownInput() Assert.assertFalse( ExpressionSelectors.canMapOverDictionary( Parser.parse("dim1 == 2", ExprMacroTable.nil()).analyzeInputs(), - ColumnCapabilities.Capable.UNKNOWN + new ColumnCapabilitiesImpl() ) ); } @@ -438,7 +453,7 @@ public void test_canMapOverDictionary_oneSingleValueInputInArrayContext() Assert.assertFalse( ExpressionSelectors.canMapOverDictionary( Parser.parse("array_contains(dim1, 2)", ExprMacroTable.nil()).analyzeInputs(), - ColumnCapabilities.Capable.FALSE + ColumnCapabilitiesImpl.createDefault().setType(ColumnType.STRING_ARRAY) ) ); } @@ -449,7 +464,7 @@ public void test_canMapOverDictionary_oneMultiValueInputInArrayContext() Assert.assertFalse( ExpressionSelectors.canMapOverDictionary( Parser.parse("array_contains(dim1, 2)", ExprMacroTable.nil()).analyzeInputs(), - ColumnCapabilities.Capable.TRUE + MULTI_VAL ) ); } @@ -460,7 +475,7 @@ public void test_canMapOverDictionary_oneUnknownInputInArrayContext() Assert.assertFalse( ExpressionSelectors.canMapOverDictionary( Parser.parse("array_contains(dim1, 2)", ExprMacroTable.nil()).analyzeInputs(), - ColumnCapabilities.Capable.UNKNOWN + new ColumnCapabilitiesImpl() ) ); } @@ -471,7 +486,7 @@ public void test_canMapOverDictionary() Assert.assertTrue( ExpressionSelectors.canMapOverDictionary( Parser.parse("dim1 == 2", ExprMacroTable.nil()).analyzeInputs(), - ColumnCapabilities.Capable.FALSE + SINGLE_VALUE ) ); } diff --git a/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionVectorSelectorsTest.java b/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionVectorSelectorsTest.java index e0295eb6fb2d..54fc20deed02 100644 --- a/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionVectorSelectorsTest.java +++ b/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionVectorSelectorsTest.java @@ -30,7 +30,6 @@ import org.apache.druid.math.expr.Parser; import org.apache.druid.query.dimension.DefaultDimensionSpec; import org.apache.druid.query.expression.TestExprMacroTable; -import org.apache.druid.segment.ColumnInspector; import org.apache.druid.segment.ColumnValueSelector; import org.apache.druid.segment.Cursor; import org.apache.druid.segment.QueryableIndex; @@ -145,17 +144,7 @@ public ExpressionVectorSelectorsTest(String expression) public void setup() { Expr parsed = Parser.parse(expression, ExprMacroTable.nil()); - outputType = parsed.getOutputType( - new ColumnInspector() - { - @Nullable - @Override - public ColumnCapabilities getColumnCapabilities(String column) - { - return QueryableIndexStorageAdapter.getColumnCapabilities(INDEX, column); - } - } - ); + outputType = parsed.getOutputType(INDEX); if (outputType == null) { outputType = ExpressionType.STRING; } From 4af850aa74a58a6019ae1f5f2f26a38c94854483 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Mon, 7 Mar 2022 15:49:05 -0800 Subject: [PATCH 02/11] fix bug --- .../query/dimension/PredicateFilteredDimensionSelector.java | 6 +++--- .../apache/druid/segment/virtual/ExpressionSelectors.java | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/dimension/PredicateFilteredDimensionSelector.java b/processing/src/main/java/org/apache/druid/query/dimension/PredicateFilteredDimensionSelector.java index 535442bdcc7c..b462552aceb4 100644 --- a/processing/src/main/java/org/apache/druid/query/dimension/PredicateFilteredDimensionSelector.java +++ b/processing/src/main/java/org/apache/druid/query/dimension/PredicateFilteredDimensionSelector.java @@ -81,8 +81,8 @@ public boolean matches() nullRow = false; } } - // null should match empty rows in multi-value columns - return nullRow && value == null; + // null should match empty rows in multi-value columns if predicate matches null + return nullRow && predicate.apply(null); } @Override @@ -97,7 +97,7 @@ public void inspectRuntimeShape(RuntimeShapeInspector inspector) @Override public ValueMatcher makeValueMatcher(final Predicate matcherPredicate) { - final boolean matchNull = predicate.apply(null); + final boolean matchNull = predicate.apply(null) && matcherPredicate.apply(null); return new ValueMatcher() { @Override diff --git a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java index efcdc7fb66ed..a691f625c02b 100644 --- a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java +++ b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionSelectors.java @@ -233,7 +233,7 @@ public static DimensionSelector makeDimensionSelector( * and that single column has a dictionary. * * @param bindingAnalysis result of calling {@link Expr#analyzeInputs()} on an expression - * @param hasMultipleValues result of calling {@link ColumnCapabilities#hasMultipleValues()} + * @param columnCapabilities {@link ColumnCapabilities} for the input binding */ public static boolean canMapOverDictionary( final Expr.BindingAnalysis bindingAnalysis, From b305f4fe4912e8b4e604c803df11e4aea195e5ec Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Mon, 7 Mar 2022 15:51:36 -0800 Subject: [PATCH 03/11] oops, fix better --- .../query/dimension/PredicateFilteredDimensionSelector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/processing/src/main/java/org/apache/druid/query/dimension/PredicateFilteredDimensionSelector.java b/processing/src/main/java/org/apache/druid/query/dimension/PredicateFilteredDimensionSelector.java index b462552aceb4..bf02bc3abe30 100644 --- a/processing/src/main/java/org/apache/druid/query/dimension/PredicateFilteredDimensionSelector.java +++ b/processing/src/main/java/org/apache/druid/query/dimension/PredicateFilteredDimensionSelector.java @@ -82,7 +82,7 @@ public boolean matches() } } // null should match empty rows in multi-value columns if predicate matches null - return nullRow && predicate.apply(null); + return nullRow && value == null && predicate.apply(null); } @Override From 3c751425a9069a3c9e10c892b7bdf648e6feee76 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Mon, 7 Mar 2022 17:55:12 -0800 Subject: [PATCH 04/11] better like, fix test, javadocs --- .../druid/query/filter/LikeDimFilter.java | 9 ++-- .../apache/druid/segment/ColumnInspector.java | 9 ++++ .../ColumnSelectorBitmapIndexSelector.java | 8 ++-- .../druid/segment/column/BitmapIndex.java | 47 ++++++++++++++++--- .../apache/druid/segment/data/Indexed.java | 4 +- .../segment/filter/ExpressionFilter.java | 11 +++-- .../druid/segment/filter/LikeFilter.java | 44 +++++------------ .../StringBitmapIndexColumnPartSupplier.java | 6 +-- .../virtual/ListFilteredVirtualColumn.java | 6 +-- .../druid/segment/filter/FiltersTest.java | 4 +- 10 files changed, 86 insertions(+), 62 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/query/filter/LikeDimFilter.java b/processing/src/main/java/org/apache/druid/query/filter/LikeDimFilter.java index a92bd44fa745..50a06b2392cb 100644 --- a/processing/src/main/java/org/apache/druid/query/filter/LikeDimFilter.java +++ b/processing/src/main/java/org/apache/druid/query/filter/LikeDimFilter.java @@ -32,7 +32,6 @@ import org.apache.druid.common.config.NullHandling; import org.apache.druid.java.util.common.StringUtils; import org.apache.druid.query.extraction.ExtractionFn; -import org.apache.druid.segment.data.Indexed; import org.apache.druid.segment.filter.LikeFilter; import javax.annotation.Nullable; @@ -299,17 +298,15 @@ private static boolean matches(@Nullable final String s, Pattern pattern) * of s are ignored. This method is useful if you've already independently verified the prefix. This method * evalutes strings.get(i) lazily to save time when it isn't necessary to actually look at the string. */ - public boolean matchesSuffixOnly(final Indexed strings, final int i) + public boolean matchesSuffixOnly(@Nullable String value) { if (suffixMatch == SuffixMatch.MATCH_ANY) { return true; } else if (suffixMatch == SuffixMatch.MATCH_EMPTY) { - final String s = strings.get(i); - return s == null ? matches(null) : s.length() == prefix.length(); + return value == null ? matches(null) : value.length() == prefix.length(); } else { // suffixMatch is MATCH_PATTERN - final String s = strings.get(i); - return matches(s); + return matches(value); } } diff --git a/processing/src/main/java/org/apache/druid/segment/ColumnInspector.java b/processing/src/main/java/org/apache/druid/segment/ColumnInspector.java index d8b4aaf083fc..6f2f478428b8 100644 --- a/processing/src/main/java/org/apache/druid/segment/ColumnInspector.java +++ b/processing/src/main/java/org/apache/druid/segment/ColumnInspector.java @@ -37,6 +37,15 @@ public interface ColumnInspector extends Expr.InputBindingInspector @Nullable ColumnCapabilities getColumnCapabilities(String column); + default ColumnCapabilities getColumnCapabilitiesWithDefault(String column, ColumnCapabilities defaultCapabilites) + { + final ColumnCapabilities capabilities = getColumnCapabilities(column); + if (capabilities != null) { + return capabilities; + } + return defaultCapabilites; + } + @Nullable @Override default ExpressionType getType(String name) diff --git a/processing/src/main/java/org/apache/druid/segment/ColumnSelectorBitmapIndexSelector.java b/processing/src/main/java/org/apache/druid/segment/ColumnSelectorBitmapIndexSelector.java index 29e988da661c..d1e5a9bc4862 100644 --- a/processing/src/main/java/org/apache/druid/segment/ColumnSelectorBitmapIndexSelector.java +++ b/processing/src/main/java/org/apache/druid/segment/ColumnSelectorBitmapIndexSelector.java @@ -42,7 +42,7 @@ import java.io.IOException; import java.util.Iterator; import java.util.Set; -import java.util.function.IntPredicate; +import java.util.function.Predicate; /** */ @@ -282,7 +282,7 @@ public Iterable getBitmapsInRange( boolean startStrict, @Nullable String endValue, boolean endStrict, - IntPredicate matcher + Predicate matcher ) { final int startIndex; // inclusive @@ -308,8 +308,8 @@ public Iterable getBitmapsInRange( } } - endIndex = startIndex > endIndex || !matcher.test(endIndex) ? startIndex : endIndex; - if (matcher.test(startIndex)) { + endIndex = startIndex > endIndex || !matcher.test(getValue(endIndex)) ? startIndex : endIndex; + if (matcher.test(getValue(startIndex))) { final IntIterator rangeIterator = IntListUtils.fromTo(startIndex, endIndex).iterator(); return () -> new Iterator() { diff --git a/processing/src/main/java/org/apache/druid/segment/column/BitmapIndex.java b/processing/src/main/java/org/apache/druid/segment/column/BitmapIndex.java index b08916146337..fb028aa60ec3 100644 --- a/processing/src/main/java/org/apache/druid/segment/column/BitmapIndex.java +++ b/processing/src/main/java/org/apache/druid/segment/column/BitmapIndex.java @@ -24,34 +24,59 @@ import javax.annotation.Nullable; import java.util.Set; -import java.util.function.IntPredicate; +import java.util.function.Predicate; /** + * Provides a mechanism to get {@link ImmutableBitmap} for a value, or {@link Iterable} of {@link ImmutableBitmap} + * for a range or exact set of values, the set bits of which correspond to which rows contain the matching values. + * + * Used to power {@link org.apache.druid.segment.BitmapOffset} and + * {@link org.apache.druid.segment.vector.BitmapVectorOffset} which are used with column cursors for fast filtering + * of indexed values. + * + * The column must be ingested with a bitmap index for filters to use them and to participate in this "pre-filtering" + * step when scanning segments + * + * @see org.apache.druid.segment.QueryableIndexStorageAdapter#analyzeFilter */ public interface BitmapIndex { + /** + * Get the cardinality of the underlying value dictionary + */ int getCardinality(); + /** + * Get the value in the underlying value dictionary of the specified dictionary id + */ @Nullable String getValue(int index); + /** + * Returns true if the underlying value dictionary has nulls + */ boolean hasNulls(); BitmapFactory getBitmapFactory(); /** - * Returns the index of "value" in this BitmapIndex, or (-(insertion point) - 1) if the value is not - * present, in the manner of Arrays.binarySearch. - * - * @param value value to search for - * @return index of value, or negative number equal to (-(insertion point) - 1). + * Returns the index of "value" in this BitmapIndex, or a negative value, if not present in the underlying dictionary */ int getIndex(@Nullable String value); + /** + * Get the {@link ImmutableBitmap} for dictionary id of the underlying dictionary + */ ImmutableBitmap getBitmap(int idx); + /** + * Get the {@link ImmutableBitmap} corresponding to the supplied value + */ ImmutableBitmap getBitmapForValue(@Nullable String value); + /** + * Get an {@link Iterable} of {@link ImmutableBitmap} corresponding to the values supplied in the specified range + */ default Iterable getBitmapsInRange( @Nullable String startValue, boolean startStrict, @@ -62,13 +87,21 @@ default Iterable getBitmapsInRange( return getBitmapsInRange(startValue, startStrict, endValue, endStrict, (index) -> true); } + /** + * Get an {@link Iterable} of {@link ImmutableBitmap} corresponding to the values supplied in the specified range + * whose dictionary ids also match some predicate + */ Iterable getBitmapsInRange( @Nullable String startValue, boolean startStrict, @Nullable String endValue, boolean endStrict, - IntPredicate matcher + Predicate matcher ); + /** + * Get an {@link Iterable} of {@link ImmutableBitmap} corresponding to the specified set of values (if they are + * contained in the underlying column) + */ Iterable getBitmapsForValues(Set values); } diff --git a/processing/src/main/java/org/apache/druid/segment/data/Indexed.java b/processing/src/main/java/org/apache/druid/segment/data/Indexed.java index e4b5b772b955..8e0cb295411d 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/Indexed.java +++ b/processing/src/main/java/org/apache/druid/segment/data/Indexed.java @@ -55,8 +55,8 @@ public interface Indexed extends Iterable, HotLoopCallee int indexOf(@Nullable T value); /** - * Returns the index of "value" in some object whose values are accessible by index some {@link IndexedGetter}, or - * (-(insertion point) - 1) if the value is not present, in the manner of Arrays.binarySearch. + * Returns the index of "value" in some object in the {@link Indexed}, or (-(insertion point) - 1) if the value is + * not present, in the manner of Arrays.binarySearch. * * This is used by {@link GenericIndexed} to strengthen the contract of {@link #indexOf(Object)}, which only * guarantees that values-not-found will return some negative number. diff --git a/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java index fe45a86425bd..f29371340245 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/ExpressionFilter.java @@ -230,9 +230,14 @@ public boolean supportsBitmapIndex(final BitmapIndexSelector selector) // Single-column expression. We can use bitmap indexes if this column has an index and the expression can // map over the values of the index. final String column = Iterables.getOnlyElement(details.getRequiredBindings()); - final ColumnCapabilities capabilities = selector.getColumnCapabilities(column); - return selector.getBitmapIndex(column) != null - && ExpressionSelectors.canMapOverDictionary(details, capabilities); + // we use a default 'all false' capabilities here because if the column has a bitmap index, but the capabilities + // are null, it means that the column is missing and should take the single valued path, while truly unknown + // things will not have a bitmap index available + final ColumnCapabilities capabilities = selector.getColumnCapabilitiesWithDefault( + column, + ColumnCapabilitiesImpl.createDefault() + ); + return selector.getBitmapIndex(column) != null && ExpressionSelectors.canMapOverDictionary(details, capabilities); } else { // Multi-column expression. return false; diff --git a/processing/src/main/java/org/apache/druid/segment/filter/LikeFilter.java b/processing/src/main/java/org/apache/druid/segment/filter/LikeFilter.java index 3676a4b23b77..d03ac71d36a4 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/LikeFilter.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/LikeFilter.java @@ -38,12 +38,8 @@ import org.apache.druid.segment.ColumnSelector; import org.apache.druid.segment.ColumnSelectorFactory; import org.apache.druid.segment.column.BitmapIndex; -import org.apache.druid.segment.data.CloseableIndexed; -import org.apache.druid.segment.data.Indexed; import org.apache.druid.segment.vector.VectorColumnSelectorFactory; -import java.io.IOException; -import java.io.UncheckedIOException; import java.util.Map; import java.util.Objects; import java.util.Set; @@ -172,16 +168,18 @@ private Iterable getBitmapIterable(final BitmapIndexSelector se return ImmutableList.of(likeMatcher.matches(null) ? Filters.allTrue(selector) : Filters.allFalse(selector)); } - // search for start, end indexes in the bitmaps; then include all matching bitmaps between those points - try (final CloseableIndexed dimValues = selector.getDimensionValues(dimension)) { - - // Union bitmaps for all matching dimension values in range. - // Use lazy iterator to allow unioning bitmaps one by one and avoid materializing all of them at once. - return getDimValueIndexIterableForPrefixMatch(bitmapIndex, dimValues); - } - catch (IOException e) { - throw new UncheckedIOException(e); - } + final String lower = NullHandling.nullToEmptyIfNeeded(likeMatcher.getPrefix()); + final String upper = NullHandling.nullToEmptyIfNeeded(likeMatcher.getPrefix()) + Character.MAX_VALUE; + + // Union bitmaps for all matching dimension values in range. + // Use lazy iterator to allow unioning bitmaps one by one and avoid materializing all of them at once. + return bitmapIndex.getBitmapsInRange( + lower, + false, + upper, + false, + (value) -> likeMatcher.matchesSuffixOnly(value) + ); } else { // fallback return Filters.matchPredicateNoUnion( @@ -208,24 +206,6 @@ private boolean isSimplePrefix() return extractionFn == null && !likeMatcher.getPrefix().isEmpty(); } - private Iterable getDimValueIndexIterableForPrefixMatch( - final BitmapIndex bitmapIndex, - final Indexed dimValues - ) - { - - final String lower = NullHandling.nullToEmptyIfNeeded(likeMatcher.getPrefix()); - final String upper = NullHandling.nullToEmptyIfNeeded(likeMatcher.getPrefix()) + Character.MAX_VALUE; - - return bitmapIndex.getBitmapsInRange( - lower, - false, - upper, - false, - (index) -> likeMatcher.matchesSuffixOnly(dimValues, index) - ); - } - @Override public boolean equals(Object o) { diff --git a/processing/src/main/java/org/apache/druid/segment/serde/StringBitmapIndexColumnPartSupplier.java b/processing/src/main/java/org/apache/druid/segment/serde/StringBitmapIndexColumnPartSupplier.java index 286df7202dcf..c0b903444d28 100644 --- a/processing/src/main/java/org/apache/druid/segment/serde/StringBitmapIndexColumnPartSupplier.java +++ b/processing/src/main/java/org/apache/druid/segment/serde/StringBitmapIndexColumnPartSupplier.java @@ -32,7 +32,7 @@ import java.util.Iterator; import java.util.NoSuchElementException; import java.util.Set; -import java.util.function.IntPredicate; +import java.util.function.Predicate; /** * Provides {@link BitmapIndex} for some dictionary encoded column, where the dictionary and bitmaps are stored in some @@ -164,7 +164,7 @@ public Iterable getBitmapsInRange( boolean startStrict, @Nullable String endValue, boolean endStrict, - IntPredicate indexMatcher + Predicate indexMatcher ) { int startIndex, endIndex; @@ -202,7 +202,7 @@ public Iterable getBitmapsInRange( private int findNext() { - while (currIndex < end && !indexMatcher.test(currIndex)) { + while (currIndex < end && !indexMatcher.test(dictionary.get(currIndex))) { currIndex++; } diff --git a/processing/src/main/java/org/apache/druid/segment/virtual/ListFilteredVirtualColumn.java b/processing/src/main/java/org/apache/druid/segment/virtual/ListFilteredVirtualColumn.java index e87db1b5aac6..96e0f1454e68 100644 --- a/processing/src/main/java/org/apache/druid/segment/virtual/ListFilteredVirtualColumn.java +++ b/processing/src/main/java/org/apache/druid/segment/virtual/ListFilteredVirtualColumn.java @@ -50,7 +50,7 @@ import java.util.NoSuchElementException; import java.util.Objects; import java.util.Set; -import java.util.function.IntPredicate; +import java.util.function.Predicate; /** * {@link VirtualColumn} form of {@link ListFilteredDimensionSpec}, powered by @@ -289,7 +289,7 @@ public Iterable getBitmapsInRange( boolean startStrict, @Nullable String endValue, boolean endStrict, - IntPredicate matcher + Predicate matcher ) { int startIndex, endIndex; @@ -327,7 +327,7 @@ public Iterable getBitmapsInRange( private int findNext() { - while (currIndex < end && !matcher.test(currIndex)) { + while (currIndex < end && !matcher.test(delegate.getValue(idMapping.getReverseId(currIndex)))) { currIndex++; } diff --git a/processing/src/test/java/org/apache/druid/segment/filter/FiltersTest.java b/processing/src/test/java/org/apache/druid/segment/filter/FiltersTest.java index b31dc60b5eb0..d0f2009345dc 100644 --- a/processing/src/test/java/org/apache/druid/segment/filter/FiltersTest.java +++ b/processing/src/test/java/org/apache/druid/segment/filter/FiltersTest.java @@ -34,7 +34,7 @@ import javax.annotation.Nullable; import java.util.List; import java.util.Set; -import java.util.function.IntPredicate; +import java.util.function.Predicate; public class FiltersTest extends InitializedNullHandlingTest { @@ -111,7 +111,7 @@ public Iterable getBitmapsInRange( boolean startStrict, @Nullable String endValue, boolean endStrict, - IntPredicate matcher + Predicate matcher ) { throw new UnsupportedOperationException(); From 3b854e3c52a968336ed1239dabff39782315f0b0 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Mon, 7 Mar 2022 20:19:18 -0800 Subject: [PATCH 05/11] fix checkstyle --- .../src/main/java/org/apache/druid/segment/ColumnInspector.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/processing/src/main/java/org/apache/druid/segment/ColumnInspector.java b/processing/src/main/java/org/apache/druid/segment/ColumnInspector.java index 6f2f478428b8..7d91d50e851b 100644 --- a/processing/src/main/java/org/apache/druid/segment/ColumnInspector.java +++ b/processing/src/main/java/org/apache/druid/segment/ColumnInspector.java @@ -39,7 +39,7 @@ public interface ColumnInspector extends Expr.InputBindingInspector default ColumnCapabilities getColumnCapabilitiesWithDefault(String column, ColumnCapabilities defaultCapabilites) { - final ColumnCapabilities capabilities = getColumnCapabilities(column); + final ColumnCapabilities capabilities = getColumnCapabilities(column); if (capabilities != null) { return capabilities; } From 86a5f53992f1751aae30c04b6389beb6905b1041 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Tue, 8 Mar 2022 20:53:37 -0800 Subject: [PATCH 06/11] simplify and fixes --- .../ColumnSelectorBitmapIndexSelector.java | 25 ++---- .../apache/druid/segment/VirtualColumns.java | 7 +- .../StringBitmapIndexColumnPartSupplier.java | 88 ++++++++----------- ...ListFilteredVirtualColumnSelectorTest.java | 16 +++- 4 files changed, 62 insertions(+), 74 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/segment/ColumnSelectorBitmapIndexSelector.java b/processing/src/main/java/org/apache/druid/segment/ColumnSelectorBitmapIndexSelector.java index d1e5a9bc4862..22a263f9e544 100644 --- a/processing/src/main/java/org/apache/druid/segment/ColumnSelectorBitmapIndexSelector.java +++ b/processing/src/main/java/org/apache/druid/segment/ColumnSelectorBitmapIndexSelector.java @@ -20,7 +20,6 @@ package org.apache.druid.segment; import com.google.common.collect.ImmutableList; -import it.unimi.dsi.fastutil.ints.IntIterator; import org.apache.druid.collections.bitmap.BitmapFactory; import org.apache.druid.collections.bitmap.ImmutableBitmap; import org.apache.druid.collections.spatial.ImmutableRTree; @@ -40,6 +39,7 @@ import javax.annotation.Nullable; import java.io.IOException; +import java.util.Collections; import java.util.Iterator; import java.util.Set; import java.util.function.Predicate; @@ -308,23 +308,12 @@ public Iterable getBitmapsInRange( } } - endIndex = startIndex > endIndex || !matcher.test(getValue(endIndex)) ? startIndex : endIndex; - if (matcher.test(getValue(startIndex))) { - final IntIterator rangeIterator = IntListUtils.fromTo(startIndex, endIndex).iterator(); - return () -> new Iterator() - { - @Override - public boolean hasNext() - { - return rangeIterator.hasNext(); - } - - @Override - public ImmutableBitmap next() - { - return getBitmap(rangeIterator.nextInt()); - } - }; + endIndex = Math.max(startIndex, endIndex); + if (startIndex == endIndex) { + return Collections.emptyList(); + } + if (matcher.test(null)) { + return ImmutableList.of(getBitmap(0)); } return ImmutableList.of(bitmapFactory.makeEmptyImmutableBitmap()); } diff --git a/processing/src/main/java/org/apache/druid/segment/VirtualColumns.java b/processing/src/main/java/org/apache/druid/segment/VirtualColumns.java index dc6441c3375a..ba8f033310a3 100644 --- a/processing/src/main/java/org/apache/druid/segment/VirtualColumns.java +++ b/processing/src/main/java/org/apache/druid/segment/VirtualColumns.java @@ -174,10 +174,9 @@ public VirtualColumn getVirtualColumn(String columnName) public BitmapIndex getBitmapIndex(String columnName, ColumnSelector columnSelector) { final VirtualColumn virtualColumn = getVirtualColumnForSelector(columnName); - return virtualColumn.capabilities(columnName).hasBitmapIndexes() ? virtualColumn.getBitmapIndex( - columnName, - columnSelector - ) : null; + return virtualColumn.capabilities(columnSelector, columnName).hasBitmapIndexes() + ? virtualColumn.getBitmapIndex(columnName, columnSelector) + : null; } /** diff --git a/processing/src/main/java/org/apache/druid/segment/serde/StringBitmapIndexColumnPartSupplier.java b/processing/src/main/java/org/apache/druid/segment/serde/StringBitmapIndexColumnPartSupplier.java index c0b903444d28..393b9ee64c3f 100644 --- a/processing/src/main/java/org/apache/druid/segment/serde/StringBitmapIndexColumnPartSupplier.java +++ b/processing/src/main/java/org/apache/druid/segment/serde/StringBitmapIndexColumnPartSupplier.java @@ -20,6 +20,8 @@ package org.apache.druid.segment.serde; import com.google.common.base.Supplier; +import it.unimi.dsi.fastutil.ints.IntIntImmutablePair; +import it.unimi.dsi.fastutil.ints.IntIntPair; import it.unimi.dsi.fastutil.ints.IntIterator; import org.apache.druid.collections.bitmap.BitmapFactory; import org.apache.druid.collections.bitmap.ImmutableBitmap; @@ -87,7 +89,6 @@ public BitmapFactory getBitmapFactory() @Override public int getIndex(@Nullable String value) { - // GenericIndexed.indexOf satisfies contract needed by BitmapIndex.indexOf return dictionary.indexOf(value); } @@ -117,33 +118,12 @@ public Iterable getBitmapsInRange( boolean endStrict ) { - int startIndex, endIndex; - if (startValue == null) { - startIndex = 0; - } else { - final int found = dictionary.indexOf(NullHandling.emptyToNullIfNeeded(startValue)); - if (found >= 0) { - startIndex = startStrict ? found + 1 : found; - } else { - startIndex = -(found + 1); - } - } - - if (endValue == null) { - endIndex = dictionary.size(); - } else { - final int found = dictionary.indexOf(NullHandling.emptyToNullIfNeeded(endValue)); - if (found >= 0) { - endIndex = endStrict ? found : found + 1; - } else { - endIndex = -(found + 1); - } - } - - endIndex = startIndex > endIndex ? startIndex : endIndex; - final IntIterator rangeIterator = IntListUtils.fromTo(startIndex, endIndex).iterator(); + final IntIntPair range = getRange(startValue, startStrict, endValue, endStrict); + final int start = range.leftInt(), end = range.rightInt(); return () -> new Iterator() { + final IntIterator rangeIterator = IntListUtils.fromTo(start, end).iterator(); + @Override public boolean hasNext() { @@ -167,31 +147,8 @@ public Iterable getBitmapsInRange( Predicate indexMatcher ) { - int startIndex, endIndex; - if (startValue == null) { - startIndex = 0; - } else { - final int found = dictionary.indexOf(NullHandling.emptyToNullIfNeeded(startValue)); - if (found >= 0) { - startIndex = startStrict ? found + 1 : found; - } else { - startIndex = -(found + 1); - } - } - - if (endValue == null) { - endIndex = dictionary.size(); - } else { - final int found = dictionary.indexOf(NullHandling.emptyToNullIfNeeded(endValue)); - if (found >= 0) { - endIndex = endStrict ? found : found + 1; - } else { - endIndex = -(found + 1); - } - } - - endIndex = startIndex > endIndex ? startIndex : endIndex; - final int start = startIndex, end = endIndex; + final IntIntPair range = getRange(startValue, startStrict, endValue, endStrict); + final int start = range.leftInt(), end = range.rightInt(); return () -> new Iterator() { int currIndex = start; @@ -274,6 +231,35 @@ private void findNext() } }; } + + private IntIntPair getRange(@Nullable String startValue, boolean startStrict, @Nullable String endValue, boolean endStrict) + { + int startIndex, endIndex; + if (startValue == null) { + startIndex = 0; + } else { + final int found = dictionary.indexOf(NullHandling.emptyToNullIfNeeded(startValue)); + if (found >= 0) { + startIndex = startStrict ? found + 1 : found; + } else { + startIndex = -(found + 1); + } + } + + if (endValue == null) { + endIndex = dictionary.size(); + } else { + final int found = dictionary.indexOf(NullHandling.emptyToNullIfNeeded(endValue)); + if (found >= 0) { + endIndex = endStrict ? found : found + 1; + } else { + endIndex = -(found + 1); + } + } + + endIndex = Math.max(startIndex, endIndex); + return new IntIntImmutablePair(startIndex, endIndex); + } }; } } diff --git a/processing/src/test/java/org/apache/druid/segment/virtual/ListFilteredVirtualColumnSelectorTest.java b/processing/src/test/java/org/apache/druid/segment/virtual/ListFilteredVirtualColumnSelectorTest.java index 9dbc25d4317e..638e9e2413fe 100644 --- a/processing/src/test/java/org/apache/druid/segment/virtual/ListFilteredVirtualColumnSelectorTest.java +++ b/processing/src/test/java/org/apache/druid/segment/virtual/ListFilteredVirtualColumnSelectorTest.java @@ -36,6 +36,7 @@ import org.apache.druid.segment.VirtualColumns; import org.apache.druid.segment.column.BitmapIndex; import org.apache.druid.segment.column.ColumnCapabilities; +import org.apache.druid.segment.column.ColumnCapabilitiesImpl; import org.apache.druid.segment.column.ColumnHolder; import org.apache.druid.segment.column.ColumnType; import org.apache.druid.segment.column.RowSignature; @@ -172,6 +173,13 @@ public void testFilterListFilteredVirtualColumnAllowIndex() BitmapFactory bitmapFactory = EasyMock.createMock(BitmapFactory.class); EasyMock.expect(selector.getColumnHolder(COLUMN_NAME)).andReturn(holder).atLeastOnce(); + EasyMock.expect(selector.getColumnCapabilities(COLUMN_NAME)) + .andReturn(new ColumnCapabilitiesImpl().setType(ColumnType.STRING) + .setDictionaryEncoded(true) + .setDictionaryValuesUnique(true) + .setDictionaryValuesSorted(true) + .setHasBitmapIndexes(true) + ).anyTimes(); EasyMock.expect(holder.getBitmapIndex()).andReturn(index).atLeastOnce(); @@ -227,7 +235,13 @@ public void testFilterListFilteredVirtualColumnDenyIndex() BitmapFactory bitmapFactory = EasyMock.createMock(BitmapFactory.class); EasyMock.expect(selector.getColumnHolder(COLUMN_NAME)).andReturn(holder).atLeastOnce(); - + EasyMock.expect(selector.getColumnCapabilities(COLUMN_NAME)) + .andReturn(new ColumnCapabilitiesImpl().setType(ColumnType.STRING) + .setDictionaryEncoded(true) + .setDictionaryValuesUnique(true) + .setDictionaryValuesSorted(true) + .setHasBitmapIndexes(true) + ).anyTimes(); EasyMock.expect(holder.getBitmapIndex()).andReturn(index).atLeastOnce(); EasyMock.expect(index.getCardinality()).andReturn(3).atLeastOnce(); From 0fc769b162c6daad3d416aeeadc6d14659207e14 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Tue, 8 Mar 2022 20:57:55 -0800 Subject: [PATCH 07/11] cache --- .../query/dimension/PredicateFilteredDimensionSelector.java | 3 ++- .../src/main/java/org/apache/druid/segment/filter/Filters.java | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/processing/src/main/java/org/apache/druid/query/dimension/PredicateFilteredDimensionSelector.java b/processing/src/main/java/org/apache/druid/query/dimension/PredicateFilteredDimensionSelector.java index bf02bc3abe30..3aaf66d2485d 100644 --- a/processing/src/main/java/org/apache/druid/query/dimension/PredicateFilteredDimensionSelector.java +++ b/processing/src/main/java/org/apache/druid/query/dimension/PredicateFilteredDimensionSelector.java @@ -64,6 +64,7 @@ public IndexedInts getRow() @Override public ValueMatcher makeValueMatcher(final String value) { + final boolean matchNull = predicate.apply(null); return new ValueMatcher() { @Override @@ -82,7 +83,7 @@ public boolean matches() } } // null should match empty rows in multi-value columns if predicate matches null - return nullRow && value == null && predicate.apply(null); + return nullRow && value == null && matchNull; } @Override diff --git a/processing/src/main/java/org/apache/druid/segment/filter/Filters.java b/processing/src/main/java/org/apache/druid/segment/filter/Filters.java index abecb0d48a5b..5eaba41efee9 100644 --- a/processing/src/main/java/org/apache/druid/segment/filter/Filters.java +++ b/processing/src/main/java/org/apache/druid/segment/filter/Filters.java @@ -246,6 +246,7 @@ public static Iterable matchPredicateNoUnion( // Missing dimension -> match all rows if the predicate matches null; match no rows otherwise try (final CloseableIndexed dimValues = selector.getDimensionValues(dimension)) { if (dimValues == null || dimValues.size() == 0) { + return ImmutableList.of(predicate.apply(null) ? allTrue(selector) : allFalse(selector)); } From 2e47a29809c7ed3cea412abecaee77423a921a25 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Wed, 9 Mar 2022 01:17:52 -0800 Subject: [PATCH 08/11] fix tests --- .../java/org/apache/druid/segment/virtual/ExpressionPlan.java | 1 + .../segment/join/BaseHashJoinSegmentStorageAdapterTest.java | 3 ++- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionPlan.java b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionPlan.java index b0b57e09178c..86dbbd782670 100644 --- a/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionPlan.java +++ b/processing/src/main/java/org/apache/druid/segment/virtual/ExpressionPlan.java @@ -270,6 +270,7 @@ public ColumnCapabilities inferColumnCapabilities(@Nullable ColumnType outputTyp .setType(ColumnType.STRING) .setDictionaryValuesSorted(false) .setDictionaryValuesUnique(false) + .setHasBitmapIndexes(false) .setHasNulls(true); } } diff --git a/processing/src/test/java/org/apache/druid/segment/join/BaseHashJoinSegmentStorageAdapterTest.java b/processing/src/test/java/org/apache/druid/segment/join/BaseHashJoinSegmentStorageAdapterTest.java index fed718de5299..d1de10fdaa54 100644 --- a/processing/src/test/java/org/apache/druid/segment/join/BaseHashJoinSegmentStorageAdapterTest.java +++ b/processing/src/test/java/org/apache/druid/segment/join/BaseHashJoinSegmentStorageAdapterTest.java @@ -39,6 +39,7 @@ import org.apache.druid.segment.join.table.IndexedTable; import org.apache.druid.segment.join.table.IndexedTableJoinable; import org.apache.druid.segment.virtual.ExpressionVirtualColumn; +import org.apache.druid.testing.InitializedNullHandlingTest; import org.apache.druid.timeline.SegmentId; import org.junit.After; import org.junit.Assert; @@ -52,7 +53,7 @@ import java.util.Collections; import java.util.List; -public class BaseHashJoinSegmentStorageAdapterTest +public class BaseHashJoinSegmentStorageAdapterTest extends InitializedNullHandlingTest { public static JoinFilterRewriteConfig DEFAULT_JOIN_FILTER_REWRITE_CONFIG = new JoinFilterRewriteConfig( true, From 758c9106bfd92f641a952a644c9cd4464d697ff0 Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Wed, 9 Mar 2022 01:23:43 -0800 Subject: [PATCH 09/11] move indexOf into GenericIndexed --- .../druid/segment/data/GenericIndexed.java | 25 +++++++++---- .../apache/druid/segment/data/Indexed.java | 35 ------------------- 2 files changed, 19 insertions(+), 41 deletions(-) diff --git a/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java b/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java index 2c6ea106157b..0d43ec30da6c 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java +++ b/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java @@ -346,16 +346,29 @@ public T get(int index) */ @Override public int indexOf(@Nullable T value) - { - return indexOf(this, value); - } - - private int indexOf(Indexed indexed, @Nullable T value) { if (!allowReverseLookup) { throw new UnsupportedOperationException("Reverse lookup not allowed."); } - return Indexed.indexOf(indexed, size, strategy, value); + int minIndex = 0; + int maxIndex = size - 1; + while (minIndex <= maxIndex) { + int currIndex = (minIndex + maxIndex) >>> 1; + + T currValue = get(currIndex); + int comparison = strategy.compare(currValue, value); + if (comparison == 0) { + return currIndex; + } + + if (comparison < 0) { + minIndex = currIndex + 1; + } else { + maxIndex = currIndex - 1; + } + } + + return -(minIndex + 1); } @Override diff --git a/processing/src/main/java/org/apache/druid/segment/data/Indexed.java b/processing/src/main/java/org/apache/druid/segment/data/Indexed.java index 8e0cb295411d..b99fc79266a3 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/Indexed.java +++ b/processing/src/main/java/org/apache/druid/segment/data/Indexed.java @@ -24,7 +24,6 @@ import org.apache.druid.query.monomorphicprocessing.HotLoopCallee; import javax.annotation.Nullable; -import java.util.Comparator; /** * Indexed is a fixed-size, immutable, indexed set of values which allows @@ -54,40 +53,6 @@ public interface Indexed extends Iterable, HotLoopCallee */ int indexOf(@Nullable T value); - /** - * Returns the index of "value" in some object in the {@link Indexed}, or (-(insertion point) - 1) if the value is - * not present, in the manner of Arrays.binarySearch. - * - * This is used by {@link GenericIndexed} to strengthen the contract of {@link #indexOf(Object)}, which only - * guarantees that values-not-found will return some negative number. - * - * @param value value to search for - * - * @return index of value, or negative number equal to (-(insertion point) - 1). - */ - static int indexOf(Indexed indexed, int size, Comparator comparator, @Nullable T value) - { - int minIndex = 0; - int maxIndex = size - 1; - while (minIndex <= maxIndex) { - int currIndex = (minIndex + maxIndex) >>> 1; - - T currValue = indexed.get(currIndex); - int comparison = comparator.compare(currValue, value); - if (comparison == 0) { - return currIndex; - } - - if (comparison < 0) { - minIndex = currIndex + 1; - } else { - maxIndex = currIndex - 1; - } - } - - return -(minIndex + 1); - } - @FunctionalInterface interface IndexedGetter { From f15746c005e5ae7d6da6633b2ae885ac478b865b Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Wed, 9 Mar 2022 02:58:06 -0800 Subject: [PATCH 10/11] oops --- .../java/org/apache/druid/segment/data/GenericIndexed.java | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java b/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java index 0d43ec30da6c..982f7b076724 100644 --- a/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java +++ b/processing/src/main/java/org/apache/druid/segment/data/GenericIndexed.java @@ -350,12 +350,17 @@ public int indexOf(@Nullable T value) if (!allowReverseLookup) { throw new UnsupportedOperationException("Reverse lookup not allowed."); } + return indexOf(this, value); + } + + private int indexOf(Indexed indexed, @Nullable T value) + { int minIndex = 0; int maxIndex = size - 1; while (minIndex <= maxIndex) { int currIndex = (minIndex + maxIndex) >>> 1; - T currValue = get(currIndex); + T currValue = indexed.get(currIndex); int comparison = strategy.compare(currValue, value); if (comparison == 0) { return currIndex; From e0b5adaf3c91c6ada4b6fef8ac8e9c59868d290c Mon Sep 17 00:00:00 2001 From: Clint Wylie Date: Wed, 9 Mar 2022 04:23:41 -0800 Subject: [PATCH 11/11] fix tests --- .../apache/druid/segment/virtual/ExpressionPlannerTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionPlannerTest.java b/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionPlannerTest.java index 7966e9a56538..ad7f1daab1f4 100644 --- a/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionPlannerTest.java +++ b/processing/src/test/java/org/apache/druid/segment/virtual/ExpressionPlannerTest.java @@ -369,7 +369,7 @@ public void testScalarStringDictionaryEncoded() Assert.assertFalse(inferred.areDictionaryValuesSorted().isMaybeTrue()); Assert.assertFalse(inferred.areDictionaryValuesUnique().isMaybeTrue()); Assert.assertFalse(inferred.hasMultipleValues().isMaybeTrue()); - Assert.assertTrue(inferred.hasBitmapIndexes()); + Assert.assertFalse(inferred.hasBitmapIndexes()); Assert.assertFalse(inferred.hasSpatialIndexes()); // multiple input columns @@ -463,7 +463,7 @@ public void testMultiValueStringDictionaryEncoded() Assert.assertFalse(inferred.areDictionaryValuesSorted().isMaybeTrue()); Assert.assertFalse(inferred.areDictionaryValuesUnique().isMaybeTrue()); Assert.assertTrue(inferred.hasMultipleValues().isTrue()); - Assert.assertTrue(inferred.hasBitmapIndexes()); + Assert.assertFalse(inferred.hasBitmapIndexes()); Assert.assertFalse(inferred.hasSpatialIndexes()); thePlan = plan("concat(scalar_string, multi_dictionary_string_nonunique)");