clintropolis commented on a change in pull request #8209: add mechanism to 
control filter optimization in historical query processing
URL: https://github.com/apache/incubator-druid/pull/8209#discussion_r311791576
 
 

 ##########
 File path: 
extensions-core/druid-bloom-filter/src/main/java/org/apache/druid/query/filter/sql/BloomFilterOperatorConversion.java
 ##########
 @@ -100,7 +100,8 @@ public DimFilter toDruidFilter(
       return new BloomDimFilter(
           druidExpression.getSimpleExtraction().getColumn(),
           holder,
-          druidExpression.getSimpleExtraction().getExtractionFn()
+          druidExpression.getSimpleExtraction().getExtractionFn(),
 
 Review comment:
   I agree, but the reason I have a second set of constructors at all was so I 
didn't have to change hundreds of lines of test code to add a `null` parameter, 
so making a static method would sort of defeat the purpose of that because the 
lines would change anyway.
   
   I do think this is worth doing, I'll add this to the ticket i create about 
refactoring `DimFilter` to also add useful test filter creation static methods, 
so we can also eliminate passing in `null` everywhere for filters without 
`extractionFn`.

----------------------------------------------------------------
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]


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to