Copilot commented on code in PR #18758:
URL: https://github.com/apache/pinot/pull/18758#discussion_r3411918607
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/readers/text/LuceneTextIndexReader.java:
##########
@@ -331,6 +356,11 @@ private TextIndexConfig
updateConfigFromProperties(Properties properties, TextIn
builder.withDocIdTranslatorMode(docIdTranslatorMode);
}
+ String buildOnDictionary =
properties.getProperty(FieldConfig.TEXT_INDEX_BUILD_ON_DICTIONARY);
+ if (buildOnDictionary != null) {
+ builder.withBuildOnDictionary(Boolean.parseBoolean(buildOnDictionary));
+ }
Review Comment:
For buffer-loaded indexes, if `buildOnDictionary` is absent from the
persisted properties (segments created before this feature), the reader
currently keeps the table-config value. That can incorrectly treat an old
per-row index as dictionary-based. Prefer defaulting to `false` when the
property is missing so the segment self-describes safely.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/TextIndexUtils.java:
##########
@@ -437,6 +440,9 @@ public static TextIndexConfig
getUpdatedConfigFromPropertiesFile(File file, Text
.withLuceneAnalyzerClassArgTypes(recoveredLuceneAnalyzerClassArgTypes)
.withLuceneQueryParserClass(properties.getString(FieldConfig.TEXT_INDEX_LUCENE_QUERY_PARSER_CLASS))
.withDocIdTranslatorMode(properties.getString(FieldConfig.TEXT_INDEX_LUCENE_DOC_ID_TRANSLATOR_MODE))
+ // Default to the existing config value when the property is absent
(segments written before this feature).
+
.withBuildOnDictionary(properties.getBoolean(FieldConfig.TEXT_INDEX_BUILD_ON_DICTIONARY,
+ config.isBuildOnDictionary()))
Review Comment:
When the segment properties file doesn’t contain `buildOnDictionary` (e.g.,
segments written before this feature), defaulting to the *table config* value
can cause the reader to misinterpret a per-row index as dictionary-based (or
vice versa), breaking the “segment self-describes” guarantee and potentially
returning incorrect results.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]