somandal commented on code in PR #9333:
URL: https://github.com/apache/pinot/pull/9333#discussion_r971529866
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/column/PhysicalColumnIndexContainer.java:
##########
@@ -133,7 +133,12 @@ public
PhysicalColumnIndexContainer(SegmentDirectory.Reader segmentReader, Colum
_rangeIndex = null;
}
- PinotDataBuffer fwdIndexBuffer = segmentReader.getIndexFor(columnName,
ColumnIndexType.FORWARD_INDEX);
+ // Setting the 'fwdIndexBuffer' to null if forward index is enabled. No-op
index readers will be setup for the
+ // forward index disabled columns which doesn't require the
'fwdIndexBuffer'.
+ boolean forwardIndexDisabled = !segmentReader.hasIndexFor(columnName,
ColumnIndexType.FORWARD_INDEX);
+ PinotDataBuffer fwdIndexBuffer = forwardIndexDisabled ? null
+ : segmentReader.getIndexFor(columnName, ColumnIndexType.FORWARD_INDEX);
Review Comment:
I can only do this if I put the `getIndexFor` in a `try catch` block. The
underlying implementation of the `ColumnIndexDirectory` throws an exception if
the index is not present in the `getIndexFor` call. Do you want me to add this
in a `try catch` block like this?:
```
PinotDataBuffer fwdIndexBuffer;
try {
fwdIndexBuffer = segmentReader.getIndexFor(columnName,
ColumnIndexType.FORWARD_INDEX);
} catch (Exception e) {
fwdIndexBuffer = null;
}
```
e.g. of the type of exception thrown down the line
(`SingleFileIndexDirectory`):
```
private PinotDataBuffer checkAndGetIndexBuffer(String column,
ColumnIndexType type) {
IndexKey key = new IndexKey(column, type);
IndexEntry entry = _columnEntries.get(key);
if (entry == null || entry._buffer == null) {
throw new RuntimeException(
"Could not find index for column: " + column + ", type: " + type +
", segment: " + _segmentDirectory
.toString());
}
return entry._buffer;
}
```
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/IndexLoadingConfig.java:
##########
@@ -315,6 +341,14 @@ public List<String> getSortedColumns() {
return _sortedColumns;
}
+ /**
+ * For tests only.
+ */
+ @VisibleForTesting
+ public void setSortedColumns(List<String> sortedColumns) {
Review Comment:
done
--
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
