[GitHub] [pinot] jugomezv commented on a diff in pull request #10623: WIP: API to get table configs in newer format

["jugomezv (via GitHub)" <gi...@apache.org>]

jugomezv commented on code in PR #10623:
URL: https://github.com/apache/pinot/pull/10623#discussion_r1170600845


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/forward/ForwardIndexType.java:
##########
@@ -56,6 +56,7 @@
 public class ForwardIndexType
     extends AbstractIndexType<ForwardIndexConfig, ForwardIndexReader, ForwardIndexCreator>
     implements ConfigurableFromIndexLoadingConfig<ForwardIndexConfig> {
+  public static final String INDEX_DISPLAY_NAME = "forward";

Review Comment:
   Can we build these by getting prefix of SandardIndexes IDs? i.e. forward_index ==> forward, etc. that eliminates the need of having two distributed set of ids and makes this more maintainable



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/bloom/BloomIndexType.java:
##########
@@ -50,6 +50,7 @@
 public class BloomIndexType
     extends AbstractIndexType<BloomFilterConfig, BloomFilterReader, BloomFilterCreator>
     implements ConfigurableFromIndexLoadingConfig<BloomFilterConfig> {
+  public static final String INDEX_DISPLAY_NAME = "bloom";

Review Comment:
   Is @VisibleForTesting better here? Same for the other types of indexes?



##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/fst/FstIndexType.java:
##########
@@ -167,4 +173,10 @@ protected TextIndexReader createIndexReader(PinotDataBuffer dataBuffer, ColumnMe
       return createIndexReader(dataBuffer, metadata);
     }
   }
+
+  @Override
+  public void convertToNewFormat(TableConfig tableConfig, Schema schema) {
+    super.convertToNewFormat(tableConfig, schema);

Review Comment:
   Here is a nice way to avoid having to do the super call in every implementation, I think it applies here and makes code more maintainable:[here](https://stackoverflow.com/questions/10635279/call-super-class-method-automatically)



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