gortiz commented on code in PR #11600:
URL: https://github.com/apache/pinot/pull/11600#discussion_r1327367316
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/forward/ForwardIndexType.java:
##########
@@ -226,6 +234,14 @@ public String getFileExtension(ColumnMetadata
columnMetadata) {
}
}
+ @Override
+ public List<String> getFileExtensions(@Nullable ColumnMetadata
columnMetadata) {
+ if (columnMetadata == null) {
+ return EXTENSIONS;
Review Comment:
> When will this be the case (ColumnMetadata is null) ?
In the current code, never. But given its role as an optional filter, it
seems ok to support sending a null filter in case we needed in future.
Ideally we shouldn't have to add this parameter and always return all the
lists, but that would break compatibility from the previous implementation.
Specifically, in a weird case when more than one file (with different
extensions) exists for the segment, the older implementation would return the
one that matches with the column metadata.
I don't know if that case is possible, but I have found so weird cases I
thought that were impossible related to segments and their indexes that I
prefer to be cautious here.
> Also since a given column can have a single and a single kind of forward
index, returning the list of extensions isn't helpful to the caller ?
But that is not true for other indexes and we need to honor the common
interface. Most indexes only expects a single extension and most of them don't
even need the columnMetadata parameter.
If the caller knows that ForwardIndexType is being used, they can call
`getFileExtension(ColumnMetadata)`.
--
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
