suneet-s commented on a change in pull request #10758:
URL: https://github.com/apache/druid/pull/10758#discussion_r558804322
##########
File path: processing/src/main/java/org/apache/druid/segment/filter/Filters.java
##########
@@ -480,68 +485,105 @@ public static boolean shouldUseBitmapIndex(
}
/**
- * Create a filter representing an AND relationship across a list of filters.
+ * Create a filter representing an AND relationship across a list of
filters. Deduplicates filters, flattens stacks,
+ * and removes literal "false" filters.
*
- * @param filterList List of filters
+ * @param filters List of filters
*
- * @return If filterList has more than one element, return an AND filter
composed of the filters from filterList
- * If filterList has a single element, return that element alone
- * If filterList is empty, return null
+ * @return If "filters" has more than one filter remaining after processing,
returns {@link AndFilter}.
+ * If "filters" has a single element remaining after processing, return that
filter alone.
+ *
+ * @throws IllegalArgumentException if "filters" is empty
*/
- @Nullable
- public static Filter and(List<Filter> filterList)
+ public static Filter and(final List<Filter> filters)
{
- if (filterList.isEmpty()) {
- return null;
- }
+ return maybeAnd(filters).orElseThrow(() -> new IAE("Expected nonempty
filters list"));
+ }
- if (filterList.size() == 1) {
- return filterList.get(0);
+ /**
+ * Like {@link #and}, but returns an empty Optional instead of throwing an
exception if "filters" is empty.
+ */
+ public static Optional<Filter> maybeAnd(List<Filter> filters)
+ {
+ if (filters.isEmpty()) {
+ return Optional.empty();
}
- return new AndFilter(filterList);
+ final List<Filter> filtersToUse = flattenAndChildren(filters);
+
+ if (filtersToUse.isEmpty()) {
+ assert !filters.isEmpty();
+ // Original "filters" list must have been 100% literally-true filters.
+ return Optional.of(TrueFilter.instance());
+ } else if (filtersToUse.stream().anyMatch(filter -> filter instanceof
FalseFilter)) {
+ // AND of FALSE with anything is FALSE.
+ return Optional.of(FalseFilter.instance());
+ } else if (filtersToUse.size() == 1) {
+ return Optional.of(filtersToUse.get(0));
+ } else {
+ return Optional.of(new AndFilter(filtersToUse));
+ }
}
/**
- * Create a filter representing an OR relationship across a set of filters.
+ * Create a filter representing an OR relationship across a list of filters.
Deduplicates filters, flattens stacks,
+ * and removes literal "false" filters.
+ *
+ * @param filters List of filters
*
- * @param filterSet Set of filters
+ * @return If "filters" has more than one filter remaining after processing,
returns {@link OrFilter}.
+ * If "filters" has a single element remaining after processing, return that
filter alone.
*
- * @return If filterSet has more than one element, return an OR filter
composed of the filters from filterSet
- * If filterSet has a single element, return that element alone
- * If filterSet is empty, return null
+ * @throws IllegalArgumentException if "filters" is empty
*/
- @Nullable
- public static Filter or(Set<Filter> filterSet)
+ public static Filter or(final List<Filter> filters)
{
- if (filterSet.isEmpty()) {
- return null;
- }
+ return maybeOr(filters).orElseThrow(() -> new IAE("Expected nonempty
filters list"));
+ }
- if (filterSet.size() == 1) {
- return filterSet.iterator().next();
+ /**
+ * Like {@link #or}, but returns an empty Optional instead of throwing an
exception if "filters" is empty.
+ */
+ public static Optional<Filter> maybeOr(final List<Filter> filters)
+ {
+ if (filters.isEmpty()) {
+ return Optional.empty();
}
- return new OrFilter(filterSet);
+ final List<Filter> filtersToUse = flattenOrChildren(filters);
+
+ if (filtersToUse.isEmpty()) {
+ assert !filters.isEmpty();
+ // Original "filters" list must have been 100% literally-false filters.
+ return Optional.of(FalseFilter.instance());
+ } else if (filtersToUse.stream().anyMatch(filter -> filter instanceof
TrueFilter)) {
Review comment:
nit: similar comment to line 518
----------------------------------------------------------------
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]
