add mechanism to control filter optimization in historical query processing #8209

clintropolis · 2019-07-31T23:56:57Z

Description

This PR introduces the ability for most DimFilter implementations to control whether or not QueryableIndexStorageAdapter will use a bitmap index while processing queries. I consider this to be laying the groundwork for allowing QueryableIndexStorageAdapter the opportunity for more sophisticated decision making when deciding when to use bitmap indexes or not, but the first step of this is to enable easy experimentation so that we can shake out what good "automatic" decisions might be.

This PR is related to #3878, and replaces #6633, which was a more targeted attempt at something similar.

This is done by adding a new type, FilterTuning, which most DimFilter implementations will accept if specified in the filter json, which is supplied to all Filter implementations. The Filter interface itself has been modified with a few new methods:

getRequiredColumns which is analog to DimFilter.getRequiredColumns
shouldUseIndex which QueryableIndexStorageAdapter and FilteringOffset call now instead of supportsBitmapIndex directly, to determine if a bitmap index should be used.

And a static default implementation of supportsBitmapIndex supplied by Filters that currently all Filter implementations are using.

FilterTuning currently has 3 properties:

property	type	description	default
`useIndex`	boolean	allow manually skipping bitmap index even if it exists	true
`maxCardinalityToUseBitmapIndex`	integer	allow using a bitmap index only if cardinality is under max value	`Integer.MAX_VALUE`
`minCardinalityToUseBitmapIndex`	integer	allow using bitmap index only if cardinality is over min value (this is probably useless, but i wanted levers to pull to see what happens).	0

Since this is an advanced feature intended primarily for experimentation, I am leaving it undocumented for now.

This PR has:

been self-reviewed.
added documentation for new or modified features or behaviors.
added Javadocs for most classes and all non-trivial methods. Linked related entities via Javadoc links.
added unit tests or modified existing tests to cover new code paths.

For reviewers: the key changed/added classes in this PR

Filter
QueryableIndexStorageAdapter
FilterTuning
FilterPartitionTest

…l query processing

gianm · 2019-08-02T20:59:32Z

the first step of this is to enable easy experimentation so that we can shake out what good "automatic" decisions might be.

👍

Since this is an advanced feature intended primarily for experimentation, I am leaving it undocumented for now.

I haven't read through the patch yet, so if it's not already, could you make sure this is in the javadoc for the major classes involved (probably just FilterTuning)? Something like: intentionally undocumented in user-facing docs because at this time the feature is meant for experimentation. And a comment that we'd like to document this (or whatever it turns into) in a future release.

gianm · 2019-08-02T21:05:45Z

...ions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/filter/BloomDimFilter.java


  @JsonCreator
  public BloomDimFilter(
      @JsonProperty("dimension") String dimension,
      @JsonProperty("bloomKFilter") BloomKFilterHolder bloomKFilterHolder,
-      @JsonProperty("extractionFn") ExtractionFn extractionFn
+      @JsonProperty("extractionFn") ExtractionFn extractionFn,
+      @JsonProperty("filterTuning") FilterTuning filterTuning


Are you missing a @JsonProperty getter for this?

added (also a couple of other missing this)

gianm · 2019-08-02T21:07:05Z

processing/src/main/java/org/apache/druid/query/filter/BooleanFilter.java

@@ -48,4 +50,20 @@ ValueMatcher makeMatcher(
      ColumnSelectorFactory columnSelectorFactory,
      RowOffsetMatcherFactory rowOffsetMatcherFactory
  );
+
+  @Override
+  default FilterTuning getManualTuning()


Please annotate with @Nullable.

removed entirely

gianm · 2019-08-02T22:25:32Z

processing/src/main/java/org/apache/druid/query/filter/FilterTuning.java

+    return new FilterTuning(useIndex, 0, Integer.MAX_VALUE);
+  }
+
+  private final Boolean useIndex;


Please mark these @Nullable too, just like the constructor parameters.

Or, apply the defaults you have in getUseIndex(), etc, in the constructors and make these fields primitives.

applied defaults in constructor

gianm · 2019-08-02T22:27:28Z

processing/src/main/java/org/apache/druid/query/filter/BoundDimFilter.java

@@ -152,6 +171,12 @@ public StringComparator getOrdering()
    return ordering;
  }

+  @JsonProperty


Please include @JsonInclude(JsonInclude.Include.NON_NULL) here, and in all other places FilterTuning appears as a json property, to avoid polluting jsonifications of filters with this experimental feature.

added to DimFilter getters

