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: 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

Reply via email to