sunithabeeram commented on a change in pull request #3528: Adding support for 
bloom filter
URL: https://github.com/apache/incubator-pinot/pull/3528#discussion_r235458649
 
 

 ##########
 File path: 
pinot-common/src/main/java/com/linkedin/pinot/common/config/IndexingConfig.java
 ##########
 @@ -113,6 +116,15 @@ public void setSortedColumn(List<String> sortedColumn) {
     _sortedColumn = sortedColumn;
   }
 
+  
+  public List<String> getBloomFilterColumns() {
 
 Review comment:
   Can we add bloomfilter for all dimension columns? 
   
   I am wondering if the configuration can be simplified - as it stands, we 
have one more tunable; if in the common case the overhead is much lower 
compared to the benefit, I would much rather have it on by default. We can have 
a global config to enable or disable bloom - so that way we can do a controlled 
rollout.
   
   Also, I am guessing generating bloom will be handled in the offline segment 
creation path (pbnj) too by default? (We have overrides that disable 
inverted-index creation, so checking what the stance will with this feature)

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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