gianm · 2019-08-02T22:31:13Z

processing/src/main/java/org/apache/druid/query/filter/Filter.java

+
+  /**
+   * "Manual" {@link FilterTuning} to allow explicit control of filter optimization behavior. This will likely be
+   * supplied by the {@link DimFilter} that is creating this {@link Filter}


Is it intended that anything outside of computeTuning call this method? If not, please update the javadocs to tell people not to call it. Maybe mention that you would have made it protected, except that Filter is an interface, so you can't do that.

this method has been removed

gianm · 2019-08-02T22:32:20Z

processing/src/main/java/org/apache/druid/query/filter/Filter.java

+  FilterTuning getManualTuning();
+
+  /**
+   * This method allows a filter to automatically compute a {@link FilterTuning} based on information it can gather


Similar comment to computeTuning, it seems like this might only be called by shouldUseIndex.

Btw, what do you think about not having getManualTuning and computeTuning, and instead having a static function in Filters that provides a useful default impl for shouldUseIndex? So most Filters would do something like:

@Override boolean shouldUseIndex(BitmapIndexSelector selector) { return Filters.shouldUseIndex(this, selector, filterTuning); }

It's not really more work or lines of code than overriding getManualTuning, which they'd have to do anyway with the current design. And it cuts 3 interface methods down to 1.

It would also be less spooky in terms of action-at-a-distance. It's nice to have methods defined in concrete classes rather than inherited from supertypes, since it makes it clearer to readers of the one class what is going on (without having to flip around as much between concrete impl and supertype).

This makes sense, I think the current structure is a an artifact of my original version of this where the work was done by QueryableIndexStorageAdapter, will refactor

gianm · 2019-08-02T22:37:49Z

processing/src/main/java/org/apache/druid/query/filter/FilterTuning.java

+  @JsonProperty
+  public Boolean getUseIndex()
+  {
+    return useIndex != null ? useIndex : true;


This'll never be null, so it'd be better to return a primitive.

Same comment for the other two methods.

gianm · 2019-08-02T22:52:06Z

processing/src/main/java/org/apache/druid/query/filter/IntervalDimFilter.java

    this.intervalLongs = makeIntervalLongs();
    this.convertedFilter = new OrDimFilter(makeBoundDimFilters());
  }

+  @VisibleForTesting
+  public IntervalDimFilter(


In general I assume you left the old constructors in to avoid a bunch of test churn. I think that's fine but could you please double check that they aren't being used in production code? (You got the one in InDimFilter.optimize that I was thinking of when I wrote this, but there are potentially others)

there were a few being created in calcite stuff, adjusted them to set null for tuning.

gianm · 2019-08-02T22:55:30Z

processing/src/main/java/org/apache/druid/segment/filter/BoundFilter.java


 public class BoundFilter implements Filter
 {
  private final BoundDimFilter boundDimFilter;
  private final Comparator<String> comparator;
  private final ExtractionFn extractionFn;
+  private final FilterTuning manualFilterTuning;


Sometimes the field is called manualFilterTuning and sometimes it's called filterTuning. Could you please pick one?

I believe it is consistently called filterTuning on DimFilter and manualFilterTuning on Filter implementations. This distinction is because everything in DimFilter is manual since it was provided by the json query, where as this is explictly the user set value on Filter implementations that was supplied by the DimFilter.

renamed all to filterTuning

gianm · 2019-08-02T22:56:04Z

processing/src/main/java/org/apache/druid/segment/filter/ColumnComparisonFilter.java

+  @Override
+  public Set<String> getRequiredColumns()
+  {
+    return dimensions.stream().map(dim -> dim.getDimension()).collect(Collectors.toSet());


map(DimensionSpec::getDimension)?

gianm · 2019-08-04T00:38:54Z

~~The CI issues look legit, seems like some tests don't compile?~~

Oh wait, sorry, my browser had an old version cached. Nevermind.

leventov

Reviewed until ExpressionDimFilter.java

leventov · 2019-08-05T16:36:36Z

...ions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/filter/BloomDimFilter.java

@@ -42,12 +45,14 @@
  private final BloomKFilter bloomKFilter;
  private final HashCode hash;
  private final ExtractionFn extractionFn;
+  private final FilterTuning filterTuning;


Should be @Nullable, along with the corresponding constructor parameter

I think I've updated all DimFilter implementations to have the appropriate things annotated with @Nullable

leventov · 2019-08-05T16:38:25Z

...ions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/filter/BloomDimFilter.java

@@ -174,6 +191,13 @@ public ExtractionFn getExtractionFn()
    return extractionFn;
  }

+  @JsonInclude(JsonInclude.Include.NON_NULL)
+  @JsonProperty
+  public FilterTuning getFilterTuning()


This method should be @Nullable

leventov · 2019-08-05T16:40:08Z

...ions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/filter/BloomDimFilter.java

    BloomDimFilter that = (BloomDimFilter) o;
+    return dimension.equals(that.dimension) &&
+           hash.equals(that.hash) &&
+           Objects.equals(extractionFn, that.extractionFn) &&


Everything about extractionFn: the field, constructor parameter, getter should be @Nullable

leventov · 2019-08-05T16:41:45Z

...om-filter/src/main/java/org/apache/druid/query/filter/sql/BloomFilterOperatorConversion.java

@@ -100,7 +100,8 @@ public DimFilter toDruidFilter(
      return new BloomDimFilter(
          druidExpression.getSimpleExtraction().getColumn(),
          holder,
-          druidExpression.getSimpleExtraction().getExtractionFn()
+          druidExpression.getSimpleExtraction().getExtractionFn(),


Constructor annotated @VisibleForTesting doesn't mean it can't be used in production code.

Yeah, this is true, but I was trying to avoid using these constructors in production code to minimize the chance of errors and make things more obvious.

I initially had it more like you are suggesting and was using some of the 'test' constructors where we were always passing null, but this comment suggested the way it is now and I agree, I think the production code explicitly passing in null for the filterTuning parameter makes it less ambiguous about if not having a tuning is intentional or not, without requiring a comment.

I think it's anyway strange that there is a chaining constructor and any code is not using an existing constructor with fewer parameters.

I think it would be better to create static factory methods in this situation. Some of them may have "InTest" suffix to strongly indicate that they are not for prod code.

I agree, but the reason I have a second set of constructors at all was so I didn't have to change hundreds of lines of test code to add a null parameter, so making a static method would sort of defeat the purpose of that because the lines would change anyway.

I do think this is worth doing, I'll add this to the ticket i create about refactoring DimFilter to also add useful test filter creation static methods, so we can also eliminate passing in null everywhere for filters without extractionFn.

mentioned making static test object creators to decouple test filters from json schema in #8256

leventov · 2019-08-05T16:41:52Z

...om-filter/src/main/java/org/apache/druid/query/filter/sql/BloomFilterOperatorConversion.java

@@ -114,6 +115,7 @@ public DimFilter toDruidFilter(
      return new BloomDimFilter(
          virtualColumn.getOutputName(),
          holder,
+          null,


leventov · 2019-08-05T17:11:14Z

processing/src/main/java/org/apache/druid/query/filter/ColumnComparisonDimFilter.java

@@ -40,14 +43,25 @@
  private static final Joiner COMMA_JOINER = Joiner.on(", ");

  private final List<DimensionSpec> dimensions;
+  private final FilterTuning filterTuning;


Please annotate with @Nullable everything that is needed in this class.

leventov · 2019-08-05T17:13:25Z

processing/src/main/java/org/apache/druid/query/Druids.java

@@ -204,7 +204,7 @@ public TimeseriesQueryBuilder filters(String dimensionName, String value)

    public TimeseriesQueryBuilder filters(String dimensionName, String value, String... values)
    {
-      dimFilter = new InDimFilter(dimensionName, Lists.asList(value, values), null);
+      dimFilter = new InDimFilter(dimensionName, Lists.asList(value, values), null, null);


Could call the old constructor

leventov · 2019-08-05T17:13:44Z

processing/src/main/java/org/apache/druid/query/Druids.java

@@ -361,7 +361,7 @@ public SearchQueryBuilder dataSource(DataSource d)

    public SearchQueryBuilder filters(String dimensionName, String value)
    {
-      dimFilter = new SelectorDimFilter(dimensionName, value, null);
+      dimFilter = new SelectorDimFilter(dimensionName, value, null, null);


Could call the old constructor

leventov · 2019-08-05T17:16:13Z

...ions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/filter/BloomDimFilter.java

+
+  @VisibleForTesting
+  public BloomDimFilter(
+      String dimension,


Unnecessary wrapping, also in many other newly added constructors below in this PR

leventov · 2019-08-05T17:17:31Z

processing/src/main/java/org/apache/druid/query/filter/ExpressionDimFilter.java

@@ -39,23 +41,42 @@
 {
  private final String expression;
  private final Supplier<Expr> parsed;
+  private final FilterTuning filterTuning;


Please annotate with @Nullable everything that is needed in this class.

…atting

…imFilter toString implementations

gianm

Just some small comments about some of the new javadocs. The rest LGTM.

gianm · 2019-08-06T02:38:07Z

...ions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/filter/BloomDimFilter.java

-    } else {
-      return StringUtils.format("%s = %s", dimension, hash.toString());
-    }
+    return new DimFilterToStringBuilder().appendDimension(dimension, extractionFn)


Whoa. This is cool.

gianm · 2019-08-06T03:00:54Z

processing/src/main/java/org/apache/druid/query/filter/Filter.java

+   * used. This method, by default, will consider a {@link FilterTuning} to make decisions about when to use an
+   * available index. Override this method in a {@link Filter} implementation when {@link FilterTuning} alone is not
+   * adequate for making this decision.
+   */


