Jackie-Jiang commented on code in PR #10371:
URL: https://github.com/apache/pinot/pull/10371#discussion_r1127097984
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/FilterOperatorUtils.java:
##########
@@ -28,9 +28,218 @@
public class FilterOperatorUtils {
+
+ private static Implementation _instance = new DefaultImplementation();
+
private FilterOperatorUtils() {
}
+ public static void setImplementation(Implementation newImplementation) {
+ _instance = newImplementation;
+ }
+
+ public interface Implementation {
+
+ /**
+ * Returns the leaf filter operator (i.e. not {@link AndFilterOperator} or
{@link OrFilterOperator}).
+ */
+ BaseFilterOperator getLeafFilterOperator(PredicateEvaluator
predicateEvaluator, DataSource dataSource,
+ int numDocs, boolean nullHandlingEnabled);
+
+ /**
+ * Returns the AND filter operator or equivalent filter operator.
+ */
+ BaseFilterOperator getAndFilterOperator(QueryContext queryContext,
+ List<BaseFilterOperator> filterOperators, int numDocs);
+
+ /**
+ * Returns the OR filter operator or equivalent filter operator.
+ */
+ BaseFilterOperator getOrFilterOperator(QueryContext queryContext,
+ List<BaseFilterOperator> filterOperators, int numDocs);
+
+ /**
+ * Returns the NOT filter operator or equivalent filter operator.
+ */
+ BaseFilterOperator getNotFilterOperator(QueryContext queryContext,
BaseFilterOperator filterOperator,
+ int numDocs);
+
+ /**
+ * For AND filter operator, reorders its child filter operators based on
the their cost and puts the ones with
+ * inverted index first in order to reduce the number of documents to be
processed.
+ * <p>Special filter operators such as {@link MatchAllFilterOperator} and
{@link EmptyFilterOperator} should be
+ * removed from the list before calling this method.
+ */
+ void reorderAndFilterChildOperators(QueryContext queryContext,
Review Comment:
This method should not be part of the interface because it is implementation
detail for `getAndFilterOperator()`
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/FilterOperatorUtils.java:
##########
@@ -28,9 +28,218 @@
public class FilterOperatorUtils {
+
+ private static Implementation _instance = new DefaultImplementation();
+
private FilterOperatorUtils() {
}
+ public static void setImplementation(Implementation newImplementation) {
+ _instance = newImplementation;
+ }
+
+ public interface Implementation {
+
+ /**
+ * Returns the leaf filter operator (i.e. not {@link AndFilterOperator} or
{@link OrFilterOperator}).
+ */
+ BaseFilterOperator getLeafFilterOperator(PredicateEvaluator
predicateEvaluator, DataSource dataSource,
+ int numDocs, boolean nullHandlingEnabled);
+
+ /**
+ * Returns the AND filter operator or equivalent filter operator.
+ */
+ BaseFilterOperator getAndFilterOperator(QueryContext queryContext,
+ List<BaseFilterOperator> filterOperators, int numDocs);
+
+ /**
+ * Returns the OR filter operator or equivalent filter operator.
+ */
+ BaseFilterOperator getOrFilterOperator(QueryContext queryContext,
+ List<BaseFilterOperator> filterOperators, int numDocs);
+
+ /**
+ * Returns the NOT filter operator or equivalent filter operator.
+ */
+ BaseFilterOperator getNotFilterOperator(QueryContext queryContext,
BaseFilterOperator filterOperator,
+ int numDocs);
+
+ /**
+ * For AND filter operator, reorders its child filter operators based on
the their cost and puts the ones with
+ * inverted index first in order to reduce the number of documents to be
processed.
+ * <p>Special filter operators such as {@link MatchAllFilterOperator} and
{@link EmptyFilterOperator} should be
+ * removed from the list before calling this method.
+ */
+ void reorderAndFilterChildOperators(QueryContext queryContext,
+ List<BaseFilterOperator> filterOperators);
+ }
+
+ public static class DefaultImplementation implements Implementation {
+ @Override
+ public BaseFilterOperator getLeafFilterOperator(PredicateEvaluator
predicateEvaluator, DataSource dataSource,
+ int numDocs, boolean nullHandlingEnabled) {
+ if (predicateEvaluator.isAlwaysFalse()) {
+ return EmptyFilterOperator.getInstance();
+ } else if (predicateEvaluator.isAlwaysTrue()) {
+ return new MatchAllFilterOperator(numDocs);
+ }
+
+ // Currently sorted index based filtering is supported only for
+ // dictionary encoded columns. The on-disk segment metadata
+ // will indicate if the column is sorted or not regardless of
+ // whether it is raw or dictionary encoded. Here when creating
+ // the filter operator, we need to make sure that sort filter
+ // operator is used only if the column is sorted and has dictionary.
+ Predicate.Type predicateType = predicateEvaluator.getPredicateType();
+ if (predicateType == Predicate.Type.RANGE) {
+ if (dataSource.getDataSourceMetadata().isSorted() &&
dataSource.getDictionary() != null) {
+ return new SortedIndexBasedFilterOperator(predicateEvaluator,
dataSource, numDocs);
+ }
+ if (RangeIndexBasedFilterOperator.canEvaluate(predicateEvaluator,
dataSource)) {
+ return new RangeIndexBasedFilterOperator(predicateEvaluator,
dataSource, numDocs);
+ }
+ return new ScanBasedFilterOperator(predicateEvaluator, dataSource,
numDocs, nullHandlingEnabled);
+ } else if (predicateType == Predicate.Type.REGEXP_LIKE) {
+ if (dataSource.getFSTIndex() != null &&
dataSource.getDataSourceMetadata().isSorted()) {
+ return new SortedIndexBasedFilterOperator(predicateEvaluator,
dataSource, numDocs);
+ }
+ if (dataSource.getFSTIndex() != null && dataSource.getInvertedIndex()
!= null) {
+ return new BitmapBasedFilterOperator(predicateEvaluator, dataSource,
numDocs);
+ }
+ return new ScanBasedFilterOperator(predicateEvaluator, dataSource,
numDocs, nullHandlingEnabled);
+ } else {
+ if (dataSource.getDataSourceMetadata().isSorted() &&
dataSource.getDictionary() != null) {
+ return new SortedIndexBasedFilterOperator(predicateEvaluator,
dataSource, numDocs);
+ }
+ if (dataSource.getInvertedIndex() != null) {
+ return new BitmapBasedFilterOperator(predicateEvaluator, dataSource,
numDocs);
+ }
+ if (RangeIndexBasedFilterOperator.canEvaluate(predicateEvaluator,
dataSource)) {
+ return new RangeIndexBasedFilterOperator(predicateEvaluator,
dataSource, numDocs);
+ }
+ return new ScanBasedFilterOperator(predicateEvaluator, dataSource,
numDocs, nullHandlingEnabled);
+ }
+ }
+
+ @Override
+ public BaseFilterOperator getAndFilterOperator(QueryContext queryContext,
List<BaseFilterOperator> filterOperators,
+ int numDocs) {
+ List<BaseFilterOperator> childFilterOperators = new
ArrayList<>(filterOperators.size());
+ for (BaseFilterOperator filterOperator : filterOperators) {
+ if (filterOperator.isResultEmpty()) {
+ return EmptyFilterOperator.getInstance();
+ } else if (!filterOperator.isResultMatchingAll()) {
+ childFilterOperators.add(filterOperator);
+ }
+ }
+ int numChildFilterOperators = childFilterOperators.size();
+ if (numChildFilterOperators == 0) {
+ // Return match all filter operator if all child filter operators
match all records
+ return new MatchAllFilterOperator(numDocs);
+ } else if (numChildFilterOperators == 1) {
+ // Return the child filter operator if only one left
+ return childFilterOperators.get(0);
+ } else {
+ // Return the AND filter operator with re-ordered child filter
operators
+ reorderAndFilterChildOperators(queryContext, childFilterOperators);
+ return new AndFilterOperator(childFilterOperators,
queryContext.getQueryOptions());
+ }
+ }
+
+ @Override
+ public BaseFilterOperator getOrFilterOperator(QueryContext queryContext,
+ List<BaseFilterOperator> filterOperators, int numDocs) {
+ List<BaseFilterOperator> childFilterOperators = new
ArrayList<>(filterOperators.size());
+ for (BaseFilterOperator filterOperator : filterOperators) {
+ if (filterOperator.isResultMatchingAll()) {
+ return new MatchAllFilterOperator(numDocs);
+ } else if (!filterOperator.isResultEmpty()) {
+ childFilterOperators.add(filterOperator);
+ }
+ }
+ int numChildFilterOperators = childFilterOperators.size();
+ if (numChildFilterOperators == 0) {
+ // Return empty filter operator if all child filter operators's result
is empty
+ return EmptyFilterOperator.getInstance();
+ } else if (numChildFilterOperators == 1) {
+ // Return the child filter operator if only one left
+ return childFilterOperators.get(0);
+ } else {
+ // Return the OR filter operator with child filter operators
+ return new OrFilterOperator(childFilterOperators, numDocs);
+ }
+ }
+
+ @Override
+ public BaseFilterOperator getNotFilterOperator(QueryContext queryContext,
BaseFilterOperator filterOperator,
+ int numDocs) {
+ if (filterOperator.isResultMatchingAll()) {
+ return EmptyFilterOperator.getInstance();
+ } else if (filterOperator.isResultEmpty()) {
+ return new MatchAllFilterOperator(numDocs);
+ }
+
+ return new NotFilterOperator(filterOperator, numDocs);
+ }
+
+ @Override
+ public void reorderAndFilterChildOperators(QueryContext queryContext,
List<BaseFilterOperator> filterOperators) {
+ filterOperators.sort(new Comparator<BaseFilterOperator>() {
+ @Override
+ public int compare(BaseFilterOperator o1, BaseFilterOperator o2) {
+ return getPriority(o1) - getPriority(o2);
+ }
+
+ int getPriority(BaseFilterOperator filterOperator) {
+ if (filterOperator instanceof SortedIndexBasedFilterOperator) {
+ return 0;
+ }
+ if (filterOperator instanceof BitmapBasedFilterOperator) {
+ return 1;
+ }
+ if (filterOperator instanceof RangeIndexBasedFilterOperator
+ || filterOperator instanceof TextContainsFilterOperator
+ || filterOperator instanceof TextMatchFilterOperator ||
filterOperator instanceof JsonMatchFilterOperator
+ || filterOperator instanceof H3IndexFilterOperator
+ || filterOperator instanceof H3InclusionIndexFilterOperator) {
+ return 2;
+ }
+ if (filterOperator instanceof AndFilterOperator) {
+ return 3;
+ }
+ if (filterOperator instanceof OrFilterOperator) {
+ return 4;
+ }
+ if (filterOperator instanceof NotFilterOperator) {
+ return getPriority(((NotFilterOperator)
filterOperator).getChildFilterOperator());
+ }
+ if (filterOperator instanceof ScanBasedFilterOperator) {
+ return getScanBasedFilterPriority(queryContext,
(ScanBasedFilterOperator) filterOperator, 5);
+ }
+ if (filterOperator instanceof ExpressionFilterOperator) {
+ return 10;
+ }
+ throw new
IllegalStateException(filterOperator.getClass().getSimpleName()
+ + " should not be reordered, remove it from the list before
calling this method");
+ }
+ });
+ }
+
+ public static int getScanBasedFilterPriority(QueryContext queryContext,
Review Comment:
I don't get why we cannot keep this method as `protected`. IMO both
`reorderAndFilterChildOperators()` and `getScanBasedFilterPriority()` should be
protected
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
