[GitHub] [druid] gianm commented on a change in pull request #10313: Make NUMERIC_HASHING_THRESHOLD configurable

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

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



##########
File path: processing/src/main/java/org/apache/druid/query/filter/InDimFilter.java
##########
@@ -80,7 +80,8 @@
 {
   // determined through benchmark that binary search on long[] is faster than HashSet until ~16 elements
   // Hashing threshold is not applied to String for now, String still uses ImmutableSortedSet
-  public static final int NUMERIC_HASHING_THRESHOLD = 16;
+  public static final int NUMERIC_HASHING_THRESHOLD =
+      Integer.parseInt(System.getProperty("druid.query.filter.inDimFilter.numericHashingThreshold", "1"));

Review comment:
       I don't think we should have a property for this, but if we did, it should be set through Guice injection rather than through System.getProperty. That way, it'd work in either a runtime props file or a system property.
   
   We won't actually see "1" in practice AFAIK, since `optimize()` will get run, and that'll turn the In filter into a Selector filter.
   
   So if this is the right default, I think it'd be better to remove the code.




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