[GitHub] [pinot] Jackie-Jiang commented on a change in pull request #7169: Support get metadata and indexes API for REALTIME tables.

[GitBox <gi...@apache.org>]

Jackie-Jiang commented on a change in pull request #7169:
URL: https://github.com/apache/pinot/pull/7169#discussion_r677705305



##########
File path: pinot-server/src/test/java/org/apache/pinot/server/api/TablesResourceTest.java
##########
@@ -264,18 +264,19 @@ public void testOfflineTableSegmentMetadata()
     Assert.assertTrue(jsonResponse.has("endTimeReadable"));
     Assert.assertTrue(jsonResponse.has("creationTimeReadable"));
     Assert.assertEquals(jsonResponse.get("columns").size(), 0);
-    Assert.assertEquals(jsonResponse.get("indexes").size(), 17);
+    Assert.assertEquals(jsonResponse.get("indexes").size(), 0);
 
     jsonResponse = JsonUtils.stringToJsonNode(
         _webTarget.path(segmentMetadataPath).queryParam("columns", "column1").queryParam("columns", "column2").request()
             .get(String.class));
     Assert.assertEquals(jsonResponse.get("columns").size(), 2);
-    Assert.assertEquals(jsonResponse.get("indexes").size(), 17);
+    Assert.assertEquals(jsonResponse.get("indexes").size(), 2);
     Assert.assertEquals(jsonResponse.get("star-tree-index").size(), 0);
 
     jsonResponse = JsonUtils.stringToJsonNode(
         (_webTarget.path(segmentMetadataPath).queryParam("columns", "*").request().get(String.class)));
     Assert.assertEquals(jsonResponse.get("columns").size(), segmentMetadata.getAllColumns().size());
+    Assert.assertEquals(jsonResponse.get("indexes").size(), 14);

Review comment:
       ```suggestion
       Assert.assertEquals(jsonResponse.get("indexes").size(), segmentMetadata.getAllColumns().size());
   ```

##########
File path: pinot-server/src/main/java/org/apache/pinot/server/api/resources/SegmentMetadataFetcher.java
##########
@@ -76,78 +75,79 @@ public static String getSegmentMetadata(SegmentDataManager segmentDataManager, L
       columnSet = new HashSet<>(columns);
     }
     ObjectNode segmentMetadataJson = (ObjectNode) segmentMetadata.toJson(columnSet);
-    segmentMetadataJson.set(COLUMN_INDEX_KEY, JsonUtils.objectToJsonNode(getIndexesForSegmentColumns(segmentDataManager)));
-    segmentMetadataJson.set(STAR_TREE_INDEX_KEY, JsonUtils.objectToJsonNode((getStarTreeIndexesForSegment(segmentDataManager))));
+    segmentMetadataJson
+        .set(COLUMN_INDEX_KEY, JsonUtils.objectToJsonNode(getIndexesForSegmentColumns(segmentDataManager, columnSet)));
+    segmentMetadataJson
+        .set(STAR_TREE_INDEX_KEY, JsonUtils.objectToJsonNode((getStarTreeIndexesForSegment(segmentDataManager))));
     return JsonUtils.objectToString(segmentMetadataJson);
   }
 
   /**
    * Get the JSON object with the segment column's indexing metadata.
    */
-  @Nullable
-  private static Map<String, Map<String, String>> getIndexesForSegmentColumns(SegmentDataManager segmentDataManager) {
+  private static Map<String, Map<String, String>> getIndexesForSegmentColumns(SegmentDataManager segmentDataManager,
+      @Nullable Set<String> columnSet) {
     IndexSegment segment = segmentDataManager.getSegment();
-    if (segment instanceof ImmutableSegmentImpl) {
-      return getImmutableSegmentColumnIndexes(((ImmutableSegmentImpl) segment).getIndexContainerMap());
-    } else {
-      return null;
+    Map<String, Map<String, String>> columnIndexMap = new LinkedHashMap<>();
+    for (String physicalColumnName : segment.getPhysicalColumnNames()) {
+      if (columnSet == null || columnSet.contains(physicalColumnName)) {
+        DataSource dataSource = segment.getDataSource(physicalColumnName);
+        getSegmentColumnIndexes(physicalColumnName, dataSource, columnIndexMap);
+      }
     }
+    return columnIndexMap;
   }
 
   /**
-   * Helper to loop through column index container to create a index map as follows for each column:
+   * Helper to loop through column datasource to create a index map as follows for each column:
    * {<"bloom-filter", "YES">, <"dictionary", "NO">}
    */
-  private static Map<String, Map<String, String>> getImmutableSegmentColumnIndexes(Map<String, ColumnIndexContainer> columnIndexContainerMap) {
-    Map<String, Map<String, String>> columnIndexMap = new LinkedHashMap<>();
-    for (Map.Entry<String, ColumnIndexContainer> entry : columnIndexContainerMap.entrySet()) {
-      ColumnIndexContainer columnIndexContainer = entry.getValue();
-      Map<String, String> indexStatus = new LinkedHashMap<>();
-      if (Objects.isNull(columnIndexContainer.getBloomFilter())) {
-        indexStatus.put(BLOOM_FILTER, INDEX_NOT_AVAILABLE);
-      } else {
-        indexStatus.put(BLOOM_FILTER, INDEX_AVAILABLE);
-      }
-
-      if (Objects.isNull(columnIndexContainer.getDictionary())) {
-        indexStatus.put(DICTIONARY, INDEX_NOT_AVAILABLE);
-      } else {
-        indexStatus.put(DICTIONARY, INDEX_AVAILABLE);
-      }
+  private static void getSegmentColumnIndexes(String columnName, DataSource dataSource,

Review comment:
       Suggest changing the signature to:
   ```
   private static Map<String, String> getColumnIndexes(DataSource dataSource)
   ```




-- 
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-unsubscribe@pinot.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org