mcvsubbu commented on a change in pull request #5074: Support segment reload
for text index
URL: https://github.com/apache/incubator-pinot/pull/5074#discussion_r381543053
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/segment/index/loader/defaultcolumn/BaseDefaultColumnHandler.java
##########
@@ -285,6 +291,89 @@ protected void removeColumnV1Indices(String column)
SegmentColumnarIndexCreator.removeColumnMetadataInfo(_segmentProperties,
column);
}
+ /**
+ * Right now the text index is supported on RAW (non-dictionary encoded)
+ * single-value STRING columns. Eventually we will relax the constraints
+ * step by step.
+ * For example, later on user should be able to create text index on
+ * a dictionary encoded STRING column that also has native Pinot's inverted
+ * index. We can also support it on BYTE columns later.
+ * @param column column name
+ * @param indexLoadingConfig index loading config
+ * @param fieldSpec field spec
+ */
+ private void checkUnsupForEnablingTextIndexOnNewColumn(String column,
IndexLoadingConfig indexLoadingConfig,
+ FieldSpec fieldSpec) {
+ if (!indexLoadingConfig.getNoDictionaryColumns().contains(column)) {
+ throw new UnsupportedOperationException("Text index is currently not
supported on dictionary encoded columns");
Review comment:
You should include the column name in each of these exceptions.
Alternatrively, it may be better to return a boolean, and use PreConditions
in the caller and include the column name in the exception thrown.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