I'd reword slightly. This method doesn't do anything by default anymore (it did in an earlier version). Maybe cut the last two sentences, and replace with:

Implementations of this methods typically consider a {@link FilterTuning} to make decisions about when to use an available index.

Oops, forgot about this, will fix this and add javadoc for Filters.shouldUseIndex and maybe link this one to that

Thanks, these new ones look good to me.

gianm · 2019-08-06T05:23:09Z

processing/src/main/java/org/apache/druid/segment/filter/Filters.java

@@ -651,4 +652,17 @@ private static void generateAllCombinations(
      generateAllCombinations(result, andList.subList(1, andList.size()), nonAndList);
    }
  }
+
+  public static boolean shouldUseIndex(Filter filter, BitmapIndexSelector indexSelector, FilterTuning filterTuning)


Could use a bit of javadocs as to its purpose ("standard" implementation of Filter.shouldUseIndex) and how it works (returns true if the filter supports indexes and if all underlying columns match the thresholds in filterTuning). Also that filterTuning is nullable.

Thanks, this looks good. Marking the filterTuning parameter @Nullable would be helpful too.

gianm

LGTM

gianm · 2019-08-06T13:20:25Z

processing/src/main/java/org/apache/druid/segment/filter/Filters.java

@@ -651,4 +652,17 @@ private static void generateAllCombinations(
      generateAllCombinations(result, andList.subList(1, andList.size()), nonAndList);
    }
  }
