gortiz commented on code in PR #10352:
URL: https://github.com/apache/pinot/pull/10352#discussion_r1122743896
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/IndexLoadingConfig.java:
##########
@@ -386,11 +387,15 @@ public void setSortedColumn(String sortedColumn) {
}
public Set<String> getInvertedIndexColumns() {
- return _invertedIndexColumns;
+ return unmodifiable(_invertedIndexColumns);
}
public Set<String> getRangeIndexColumns() {
- return _rangeIndexColumns;
+ return unmodifiable(_rangeIndexColumns);
+ }
+
+ public void addRangeIndexColumn(String... columns) {
+ _rangeIndexColumns.addAll(Arrays.asList(columns));
Review Comment:
I don't like the suggestion of having a single column as parameter. It is
easier to use using `String...`. This is the pattern I used in all places
because it let you either add one or multiple columns with the same syntax. The
returned value is not useful, as it is not used anywhere.
Usually using varargs is not the best because it implies to create an array.
But this is not in the hotpath, these methods are only used in tests where we
can afford the extra cost.
About the repetition, I have to check it and remove them in case they are
actually repeated
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
