abhishekagarwal87 commented on a change in pull request #10758:
URL: https://github.com/apache/druid/pull/10758#discussion_r559635634
##########
File path: processing/src/main/java/org/apache/druid/segment/filter/Filters.java
##########
@@ -552,4 +594,96 @@ public static boolean filterMatchesNull(Filter filter)
ValueMatcher valueMatcher =
filter.makeMatcher(ALL_NULL_COLUMN_SELECTOR_FACTORY);
return valueMatcher.matches();
}
+
+ /**
+ * Flattens children of an AND, removes duplicates, and removes
literally-true filters.
+ */
+ private static List<Filter> flattenAndChildren(final List<Filter> filters)
+ {
+ final List<Filter> retVal = new ArrayList<>();
+ final Set<EquivalenceCheckedFilter> seenFilters = new HashSet<>();
+
+ for (Filter child : filters) {
+ if (child instanceof AndFilter) {
+ retVal.addAll(flattenAndChildren(((AndFilter) child).getFilters()));
+ } else if (!(child instanceof TrueFilter) && seenFilters.add(new
EquivalenceCheckedFilter(child))) {
+ retVal.add(child);
+ }
+ }
+
+ return retVal;
+ }
+
+ /**
+ * Flattens children of an OR, removes duplicates, and removes
literally-false filters.
+ */
+ private static List<Filter> flattenOrChildren(final List<Filter> filters)
+ {
+ final List<Filter> retVal = new ArrayList<>();
+ final Set<EquivalenceCheckedFilter> seenFilters = new HashSet<>();
+
+ for (Filter child : filters) {
+ if (child instanceof OrFilter) {
+ retVal.addAll(flattenOrChildren(((OrFilter) child).getFilters()));
+ } else if (!(child instanceof FalseFilter) && seenFilters.add(new
EquivalenceCheckedFilter(child))) {
+ retVal.add(child);
+ }
+ }
+
+ return retVal;
+ }
+
+ /**
+ * Wraps a {@link Filter} in an object whose {@link #equals} and {@link
#hashCode} methods are overridden to
+ * ignore order of children for {@link AndFilter} and {@link OrFilter}.
Useful for deduplication in
+ * {@link #flattenAndChildren} and {@link #flattenOrChildren}.
+ */
+ private static class EquivalenceCheckedFilter
+ {
+ private final Filter filter;
+
+ EquivalenceCheckedFilter(Filter filter)
+ {
+ this.filter = filter;
+ }
+
+ @Override
+ public boolean equals(Object o)
+ {
+ if (this == o) {
+ return true;
+ }
+ if (o == null || getClass() != o.getClass()) {
+ return false;
+ }
+ EquivalenceCheckedFilter that = (EquivalenceCheckedFilter) o;
+
+ if (filter instanceof BooleanFilter &&
filter.getClass().equals(that.filter.getClass())) {
+ // Order of children doesn't matter for equivalence.
+ final Set<Filter> ourFilterSet = new HashSet<>(((BooleanFilter)
filter).getFilters());
Review comment:
will your changes fix https://github.com/apache/druid/pull/10316? I am
guessing, your changes will make the situations describe in that PR less likely
as Set is being created only when two AndFilter/OrFilters are compared. could
we also compare the size of child filter and return early before creating a
possible expensive hashSet?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
