[GitHub] [druid] gianm commented on a change in pull request #10119: Combine InDimFilter, InFilter.

[GitBox <gi...@apache.org>]

gianm commented on a change in pull request #10119:
URL: https://github.com/apache/druid/pull/10119#discussion_r466075051



##########
File path: processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java
##########
@@ -56,12 +69,14 @@
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.HashSet;
+import java.util.Iterator;
 import java.util.List;
+import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
 
-public class InDimFilter implements DimFilter
+public class InDimFilter implements DimFilter, Filter

Review comment:
       > Imagining a future world where `DimFilter` and `Filter` are combined, in the future can we rename this to `InFilter` instead of `InDimFilter` and drop all the `DimFilter` terminology?
   
   I think that would be best, yes.
   
   > Is the place where the `DimFilter` implementations live `org.apache.druid.query.filter` more appropriate packaging than the place where the `Filter` implementations live `org.apache.druid.segment.filter`?
   
   I think you could argue for either one. Just from looking at them, it seems like most other stuff that mixes Jackson API and implementation lives in `org.apache.druid.query`.




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@druid.apache.org
For additional commands, e-mail: commits-help@druid.apache.org