somandal commented on code in PR #9740:
URL: https://github.com/apache/pinot/pull/9740#discussion_r1015887403
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/ForwardIndexHandler.java:
##########
@@ -120,12 +128,47 @@ public void updateIndices(SegmentDirectory.Writer
segmentWriter, IndexCreatorPro
createDictBasedForwardIndex(column, segmentWriter,
indexCreatorProvider);
break;
}
+ case DELETE_FORWARD_INDEX: {
+ // Deletion of the forward index will be handled outside the index
handler to ensure that other index
+ // handlers that need the forward index to construct their own
indexes will have it available.
+ // The existing forward index must be in dictionary format for this
to be a no-op.
+ _forwardIndexDisabledColumnsToCleanup.add(column);
+ break;
+ }
+ case
CREATE_TEMP_DICTIONARY_BASED_FORWARD_INDEX_FROM_EXISTING_RAW_FORWARD_INDEX: {
+ // The forward index has been disabled for a column which has a
noDictionary based forward index. A dictionary
+ // and inverted index need to be created before we can delete the
forward index. We create a dictionary here,
+ // but let the InvertedIndexHandler handle the creation of the
inverted index. We create a temporary
+ // forward index here which is dictionary based and allow the post
deletion step handle the actual deletion
+ // of the forward index.
+ createDictBasedForwardIndex(column, segmentWriter,
indexCreatorProvider);
+ Preconditions.checkState(segmentWriter.hasIndexFor(column,
ColumnIndexType.FORWARD_INDEX),
+ String.format("Temporary forward index was not created for
column: %s", column));
+ _forwardIndexDisabledColumnsToCleanup.add(column);
+ break;
+ }
default:
throw new IllegalStateException("Unsupported operation for column "
+ column);
}
}
}
+ @Override
+ public void postUpdateIndicesCleanup(SegmentDirectory.Writer segmentWriter)
+ throws Exception {
+ // Delete the forward index for columns which have it disabled. Perform
this as a post-processing step after all
+ // IndexHandlers have updated their indexes as some of them need to
temporarily create a forward index to
+ // generate other indexes off of.
+ for (String column : _forwardIndexDisabledColumnsToCleanup) {
+ if ((_indexLoadingConfig.getForwardIndexDisabledColumns() != null
+ &&
_indexLoadingConfig.getForwardIndexDisabledColumns().contains(column))
Review Comment:
Good point, this check is not needed here since we only add to the
`_forwardIndexDisabledColumnsToCleanup` if the check in `computeOperationMap`
is done. Will remove this.
--
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]
