xiangfu0 commented on code in PR #18920:
URL: https://github.com/apache/pinot/pull/18920#discussion_r3532887007
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/invertedindex/JsonIndexHandler.java:
##########
@@ -65,12 +70,21 @@ public boolean needUpdateIndices(SegmentDirectory.Reader
segmentReader) {
String segmentName = _segmentDirectory.getSegmentMetadata().getName();
Set<String> columnsToAddIdx = new HashSet<>(_jsonIndexConfigs.keySet());
Set<String> existingColumns =
segmentReader.toSegmentDirectory().getColumnsWithIndex(StandardIndexes.json());
- // Check if any existing index need to be removed.
+ // Load metadata properties once to avoid repeated disk reads when
checking multiple columns.
+ PropertiesConfiguration properties = loadMetadataProperties();
+ // Check if any existing index need to be removed or if the config changed.
for (String column : existingColumns) {
if (!columnsToAddIdx.remove(column)) {
LOGGER.info("Need to remove existing json index from segment: {},
column: {}", segmentName, column);
return true;
}
+ // Column exists in both existing indexes and config; check if config
changed.
+ JsonIndexConfig currentConfig = _jsonIndexConfigs.get(column);
+ JsonIndexConfig storedConfig = readStoredJsonIndexConfig(column,
properties);
+ if (storedConfig != null && !storedConfig.equals(currentConfig)) {
Review Comment:
This skips legacy segments that already have a JSON index but no persisted
jsonIndexConfig. Because needProcess() is gated on this result, those segments
never rebuild or backfill metadata; after upgrading, changing
maxLevels/includePaths/etc. can leave the old JSON index in place and produce
stale json_match results. Treat missing stored config as a one-time
rebuild/backfill case and cover legacy-index plus config-change in a regression
test.
--
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]