This is an automated email from the ASF dual-hosted git repository. xiangfu 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 07a6289 Adding dependency validation check on Indexing config (#6038) 07a6289 is described below commit 07a6289a07fc5b268707a09086625f1fc023f144 Author: icefury71 <chinmay.cere...@gmail.com> AuthorDate: Fri Sep 25 15:25:53 2020 -0700 Adding dependency validation check on Indexing config (#6038) * Adding dependency validation check between inverted index, bloom filter and no dictionary column config * Fixing assert comment in valid star tree index config test --- .../apache/pinot/core/util/TableConfigUtils.java | 25 ++++++--- .../pinot/core/util/TableConfigUtilsTest.java | 59 ++++++++++++---------- 2 files changed, 52 insertions(+), 32 deletions(-) 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 755aa7e..caa32d5 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 @@ -237,29 +237,42 @@ public final class TableConfigUtils { /** * Validates the Indexing Config - * Ensures that every referred column name exists in the corresponding schema + * Ensures that every referred column name exists in the corresponding schema. + * Also ensures proper dependency between index types (eg: Inverted Index columns + * cannot be present in no-dictionary columns). */ private static void validateIndexingConfig(@Nullable IndexingConfig indexingConfig, @Nullable Schema schema) { if (indexingConfig == null || schema == null) { return; } Map<String, String> columnNameToConfigMap = new HashMap<>(); + Set<String> noDictionaryColumnsSet = new HashSet<>(); + if (indexingConfig.getNoDictionaryColumns() != null) { + for (String columnName : indexingConfig.getNoDictionaryColumns()) { + columnNameToConfigMap.put(columnName, "No Dictionary Column Config"); + noDictionaryColumnsSet.add(columnName); + } + } if (indexingConfig.getBloomFilterColumns() != null) { for (String columnName : indexingConfig.getBloomFilterColumns()) { + if (noDictionaryColumnsSet.contains(columnName)) { + throw new IllegalStateException( + "Cannot create a Bloom Filter on column " + columnName + " specified in the noDictionaryColumns config"); + } columnNameToConfigMap.put(columnName, "Bloom Filter Config"); } } if (indexingConfig.getInvertedIndexColumns() != null) { for (String columnName : indexingConfig.getInvertedIndexColumns()) { + if (noDictionaryColumnsSet.contains(columnName)) { + throw new IllegalStateException("Cannot create an Inverted index on column " + columnName + + " specified in the noDictionaryColumns config"); + } columnNameToConfigMap.put(columnName, "Inverted Index Config"); } } - if (indexingConfig.getNoDictionaryColumns() != null) { - for (String columnName : indexingConfig.getNoDictionaryColumns()) { - columnNameToConfigMap.put(columnName, "No Dictionary Column Config"); - } - } + if (indexingConfig.getOnHeapDictionaryColumns() != null) { for (String columnName : indexingConfig.getOnHeapDictionaryColumns()) { columnNameToConfigMap.put(columnName, "On Heap Dictionary Column Config"); 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 fc9b29b..de2493b 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 @@ -21,6 +21,7 @@ package org.apache.pinot.core.util; import com.google.common.collect.Lists; import java.util.Arrays; import java.util.Collections; +import java.util.List; import java.util.HashMap; import java.util.Map; import org.apache.pinot.common.tier.TierFactory; @@ -562,27 +563,20 @@ public class TableConfigUtilsTest { } // Although this config makes no sense, it should pass the validation phase - StarTreeIndexConfig starTreeIndexConfig = new StarTreeIndexConfig(Arrays.asList("myCol"), - Arrays.asList("myCol"), - Arrays.asList("SUM__myCol"), - 1); + StarTreeIndexConfig starTreeIndexConfig = + new StarTreeIndexConfig(Arrays.asList("myCol"), Arrays.asList("myCol"), Arrays.asList("SUM__myCol"), 1); tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME) - .setStarTreeIndexConfigs(Arrays.asList(starTreeIndexConfig)) - .build(); + .setStarTreeIndexConfigs(Arrays.asList(starTreeIndexConfig)).build(); try { TableConfigUtils.validate(tableConfig, schema); } catch (Exception e) { - Assert.fail("Should fail for valid StarTreeIndex config column name"); - // expected + Assert.fail("Should not fail for valid StarTreeIndex config column name"); } - starTreeIndexConfig = new StarTreeIndexConfig(Arrays.asList("myCol2"), - Arrays.asList("myCol"), - Arrays.asList("SUM__myCol"), - 1); + starTreeIndexConfig = + new StarTreeIndexConfig(Arrays.asList("myCol2"), Arrays.asList("myCol"), Arrays.asList("SUM__myCol"), 1); tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME) - .setStarTreeIndexConfigs(Arrays.asList(starTreeIndexConfig)) - .build(); + .setStarTreeIndexConfigs(Arrays.asList(starTreeIndexConfig)).build(); try { TableConfigUtils.validate(tableConfig, schema); Assert.fail("Should fail for invalid StarTreeIndex config column name in dimension split order"); @@ -590,13 +584,10 @@ public class TableConfigUtilsTest { // expected } - starTreeIndexConfig = new StarTreeIndexConfig(Arrays.asList("myCol"), - Arrays.asList("myCol2"), - Arrays.asList("SUM__myCol"), - 1); + starTreeIndexConfig = + new StarTreeIndexConfig(Arrays.asList("myCol"), Arrays.asList("myCol2"), Arrays.asList("SUM__myCol"), 1); tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME) - .setStarTreeIndexConfigs(Arrays.asList(starTreeIndexConfig)) - .build(); + .setStarTreeIndexConfigs(Arrays.asList(starTreeIndexConfig)).build(); try { TableConfigUtils.validate(tableConfig, schema); Assert.fail("Should fail for invalid StarTreeIndex config column name in skip star node for dimension"); @@ -604,13 +595,10 @@ public class TableConfigUtilsTest { // expected } - starTreeIndexConfig = new StarTreeIndexConfig(Arrays.asList("myCol"), - Arrays.asList("myCol"), - Arrays.asList("SUM__myCol2"), - 1); + starTreeIndexConfig = + new StarTreeIndexConfig(Arrays.asList("myCol"), Arrays.asList("myCol"), Arrays.asList("SUM__myCol2"), 1); tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME) - .setStarTreeIndexConfigs(Arrays.asList(starTreeIndexConfig)) - .build(); + .setStarTreeIndexConfigs(Arrays.asList(starTreeIndexConfig)).build(); try { TableConfigUtils.validate(tableConfig, schema); Assert.fail("Should fail for invalid StarTreeIndex config column name in function column pair"); @@ -627,5 +615,24 @@ public class TableConfigUtilsTest { } catch (Exception e) { // expected } + + List<String> columnList = Arrays.asList("myCol"); + tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME).setNoDictionaryColumns(columnList) + .setInvertedIndexColumns(columnList).build(); + try { + TableConfigUtils.validate(tableConfig, schema); + Assert.fail("Should fail for valid column name in both no dictionary and inverted index column config"); + } catch (Exception e) { + // expected + } + + tableConfig = new TableConfigBuilder(TableType.OFFLINE).setTableName(TABLE_NAME).setNoDictionaryColumns(columnList) + .setBloomFilterColumns(columnList).build(); + try { + TableConfigUtils.validate(tableConfig, schema); + Assert.fail("Should fail for valid column name in both no dictionary and bloom filter column config"); + } catch (Exception e) { + // expected + } } } --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org