siddharthteotia commented on code in PR #9333:
URL: https://github.com/apache/pinot/pull/9333#discussion_r988647749


##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/utils/TableConfigUtils.java:
##########
@@ -897,6 +903,40 @@ private static void validateFieldConfigList(@Nullable 
List<FieldConfig> fieldCon
     }
   }
 
+  /**
+   * Validates the compatibility of the indexes if the column has the forward 
index disabled. Throws exceptions due to
+   * compatibility mismatch. The checks performed are:
+   *     - Validate dictionary is enabled.
+   *     - Validate inverted index is enabled.
+   *     - Validate that either no range index exists for column or the range 
index version is at least 2 and isn't a
+   *       multi-value column (since mulit-value defaults to index v1).
+   */
+  private static void validateForwardIndexDisabledIndexCompatibility(String 
columnName, FieldConfig fieldConfig,

Review Comment:
   (nit) we should consider keeping the check either here or during segment 
generation in `SegmentColumnarIndexCreator`. For now it is fine to safeguard 
multiple places until feature gets used for a while. 
   
   I am guessing basic validation at the table config level will always be 
needed since it will come handy even during  enabling / disabling the feature 
on an existing column. So an unsupported combination will be caught when user 
attempts to update the table config in controller as opposed to detecting later 
on the reload path. 



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