This is an automated email from the ASF dual-hosted git repository.
jackie pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push:
new d7d504b7f6 Fix regression in ForwardIndexType detected in
https://github.com/apache/pinot/issues/11745 (#11784)
d7d504b7f6 is described below
commit d7d504b7f6ec10ed2b96a9442420d5967bfd2f83
Author: Gonzalo Ortiz Jaureguizar <[email protected]>
AuthorDate: Sat Oct 14 03:03:11 2023 +0200
Fix regression in ForwardIndexType detected in
https://github.com/apache/pinot/issues/11745 (#11784)
---
.../segment/index/forward/ForwardIndexType.java | 40 ++++++++++++------
.../index/forward/ForwardIndexTypeTest.java | 47 ++++++++++++++++++++++
2 files changed, 74 insertions(+), 13 deletions(-)
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/forward/ForwardIndexType.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/forward/ForwardIndexType.java
index d81a233625..369341be6c 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/forward/ForwardIndexType.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/forward/ForwardIndexType.java
@@ -20,8 +20,10 @@
package org.apache.pinot.segment.local.segment.index.forward;
import com.google.common.collect.Lists;
+import com.google.common.collect.Maps;
import com.google.common.collect.Sets;
import java.io.IOException;
+import java.util.Collection;
import java.util.Collections;
import java.util.HashMap;
import java.util.List;
@@ -41,6 +43,7 @@ import
org.apache.pinot.segment.spi.compression.ChunkCompressionType;
import org.apache.pinot.segment.spi.creator.IndexCreationContext;
import org.apache.pinot.segment.spi.index.AbstractIndexType;
import org.apache.pinot.segment.spi.index.ColumnConfigDeserializer;
+import org.apache.pinot.segment.spi.index.DictionaryIndexConfig;
import org.apache.pinot.segment.spi.index.FieldIndexConfigs;
import org.apache.pinot.segment.spi.index.ForwardIndexConfig;
import org.apache.pinot.segment.spi.index.IndexConfigDeserializer;
@@ -153,24 +156,35 @@ public class ForwardIndexType extends
AbstractIndexType<ForwardIndexConfig, Forw
@Override
public ColumnConfigDeserializer<ForwardIndexConfig> createDeserializer() {
// reads tableConfig.fieldConfigList and decides what to create using the
FieldConfig properties and encoding
- ColumnConfigDeserializer<ForwardIndexConfig> fromFieldConfig =
IndexConfigDeserializer.fromCollection(
- TableConfig::getFieldConfigList,
- (accum, fieldConfig) -> {
+ ColumnConfigDeserializer<ForwardIndexConfig> fromOld = (tableConfig,
schema) -> {
+ Map<String, DictionaryIndexConfig> dictConfigs =
StandardIndexes.dictionary().getConfig(tableConfig, schema);
+
+ Map<String, ForwardIndexConfig> fwdConfig =
Maps.newHashMapWithExpectedSize(
+ Math.max(dictConfigs.size(), schema.size()));
+
+ Collection<FieldConfig> fieldConfigs = tableConfig.getFieldConfigList();
+ if (fieldConfigs != null) {
+ for (FieldConfig fieldConfig : fieldConfigs) {
Map<String, String> properties = fieldConfig.getProperties();
if (properties != null && isDisabled(properties)) {
- accum.put(fieldConfig.getName(), ForwardIndexConfig.DISABLED);
- } else if (fieldConfig.getEncodingType() ==
FieldConfig.EncodingType.RAW) {
- accum.put(fieldConfig.getName(),
createConfigFromFieldConfig(fieldConfig));
+ fwdConfig.put(fieldConfig.getName(), ForwardIndexConfig.DISABLED);
+ } else {
+ DictionaryIndexConfig dictConfig =
dictConfigs.get(fieldConfig.getName());
+ if (dictConfig != null && dictConfig.isDisabled()) {
+ fwdConfig.put(fieldConfig.getName(),
createConfigFromFieldConfig(fieldConfig));
+ }
+ // On other case encoding is DICTIONARY. We create the default
forward index by default. That means that if
+ // field config indicates for example a compression while using
encoding dictionary, the compression is
+ // ignored.
+ // It is important to do not explicitly add the default value here
in order to avoid exclusive problems with
+ // the default `fromIndexes` deserializer.
}
- // On other case encoding is DICTIONARY. We create the default
forward index by default. That means that if
- // field config indicates for example a compression while using
encoding dictionary, the compression is
- // ignored.
- // It is important to do not explicitly add the default value here
in order to avoid exclusive problems with
- // the default `fromIndexes` deserializer.
}
- );
+ }
+ return fwdConfig;
+ };
return IndexConfigDeserializer.fromIndexes(getPrettyName(),
getIndexConfigClass())
- .withExclusiveAlternative(fromFieldConfig);
+ .withExclusiveAlternative(fromOld);
}
private boolean isDisabled(Map<String, String> props) {
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/forward/ForwardIndexTypeTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/forward/ForwardIndexTypeTest.java
index 2210211d71..2f438a5b6a 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/forward/ForwardIndexTypeTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/forward/ForwardIndexTypeTest.java
@@ -20,7 +20,9 @@
package org.apache.pinot.segment.local.segment.index.forward;
import com.fasterxml.jackson.core.JsonProcessingException;
+import com.fasterxml.jackson.core.type.TypeReference;
import java.io.IOException;
+import java.util.Map;
import java.util.stream.Collectors;
import org.apache.pinot.segment.local.segment.index.AbstractSerdeIndexContract;
import org.apache.pinot.segment.spi.compression.ChunkCompressionType;
@@ -102,6 +104,51 @@ public class ForwardIndexTypeTest {
assertEquals(ForwardIndexConfig.DEFAULT);
}
+ @Test
+ public void oldConfNoDictionaryConfig()
+ throws IOException {
+ _tableConfig.getIndexingConfig().setNoDictionaryConfig(
+ JsonUtils.stringToObject("{\"dimInt\": \"RAW\"}",
+ new TypeReference<Map<String, String>>() {
+ })
+ );
+ addFieldIndexConfig(""
+ + " {\n"
+ + " \"name\": \"dimInt\","
+ + " \"compressionCodec\": \"SNAPPY\"\n"
+ + " }"
+ );
+
+ assertEquals(
+ new ForwardIndexConfig.Builder()
+ .withCompressionType(ChunkCompressionType.SNAPPY)
+ .withDeriveNumDocsPerChunk(false)
+ .withRawIndexWriterVersion(2)
+ .build()
+ );
+ }
+
+ @Test
+ public void oldConfNoDictionaryColumns()
+ throws IOException {
+ _tableConfig.getIndexingConfig().setNoDictionaryColumns(
+ JsonUtils.stringToObject("[\"dimInt\"]", _stringListTypeRef));
+ addFieldIndexConfig(""
+ + " {\n"
+ + " \"name\": \"dimInt\","
+ + " \"compressionCodec\": \"SNAPPY\"\n"
+ + " }"
+ );
+
+ assertEquals(
+ new ForwardIndexConfig.Builder()
+ .withCompressionType(ChunkCompressionType.SNAPPY)
+ .withDeriveNumDocsPerChunk(false)
+ .withRawIndexWriterVersion(2)
+ .build()
+ );
+ }
+
@Test
public void oldConfEnableDict()
throws IOException {
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
