gortiz commented on code in PR #12223:
URL: https://github.com/apache/pinot/pull/12223#discussion_r1445870792
##########
pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/dictionary/DictionaryIndexTypeTest.java:
##########
@@ -191,7 +194,98 @@ public void newOnHeap()
+ " }"
+ " }\n"
+ " }");
- assertEquals(new DictionaryIndexConfig(true, false));
+ assertEquals(new DictionaryIndexConfig(true, false, null));
+ }
+
+ @Test
+ public void newOnHeapWithConfig()
+ throws IOException {
+ addFieldIndexConfig(""
+ + " {\n"
+ + " \"name\": \"dimInt\","
+ + " \"indexes\" : {\n"
+ + " \"dictionary\": {\n"
+ + " \"onHeap\": true,\n"
+ + " \"onHeapDictionaryConfig\": {\n"
+ + " \"enableInterning\":true ,\n"
+ + " \"internerCapacity\":1000\n"
+ + " }"
+ + " }"
+ + " }\n"
+ + " }");
+ assertEquals(new DictionaryIndexConfig(true, false, new
OnHeapDictionaryConfig(true, 1000)));
+ }
+
+ @Test
+ public void newDisabledOnHeapWithConfig()
+ throws IOException {
+ addFieldIndexConfig(""
+ + " {\n"
+ + " \"name\": \"dimInt\","
+ + " \"indexes\" : {\n"
+ + " \"dictionary\": {\n"
+ + " \"onHeap\": false,\n"
+ + " \"onHeapDictionaryConfig\": {\n"
+ + " \"enableInterning\":true ,\n"
+ + " \"internerCapacity\":1000\n"
+ + " }"
+ + " }"
+ + " }\n"
+ + " }");
+ assertEquals(new DictionaryIndexConfig(false, false, null));
Review Comment:
This is interesting. If onHeap is false we ignore onHeapDictionaryConfig.
I'm not totally against that, but it has a side effect we should discuss.
Ignoring `onHeapDictionaryConfig` means that when users get the table config of
the table using controller endpoints (like using swagger UI) the returned table
config will not contain the `onHeapDictionaryConfig` we ignored at runtime.
--
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
