[GitHub] [incubator-pinot] siddharthteotia commented on a change in pull request #6541: - Adding new validation for Json, TEXT indexing

[GitBox <gi...@apache.org>]

siddharthteotia commented on a change in pull request #6541:
URL: https://github.com/apache/incubator-pinot/pull/6541#discussion_r572510770



##########
File path: pinot-core/src/main/java/org/apache/pinot/core/util/TableConfigUtils.java
##########
@@ -488,18 +527,37 @@ private static void validateFieldConfigList(@Nullable List<FieldConfig> fieldCon
 
     for (FieldConfig fieldConfig : fieldConfigList) {
       String columnName = fieldConfig.getName();
-      Preconditions.checkState(schema.getFieldSpecFor(columnName) != null,
+      FieldSpec fieldConfigColSpec = schema.getFieldSpecFor(columnName);
+      Preconditions.checkState(fieldConfigColSpec != null,
           "Column Name " + columnName + " defined in field config list must be a valid column defined in the schema");
 
-      if (fieldConfig.getEncodingType() == FieldConfig.EncodingType.DICTIONARY &&
-          indexingConfigs.getNoDictionaryColumns() != null) {
-        Preconditions.checkArgument(!indexingConfigs.getNoDictionaryColumns().contains(columnName),
-            "FieldConfig encoding type is different from indexingConfig for column: " + columnName);
+      List<String> noDictionaryColumns = indexingConfigs.getNoDictionaryColumns();
+      switch (fieldConfig.getEncodingType()) {
+        case RAW:
+          Preconditions.checkState(noDictionaryColumns != null && noDictionaryColumns.contains(columnName),
+              "FieldConfig encoding type is different from indexingConfig for column: " + columnName);
+          break;
+        case DICTIONARY:
+          if (noDictionaryColumns != null) {
+            Preconditions.checkArgument(!noDictionaryColumns.contains(columnName),
+                "FieldConfig encoding type is different from indexingConfig for column: " + columnName);
+          }
       }
-      // FST Index is only available on dictionary encoded columns.
-      if (fieldConfig.getIndexType() == FieldConfig.IndexType.FST) {
-        Preconditions.checkArgument(fieldConfig.getEncodingType() == FieldConfig.EncodingType.DICTIONARY,
-            "FST Index is only enabled on dictionary encoded columns");
+
+      switch (fieldConfig.getIndexType()) {
+        case FST:
+          Preconditions.checkArgument(fieldConfig.getEncodingType() == FieldConfig.EncodingType.DICTIONARY,
+              "FST Index is only enabled on dictionary encoded columns");
+          Preconditions.checkState(
+              fieldConfigColSpec.isSingleValueField() && fieldConfigColSpec.getDataType() == FieldSpec.DataType.STRING,
+              "FST Index is only supported for single value string columns");
+          break;
+        case TEXT:
+          Preconditions.checkArgument(fieldConfig.getEncodingType() == FieldConfig.EncodingType.RAW,

Review comment:
       Text index is supported on dictionary encoded SV string columns. Why add this check?




----------------------------------------------------------------
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:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org