[GitHub] [pinot] klsince commented on a diff in pull request #10835: [feature] Add flags to indicate if FST, H3, and Text indexes are on a segment

[via GitHub]

klsince commented on code in PR #10835:
URL: https://github.com/apache/pinot/pull/10835#discussion_r1214687960


##########
pinot-server/src/main/java/org/apache/pinot/server/api/resources/SegmentMetadataFetcher.java:
##########
@@ -152,6 +155,24 @@ private static Map<String, String> 
getColumnIndexes(DataSource dataSource) {
       indexStatus.put(JSON_INDEX, INDEX_AVAILABLE);
     }
 
+    if (Objects.isNull(dataSource.getH3Index())) {
+      indexStatus.put(H3_INDEX, INDEX_NOT_AVAILABLE);
+    } else {
+      indexStatus.put(H3_INDEX, INDEX_AVAILABLE);
+    }
+
+    if (Objects.isNull(dataSource.getFSTIndex())) {
+      indexStatus.put(FST_INDEX, INDEX_NOT_AVAILABLE);
+    } else {
+      indexStatus.put(FST_INDEX, INDEX_AVAILABLE);
+    }
+
+    if (Objects.isNull(dataSource.getTextIndex())) {
+      indexStatus.put(TEXT_INDEX, INDEX_NOT_AVAILABLE);
+    } else {
+      indexStatus.put(TEXT_INDEX, INDEX_AVAILABLE);
+    }

Review Comment:
   good catch. but I think this getColumnIndexes() is simply broken after 
adding the new index-spi support, which allows one to add new index types, not 
in this hard coded list of index types. Perhaps need to use 
`IndexService.getInstance().getAllIndexes()` to refactor this method.
   
   cc @gortiz who added index-spi support, to shed more light on this. 



-- 
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