xiangfu0 commented on code in PR #18758:
URL: https://github.com/apache/pinot/pull/18758#discussion_r4410753025
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/store/TextIndexUtils.java:
##########
@@ -530,6 +530,9 @@ public static void writeConfigToPropertiesFile(File
indexDir, TextIndexConfig co
properties.setProperty(FieldConfig.TEXT_INDEX_LUCENE_ANALYZER_CLASS_ARG_TYPES,
escapedLuceneAnalyzerClassArgTypes);
properties.setProperty(FieldConfig.TEXT_INDEX_LUCENE_QUERY_PARSER_CLASS,
config.getLuceneQueryParserClass());
properties.setProperty(FieldConfig.TEXT_INDEX_LUCENE_DOC_ID_TRANSLATOR_MODE,
config.getDocIdTranslatorMode());
+ // Persist the build mode so the reader self-describes (a table may hold a
mix of dictionary-based and per-row
+ // text segments; reads must not depend on the live table config).
+ properties.setProperty(FieldConfig.TEXT_INDEX_BUILD_ON_DICTIONARY,
config.isBuildOnDictionary());
Review Comment:
Added an explicit Upgrade notes warning to the PR description and
TextIndexConfig.isBuildOnDictionary() Javadoc in b6044e3b53: enable only after
all servers support the format; before downgrading, disable the option and
rebuild all dictionary-based text indexes. The warning calls out silent wrong
results from older readers interpreting dictIds as row IDs.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/text/LuceneTextIndexCreator.java:
##########
@@ -153,7 +160,10 @@ public LuceneTextIndexCreator(String column, File
segmentIndexDir, boolean commi
// 1. Not the realtime index, i.e. commit is set to true
// 2. Happens during realtime segment conversion
// 3. Mutable segment not compacted
- _reuseMutableIndex = config.isReuseMutableIndex() && commit &&
realtimeConversion && !mutableSegmentCompacted;
+ // 4. Not a dictionary-based index: the mutable index is per-row, so a
dictionary-based immutable index must be
+ // rebuilt from the sealed dictionary rather than reusing the mutable
index.
+ _reuseMutableIndex = config.isReuseMutableIndex() && commit &&
realtimeConversion && !mutableSegmentCompacted
+ && !_buildOnDictionary;
Review Comment:
Added RealtimeSegmentConverterTest#testDictionaryBasedTextIndexConversion in
b6044e3b53 with normal MutableColumnStatistics and commit-compacted
CompactedColumnStatistics variants. Both use a real consuming index and
converter with buildOnDictionary=true/reuseMutableIndex=true; they verify
per-row consuming matches, sealed dictionary mode without loader preprocessing,
retained-row matches, and removal of a value present only in discarded rows.
Both cases pass on JDK 25; TextIndexConfigTest (10 tests), affected-module
formatting, checkstyle, and license checks pass.
--
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]