+
+  public static boolean shouldUseIndex(Filter filter, BitmapIndexSelector indexSelector, FilterTuning filterTuning)


Thanks, this looks good. Marking the filterTuning parameter @Nullable would be helpful too.

leventov · 2019-08-07T17:20:30Z

processing/src/main/java/org/apache/druid/query/filter/DimFilter.java

+    /**
+     * Append dimension name OR {@link ExtractionFn#toString()} with dimension wrapped in parenthesis
+     */
+    DimFilterToStringBuilder appendDimension(String dimension, ExtractionFn extractionFn)


extractionFn should be @Nullable

leventov · 2019-08-07T17:20:48Z

processing/src/main/java/org/apache/druid/query/filter/DimFilter.java

+    /**
+     * Add filter tuning to {@link #builder} if tuning exists
+     */
+    DimFilterToStringBuilder appendFilterTuning(FilterTuning tuning)


tuning should be @Nullable

leventov · 2019-08-07T17:21:22Z

processing/src/main/java/org/apache/druid/query/filter/BoundDimFilter.java

+        extractionFn,
+        ordering,
+        filterTuning
+    );
  }

  @Override


Thanks for this refactoring

leventov · 2019-08-07T17:30:47Z

processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java

@@ -73,7 +78,8 @@
  public InDimFilter(
      @JsonProperty("dimension") String dimension,
      @JsonProperty("values") Collection<String> values,
-      @JsonProperty("extractionFn") ExtractionFn extractionFn
+      @Nullable @JsonProperty("extractionFn") ExtractionFn extractionFn,


Optional: I think it's more semantic to put @Nullable immediately before the type so that @Nullable ExtractionFn is the "extended type" of the variable.

agree, done to all occurrences I found in the codebase

leventov · 2019-08-07T17:39:45Z

processing/src/main/java/org/apache/druid/query/filter/Filter.java

+   * Determine if a filter *should* use a bitmap index based on information collected from the supplied
+   * {@link BitmapIndexSelector}. This method differs from {@link #supportsBitmapIndex(BitmapIndexSelector)} in that
+   * the former only indicates if a bitmap index is available and {@link #getBitmapIndex(BitmapIndexSelector)} may be
+   * used. Implementations of this methods typically consider a {@link FilterTuning} to make decisions about when to


Before the sentence "Implementations of this methods...", please add

* * If shouldUseFilter(selector) returns true, {@link #supportsBitmapIndex} must also return true when called with the * same selector object. *

as a separate paragraph.

leventov · 2019-08-07T17:42:33Z

processing/src/main/java/org/apache/druid/query/filter/SpatialDimFilter.java


 /**
 */
 public class SpatialDimFilter implements DimFilter
 {
  private final String dimension;
  private final Bound bound;
+  @Nullable
+  private final FilterTuning filterTuning;


There is significant repetition between all these Filter implementations. Did you research if there is a way to cross-cut the things, e. g. by somehow providing filterTuning externally, so that each Filter doesn't have to have it as a field? Or, maybe an abstract superclass could be extracted?

Same applies to extractionFn.

I totally agree here. Doing it externally seemed difficult because I wanted it to be per filter, but I did heavily consider making some sort of abstract class on top of DimFilter to handle extractionFn and filterTuning, but I think at this point would prefer to do as a follow-up so I will open an issue soon about refactoring DimFilter, and potentially merging Filter and DimFilter as well since there isn't a good reason for them to be split.

created #8256

leventov · 2019-08-07T17:45:04Z

processing/src/main/java/org/apache/druid/segment/FilteredOffset.java

@@ -56,7 +56,7 @@
          rowOffsetMatcherFactory
      );
    } else {
-      if (postFilter.supportsBitmapIndex(bitmapIndexSelector)) {
+      if (postFilter.shouldUseIndex(bitmapIndexSelector)) {


Please use consistent naming: either "supportsBitmapIndex" and "shouldUseBitmapIndex", or both names without "Bitmap" part.

renamed to shouldUseBitmapIndex

leventov · 2019-08-07T17:49:57Z

processing/src/main/java/org/apache/druid/segment/filter/AndFilter.java

@@ -125,6 +125,17 @@ public boolean canVectorizeMatcher()
    return filters.stream().allMatch(Filter::canVectorizeMatcher);
  }

+  @Override
+  public boolean shouldUseIndex(BitmapIndexSelector bitmapIndexSelector)


Since shouldUseIndex related to supportsBitmapIndex and their implementations look similar, please keep them adjacent in code. It makes easier to visually validate that there are no bugs like forgetting !.

Same applies to all other classes where shouldUseIndex is implemented, and to the definition of this method in Filter interface itself.

leventov · 2019-08-07T18:05:24Z

processing/src/test/java/org/apache/druid/segment/filter/BaseFilterTest.java

+      @Override
+      public Set<String> getRequiredColumns()
+      {
+        return null;


Collections.emptySet()

leventov · 2019-08-07T18:05:58Z

...g/src/test/java/org/apache/druid/segment/incremental/IncrementalIndexStorageAdapterTest.java

+    @Override
+    public Set<String> getRequiredColumns()
+    {
+      return null;


Collections.emptySet()

leventov · 2019-08-07T18:06:16Z

sql/src/main/java/org/apache/druid/sql/calcite/filtration/Bounds.java

@@ -127,7 +130,8 @@ public static BoundDimFilter equalTo(final BoundRefKey boundRefKey, final String
        false,
        null,
        boundRefKey.getExtractionFn(),
-        boundRefKey.getComparator()
+        boundRefKey.getComparator(),
+         null


Extra space

leventov · 2019-08-07T18:10:39Z

processing/src/main/java/org/apache/druid/query/filter/FilterTuning.java

+  }
+
+  private final boolean useIndex;
+  private final int useIndexMinCardinalityThreshold;


I don't think it's that important here to use the same prefix for all three fields. It's not very clear what does "useIndexMinCardinalityThreshold" mean. My suggestion is "minBitmapIndexCardinalityToUse".

changed to minCardinalityToUseBitmapIndex

leventov · 2019-08-07T18:12:24Z

processing/src/main/java/org/apache/druid/query/filter/FilterTuning.java

+    return new FilterTuning(filter.supportsBitmapIndex(selector), null, null);
+  }
+
+  private final boolean useIndex;


If you decide to rename Filter.shouldUseIndex() to shouldUseBitmapIndex then this field should be renamed, too, for total consistency.

leventov · 2019-08-08T19:18:20Z

processing/src/main/java/org/apache/druid/query/filter/Filter.java

-   * Indicates whether this filter can return a bitmap index for filtering, based on
-   * the information provided by the input BitmapIndexSelector.
+   * Indicates whether this filter can return a bitmap index for filtering, based on the information provided by the
+   * input BitmapIndexSelector.


Please wrap BitmapIndexSelector with {@link }

leventov · 2019-08-08T19:20:43Z

processing/src/main/java/org/apache/druid/segment/filter/ColumnComparisonFilter.java

  {
-    return Filters.shouldUseIndex(this, bitmapIndexSelector, filterTuning);
+    return Filters.shouldUseBitmapIndex(this, selector, filterTuning);


It's confusing that this method doesn't just return false while supportsBitmapIndex() does.

agree, changed

leventov · 2019-08-08T19:26:28Z

processing/src/main/java/org/apache/druid/segment/filter/Filters.java

-        return cardinality >= tuning.getUseIndexMinCardinalityThreshold()
-               && cardinality <= tuning.getUseIndexMaxCardinalityThreshold();
+        final BitmapIndex index = indexSelector.getBitmapIndex(column);
+        Preconditions.checkNotNull(index, "WTF?! column doesn't have a bitmap index");


I suggest not to use "WTF" from now on in the project. See also #8269.

clintropolis added 4 commits July 31, 2019 16:33

add support for mechanism to control filter optimization in historica…

4177dc3

…l query processing

oops

d4d6629

adjust

383eeb4

woo

cc9c9b0

clintropolis added Area - Querying Performance labels Aug 1, 2019

Merge remote-tracking branch 'upstream/master' into filter-tuning

212c9a8

javadoc

f7b8a49

gianm reviewed Aug 2, 2019

View reviewed changes

clintropolis added 6 commits August 2, 2019 17:15

review comments

ef28052

fix

922c740

default

9587425

oops

1f65d05

oof

3fbf5e6

this will fix it

6b86b32

leventov self-requested a review August 3, 2019 09:04

leventov reviewed Aug 5, 2019

View reviewed changes

clintropolis mentioned this pull request Aug 5, 2019

add method getRequiredColumns for DimFilter #5872

Merged

clintropolis added 5 commits August 5, 2019 15:05

more nullable, refactor DimFilter.getRequiredColumns to use Set, form…

bb48537

…atting

extract class DimFilterToStringBuilder with common code from custom D…

d0d7f05

…imFilter toString implementations

adjust variable naming

1aa5484

missing nullable

359cf4f

more nullable

2be81b0

gianm reviewed Aug 6, 2019

View reviewed changes

fix javadocs

a6e6a31

gianm approved these changes Aug 6, 2019

View reviewed changes

nullable

346e8fd

leventov reviewed Aug 7, 2019

View reviewed changes

leventov requested changes Aug 7, 2019

View reviewed changes

clintropolis added 5 commits August 7, 2019 14:27

Merge remote-tracking branch 'upstream/master' into filter-tuning

4402160

address review comments

b8a3cae

javadocs, precondition

cdb63a7

nullable

35098b0

rename method to be consistent

9e6a2e0

clintropolis mentioned this pull request Aug 7, 2019

refactor filters #8256

Open

leventov mentioned this pull request Aug 8, 2019

@Nullable after @JsonProperty #8268

Open

leventov requested changes Aug 8, 2019

View reviewed changes

clintropolis added 2 commits August 8, 2019 12:52

review comments

8fc735a

remove tuning from ColumnComparisonFilter/ColumnComparisonDimFilter

505b668

leventov approved these changes Aug 9, 2019

View reviewed changes

clintropolis merged commit 1054d85 into apache:master Aug 9, 2019

clintropolis deleted the filter-tuning branch August 9, 2019 23:36

clintropolis mentioned this pull request Aug 21, 2019

0.16.0-incubating release notes #8369

Closed

clintropolis added this to the 0.16.0 milestone Aug 23, 2019

clintropolis mentioned this pull request Apr 9, 2020

document useFilterCNF query context parameter #9647

Merged

add mechanism to control filter optimization in historical query processing #8209

add mechanism to control filter optimization in historical query processing #8209

Conversation

clintropolis commented Jul 31, 2019 • edited

Description

gianm commented Aug 2, 2019

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gianm commented Aug 4, 2019 • edited

leventov left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

leventov Aug 7, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clintropolis Aug 7, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gianm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gianm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

clintropolis commented Jul 31, 2019 •

edited

gianm commented Aug 4, 2019 •

edited

leventov Aug 7, 2019 •

edited

clintropolis Aug 7, 2019 •

edited