cecemei commented on code in PR #17055:
URL: https://github.com/apache/druid/pull/17055#discussion_r1759296274
##########
processing/src/main/java/org/apache/druid/query/filter/Filter.java:
##########
@@ -31,12 +31,27 @@
import org.apache.druid.segment.vector.VectorColumnSelectorFactory;
import javax.annotation.Nullable;
+import java.util.LinkedHashSet;
import java.util.Map;
import java.util.Set;
@SubclassesMustOverrideEqualsAndHashCode
public interface Filter
{
+ default String getFilterString()
+ {
+ return toString();
+ }
+
+ /**
+ * Returns a LinkedHashSet of all child filters for this filter with no
duplicates.
+ * <p>The ordering of child filters is important in some cases,
e.x.short-curcuiting.</p>
+ */
+ default LinkedHashSet<Filter> getFilters()
+ {
+ return new LinkedHashSet<>();
+ }
Review Comment:
true - i'm planning to make it return an `ImmutableSortedSet` which returns
a static empty set just by `ImmutableSortedSet.of()`. Also it would indicate
that child filters are not modifiable once constructed. would add more javadoc
indicating only AndFilter/OrFilter have child filters. The `BooleanFilter`
class is not a good name IMO, boolean would make sense for every type of
filter?
##########
processing/src/main/java/org/apache/druid/segment/QueryableIndexCursorHolder.java:
##########
@@ -732,8 +731,7 @@ private static FilterBundle makeFilterBundle(
return null;
}
final long bitmapConstructionStartNs = System.nanoTime();
- final FilterBundle filterBundle = filter.makeFilterBundle(
- bitmapIndexSelector,
+ final FilterBundle filterBundle = new FilterBundle.Builder(filter,
bitmapIndexSelector).build(
Review Comment:
would make the change to add the flag - thanks for the suggestion!
##########
processing/src/main/java/org/apache/druid/query/filter/FilterBundle.java:
##########
@@ -151,6 +151,91 @@ public interface MatcherBundle
boolean canVectorize();
}
+ /**
+ * Wraps info needed to build a {@link FilterBundle}, and provides an
estimated compute cost for
+ * {@link BitmapColumnIndex#computeBitmapResult}.
+ */
+ public static class Builder
+ {
+ private final Filter filter;
+ private final ColumnIndexSelector columnIndexSelector;
+ @Nullable
+ private final BitmapColumnIndex bitmapColumnIndex;
+ private final List<FilterBundle.Builder> childBuilders;
+ private final int estimatedComputeCost;
+
+ public Builder(Filter filter, ColumnIndexSelector columnIndexSelector)
+ {
+ this.filter = filter;
+ this.columnIndexSelector = columnIndexSelector;
+ this.bitmapColumnIndex =
filter.getBitmapColumnIndex(columnIndexSelector);
+ // Construct Builder instances for all child filters recursively.
+ this.childBuilders = new ArrayList<>(filter.getFilters().size());
+ for (Filter childFilter : filter.getFilters()) {
+ childBuilders.add(new FilterBundle.Builder(childFilter,
columnIndexSelector));
+ }
+ // Sort child builders by cost in ASCENDING order, should be stable by
default.
+
this.childBuilders.sort(Comparator.comparingInt(FilterBundle.Builder::getEstimatedComputeCost));
+ this.estimatedComputeCost = calculateEstimatedComputeCost();
+ }
+
+ private int calculateEstimatedComputeCost()
+ {
+ if (this.bitmapColumnIndex == null) {
+ return Integer.MAX_VALUE;
+ }
+ int cost = this.bitmapColumnIndex.estimatedComputeCost();
+ if (cost == Integer.MAX_VALUE) {
+ return Integer.MAX_VALUE;
+ }
+
+ for (FilterBundle.Builder childBuilder : this.childBuilders) {
+ int childCost = childBuilder.getEstimatedComputeCost();
+ if (childCost >= Integer.MAX_VALUE - cost) {
+ return Integer.MAX_VALUE;
+ }
+ cost += childCost;
+ }
+ return cost;
+ }
+
+ public Filter getFilter()
+ {
+ return this.filter;
+ }
+
+ public ColumnIndexSelector getColumnIndexSelector()
+ {
+ return this.columnIndexSelector;
+ }
+
+ @Nullable
+ public BitmapColumnIndex getBitmapColumnIndex()
+ {
+ return this.bitmapColumnIndex;
+ }
+
+ public List<FilterBundle.Builder> getChildBuilders()
+ {
+ return this.childBuilders;
+ }
+
+ public int getEstimatedComputeCost()
Review Comment:
`getIndexComputeCost` sounds good
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
