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]

Reply via email to