gortiz commented on code in PR #12223:
URL: https://github.com/apache/pinot/pull/12223#discussion_r1445866163
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/IndexLoadingConfig.java:
##########
@@ -92,6 +93,7 @@ public class IndexLoadingConfig {
private Set<String> _onHeapDictionaryColumns = new HashSet<>();
private Set<String> _forwardIndexDisabledColumns = new HashSet<>();
private Map<String, BloomFilterConfig> _bloomFilterConfigs = new HashMap<>();
+ private Map<String, OnHeapDictionaryConfig> _onHeapDictionaryConfigs = new
HashMap<>();
Review Comment:
This is another thing I would vote *against*.
This class is an antipattern. @Jackie-Jiang and I tried to remove it when
working on index-spi, but its usage is spread in the code too much, so we ended
up keeping it. Ideally we should not increase its usage and complexity. Instead
of using this attribute (and its getter method), we should use this class as a
proxy and in order to know if there is heap config or not we can just do:
```
DictionaryIndexConfig config =
indexLoadingConfig.getFieldIndexConfig(colName).getConfig(StandardIndexes.dictionary());`
```
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
