gortiz commented on code in PR #12223:
URL: https://github.com/apache/pinot/pull/12223#discussion_r1450087284
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/dictionary/DictionaryIndexType.java:
##########
@@ -162,10 +167,11 @@ public ColumnConfigDeserializer<DictionaryIndexConfig>
createDeserializer() {
Set<String> varLength = new HashSet<>(
ic.getVarLengthDictionaryColumns() == null ? Collections.emptyList()
: ic.getVarLengthDictionaryColumns()
);
+
+ // Intern configs can only be used through FieldConfigLists.
Function<String, DictionaryIndexConfig> valueCalculator =
- column -> new DictionaryIndexConfig(onHeap.contains(column),
varLength.contains(column));
- return Sets.union(onHeap, varLength).stream()
- .collect(Collectors.toMap(Function.identity(), valueCalculator));
+ column -> new DictionaryIndexConfig(onHeap.contains(column),
varLength.contains(column), Intern.DISABLED);
+ return Sets.union(onHeap,
varLength).stream().collect(Collectors.toMap(Function.identity(),
valueCalculator));
Review Comment:
Minor: Like most fluent DSLs, usually stream operators are written in
separated lines. So I would prefer to keep the previous code style. Also, if we
add the older constructor to DictionaryIndexConfig, we don't actually need to
modify these lines (neither 117-119) beyond adding a comment saying that intern
configs can only be used through FieldConfigLists, which I think it is a nice
addition
--
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
