siddharthteotia commented on code in PR #9333:
URL: https://github.com/apache/pinot/pull/9333#discussion_r988629632
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/creator/impl/SegmentColumnarIndexCreator.java:
##########
@@ -296,6 +309,40 @@ public void init(SegmentGeneratorConfig
segmentCreationSpec, SegmentIndexCreatio
}
}
+ /**
+ * 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 multi-value defaults to index v1).
+ *
+ * @param columnName Name of the column
+ * @param forwardIndexDisabled Whether the forward index is disabled for
column or not
+ * @param dictEnabledColumn Whether the column is dictionary enabled or not
+ * @param columnIndexCreationInfo Column index creation info
+ * @param invertedIndexColumns Set of columns with inverted index enabled
+ * @param rangeIndexColumns Set of columns with range index enabled
+ * @param rangeIndexVersion Range index version
+ * @param fieldSpec FieldSpec of column
+ */
+ private void validateForwardIndexDisabledIndexCompatibility(String
columnName, boolean forwardIndexDisabled,
+ boolean dictEnabledColumn, ColumnIndexCreationInfo
columnIndexCreationInfo, Set<String> invertedIndexColumns,
+ Set<String> rangeIndexColumns, int rangeIndexVersion, FieldSpec
fieldSpec) {
+ if (!forwardIndexDisabled) {
+ return;
+ }
+
+ Preconditions.checkState(dictEnabledColumn,
+ String.format("Cannot disable forward index for column %s without
dictionary", columnName));
+ Preconditions.checkState(invertedIndexColumns.contains(columnName),
+ String.format("Cannot disable forward index for column %s without
inverted index enabled", columnName));
+ Preconditions.checkState(!rangeIndexColumns.contains(columnName)
+ || (fieldSpec.isSingleValueField() && rangeIndexVersion ==
BitSlicedRangeIndexCreator.VERSION), String.format(
+ "Cannot disable forward index for column %s which has range index
with version < 2 or is multi-value",
Review Comment:
The range index condition and message is little less readable.
Suggest breaking it up like done for others
if range index
- if version < 2 -> feature not supported on column with range index
version < 2. either disable range index or use range index version >=2 to use
this feature
- else if MV -> feature not supported on MV column with range index.
Disable range index on this column in order to use this feature
--
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]
