Jackie-Jiang commented on code in PR #15591: URL: https://github.com/apache/pinot/pull/15591#discussion_r2129810427
########## pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImpl.java: ########## @@ -164,10 +176,58 @@ public Set<Integer> getPartitions() { return _partitions; } - @Nullable @Override - public Map<IndexType<?, ?, ?>, Long> getIndexSizeMap() { - return _indexSizeMap; + public long getIndexSizeFor(IndexType type) { + short indexId = IndexService.getInstance().getNumericId(type); + for (int i = 0, n = getNumIndexes(); i < n; i++) { Review Comment: ^^ ########## pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImpl.java: ########## @@ -335,6 +378,22 @@ public static FieldSpec generateFieldSpec(String column, PropertiesConfiguration return fieldSpec; } + // This method is only meant to retain compatibility of deserialization for + // /tables/{tableName}/segments/{segmentName}/metadata endpoint . + @SuppressWarnings("unused") + public Map<IndexType<?, ?, ?>, Long> getIndexSizeMap() { + IndexService service = IndexService.getInstance(); + Map<IndexType<?, ?, ?>, Long> result = new HashMap<>(); + + for (int i = 0, n = getNumIndexes(); i < n; i++) { Review Comment: (minor) Same here, you can directly loop over `_indexTypeSizeList` to reduce access ########## pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/metadata/ColumnMetadataImpl.java: ########## @@ -164,10 +174,43 @@ public Set<Integer> getPartitions() { return _partitions; } - @Nullable @Override - public Map<IndexType<?, ?, ?>, Long> getIndexSizeMap() { - return _indexSizeMap; + @VisibleForTesting + public long getIndexSizeFor(IndexType type) { + short indexId = IndexService.getInstance().getNumericId(type); + for (int i = 0, n = getNumIndexes(); i < n; i++) { + if (indexId == getIndexType(i)) { + return getIndexSize(i); + } + } + + return INDEX_NOT_FOUND; + } + + // size should be non-negative 48-bit value + public void addIndexSize(short indexType, long size) { + if (size < 0 || size > SIZE_MASK) { + throw new IllegalArgumentException( + "Index size should be a non-negative integer value between 0 and " + SIZE_MASK); + } + long typeAndSize = ((long) indexType) << 48 | (size & SIZE_MASK); + _indexTypeSizeList.add(typeAndSize); + } + + @Override + public short getIndexType(int index) { + return (short) ((_indexTypeSizeList.getLong(index) >>> 48)); + } + + @Override + public long getIndexSize(int i) { + return (_indexTypeSizeList.getLong(i) & SIZE_MASK); + } Review Comment: (minor) Keep the argument name consistent (`position`) -- 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