cheddar commented on a change in pull request #12314:
URL: https://github.com/apache/druid/pull/12314#discussion_r822357002
##########
File path: processing/src/main/java/org/apache/druid/segment/filter/Filters.java
##########
@@ -433,10 +434,15 @@ public static Filter convertToCNFFromQueryContext(Query
query, @Nullable Filter
return null;
}
boolean useCNF = query.getContextBoolean(QueryContexts.USE_FILTER_CNF_KEY,
QueryContexts.DEFAULT_USE_FILTER_CNF);
- return useCNF ? Filters.toCnf(filter) : filter;
+ try {
+ return useCNF ? Filters.toCnf(filter) : filter;
+ }
+ catch (CNFFilterExplosionException cnfFilterExplosionException) {
+ return filter; // cannot convert to CNF, return the filter as is
Review comment:
For clusters where this happens regularly, that can generate a lot of
log spew. You could argue that they should respond to the log spew by not
doing the queries, but it would still be a sudden chunk of log spew. In
general, CNF is a potential optimization done by the planner, we don't log
other optimization choices that we make, we just make them. In this case, it's
deeming that converting to CNF is more expensive than just leaving it as is,
which is yet another optimization decision. I don't think it needs a log, or
if it does, maybe at debug level.
--
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]
