Jackie-Jiang commented on code in PR #15672:
URL: https://github.com/apache/pinot/pull/15672#discussion_r2070558978
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessor.java:
##########
@@ -130,7 +130,24 @@ public void process(@Nullable SegmentOperationsThrottler
segmentOperationsThrott
if (type != StandardIndexes.forward()) {
IndexHandler handler = createHandler(type);
indexHandlers.add(handler);
- handler.updateIndices(segmentWriter);
+ try {
+ handler.updateIndices(segmentWriter);
+ } catch (IllegalStateException | UnsupportedOperationException e) {
Review Comment:
IMO we don't want to differentiate the handling of different exceptions. We
are not strictly following a convention for exception type, e.g. why not
catching `IllegalArgumentException`? We should just use a flag to control
whether to continue on error to make the behavior more deterministic.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/SegmentPreProcessor.java:
##########
@@ -130,7 +130,24 @@ public void process(@Nullable SegmentOperationsThrottler
segmentOperationsThrott
if (type != StandardIndexes.forward()) {
IndexHandler handler = createHandler(type);
indexHandlers.add(handler);
- handler.updateIndices(segmentWriter);
+ try {
+ handler.updateIndices(segmentWriter);
+ } catch (IllegalStateException | UnsupportedOperationException e) {
+ // The index creation failure is expected in some cases, like when
the index is not supported for the
+ // column or segment type.
+ // E.g. non-dictionary encoded TIMESTAMP type doesn't support
range index, meanwhile dictionary encoded
Review Comment:
For this example, if we skip updating index, I believe query can fail during
query time, and it will be extremely hard to reason about.
--
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]