This is an automated email from the ASF dual-hosted git repository.
snlee pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pinot.git
The following commit(s) were added to refs/heads/master by this push:
new ec1c859 Relaxing timeColumnName and indexingConfig validation (#6185)
ec1c859 is described below
commit ec1c85987a25416ecb3ce57961d888381b29027d
Author: Seunghyun Lee <sn...@linkedin.com>
AuthorDate: Mon Oct 26 19:24:02 2020 -0700
Relaxing timeColumnName and indexingConfig validation (#6185)
Current validation code will fail when the time column name or
indexing config list includes the empty string. We can relax the
validation by treating the empty string the same as the null value.
Added the unit test.
---
.../apache/pinot/core/util/TableConfigUtils.java | 30 +++++++++++++++++++++-
.../pinot/core/util/TableConfigUtilsTest.java | 30 ++++++++++++++++++++++
2 files changed, 59 insertions(+), 1 deletion(-)
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/util/TableConfigUtils.java
b/pinot-core/src/main/java/org/apache/pinot/core/util/TableConfigUtils.java
index f2c787e..83d752c 100644
--- a/pinot-core/src/main/java/org/apache/pinot/core/util/TableConfigUtils.java
+++ b/pinot-core/src/main/java/org/apache/pinot/core/util/TableConfigUtils.java
@@ -26,6 +26,7 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
import java.util.concurrent.TimeUnit;
+import java.util.stream.Collectors;
import javax.annotation.Nullable;
import org.apache.commons.collections.CollectionUtils;
import org.apache.pinot.common.tier.TierFactory;
@@ -75,6 +76,8 @@ public final class TableConfigUtils {
if (tableConfig.getTableType() == TableType.REALTIME) {
Preconditions.checkState(schema != null, "Schema should not be null for
REALTIME table");
}
+ // Sanitize the table config before validation
+ sanitize(tableConfig);
validateValidationConfig(tableConfig, schema);
validateIngestionConfig(tableConfig.getIngestionConfig(), schema);
validateTierConfigList(tableConfig.getTierConfigsList());
@@ -150,7 +153,7 @@ public final class TableConfigUtils {
Preconditions.checkState(timeColumnName != null, "'timeColumnName'
cannot be null in REALTIME table config");
}
// timeColumnName can be null in OFFLINE table
- if (timeColumnName != null && schema != null) {
+ if (timeColumnName != null && !timeColumnName.isEmpty() && schema != null)
{
Preconditions.checkState(schema.getSpecForTimeColumn(timeColumnName) !=
null,
"Cannot find valid fieldSpec for timeColumn: %s from the table
config: %s, in the schema: %s", timeColumnName,
tableConfig.getTableName(), schema.getSchemaName());
@@ -438,4 +441,29 @@ public final class TableConfigUtils {
"Column Name " + columnName + " defined in field config list must be
a valid column defined in the schema");
}
}
+
+ private static void sanitize(TableConfig tableConfig) {
+
tableConfig.setIndexingConfig(sanitizeIndexingConfig(tableConfig.getIndexingConfig()));
+ }
+
+ private static IndexingConfig sanitizeIndexingConfig(IndexingConfig
indexingConfig) {
+
indexingConfig.setInvertedIndexColumns(sanitizeListBasedIndexingColumns(indexingConfig.getInvertedIndexColumns()));
+
indexingConfig.setNoDictionaryColumns(sanitizeListBasedIndexingColumns(indexingConfig.getNoDictionaryColumns()));
+
indexingConfig.setSortedColumn(sanitizeListBasedIndexingColumns(indexingConfig.getSortedColumn()));
+
indexingConfig.setBloomFilterColumns(sanitizeListBasedIndexingColumns(indexingConfig.getBloomFilterColumns()));
+ indexingConfig
+
.setOnHeapDictionaryColumns(sanitizeListBasedIndexingColumns(indexingConfig.getOnHeapDictionaryColumns()));
+
indexingConfig.setRangeIndexColumns(sanitizeListBasedIndexingColumns(indexingConfig.getRangeIndexColumns()));
+ indexingConfig.setVarLengthDictionaryColumns(
+
sanitizeListBasedIndexingColumns(indexingConfig.getVarLengthDictionaryColumns()));
+ return indexingConfig;
+ }
+
+ private static List<String> sanitizeListBasedIndexingColumns(List<String>
indexingColumns) {
+ if (indexingColumns != null) {
+ // Disregard the empty string value for indexing columns
+ return indexingColumns.stream().filter(v ->
!v.isEmpty()).collect(Collectors.toList());
+ }
+ return null;
+ }
}
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/util/TableConfigUtilsTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/util/TableConfigUtilsTest.java
index 55aa1d6..8594463 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/util/TableConfigUtilsTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/util/TableConfigUtilsTest.java
@@ -157,6 +157,12 @@ public class TableConfigUtilsTest {
// expected
}
+ // empty timeColumnName - valid
+ schema = new Schema.SchemaBuilder().setSchemaName(TABLE_NAME).build();
+ tableConfig =
+ new
TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME).setTimeColumnName("").build();
+ TableConfigUtils.validate(tableConfig, schema);
+
// valid
schema = new Schema.SchemaBuilder().setSchemaName(TABLE_NAME)
.addDateTime(TIME_COLUMN, FieldSpec.DataType.LONG,
"1:MILLISECONDS:EPOCH", "1:MILLISECONDS").build();
@@ -501,6 +507,10 @@ public class TableConfigUtilsTest {
}
tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME)
+ .setInvertedIndexColumns(Arrays.asList("")).build();
+ TableConfigUtils.validate(tableConfig, schema);
+
+ tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME)
.setInvertedIndexColumns(Arrays.asList("myCol2")).build();
try {
TableConfigUtils.validate(tableConfig, schema);
@@ -510,6 +520,10 @@ public class TableConfigUtilsTest {
}
tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME)
+ .setNoDictionaryColumns(Arrays.asList("")).build();
+ TableConfigUtils.validate(tableConfig, schema);
+
+ tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME)
.setNoDictionaryColumns(Arrays.asList("myCol2")).build();
try {
TableConfigUtils.validate(tableConfig, schema);
@@ -519,6 +533,10 @@ public class TableConfigUtilsTest {
}
tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME)
+ .setOnHeapDictionaryColumns(Arrays.asList("")).build();
+ TableConfigUtils.validate(tableConfig, schema);
+
+ tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME)
.setOnHeapDictionaryColumns(Arrays.asList("myCol2")).build();
try {
TableConfigUtils.validate(tableConfig, schema);
@@ -528,6 +546,11 @@ public class TableConfigUtilsTest {
}
tableConfig =
+ new
TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME).setRangeIndexColumns(Arrays.asList(""))
+ .build();
+ TableConfigUtils.validate(tableConfig, schema);
+
+ tableConfig =
new
TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME).setRangeIndexColumns(Arrays.asList("myCol2"))
.build();
try {
@@ -537,6 +560,9 @@ public class TableConfigUtilsTest {
// expected
}
+ tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME).setSortedColumn("").build();
+ TableConfigUtils.validate(tableConfig, schema);
+
tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME).setSortedColumn("myCol2").build();
try {
TableConfigUtils.validate(tableConfig, schema);
@@ -546,6 +572,10 @@ public class TableConfigUtilsTest {
}
tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME)
+ .setVarLengthDictionaryColumns(Arrays.asList("")).build();
+ TableConfigUtils.validate(tableConfig, schema);
+
+ tableConfig = new
TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME)
.setVarLengthDictionaryColumns(Arrays.asList("myCol2")).build();
try {
TableConfigUtils.validate(tableConfig, schema);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
