You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by ja...@apache.org on 2023/06/09 06:51:09 UTC
[pinot] branch master updated: Fix #10713 by giving metainfo more priority than config (#10851)
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 a75544f2c6 Fix #10713 by giving metainfo more priority than config (#10851)
a75544f2c6 is described below
commit a75544f2c644874a88b61cea48b640d9cf05283c
Author: Gonzalo Ortiz Jaureguizar <go...@users.noreply.github.com>
AuthorDate: Thu Jun 8 23:51:03 2023 -0700
Fix #10713 by giving metainfo more priority than config (#10851)
---
.../org/apache/pinot/queries/FastHllQueriesTest.java | 4 ----
.../index/column/PhysicalColumnIndexContainer.java | 3 +++
.../local/segment/index/inverted/InvertedIndexType.java | 2 +-
.../segment/index/nullvalue/NullValueIndexType.java | 3 ---
.../segment/local/segment/index/text/TextIndexType.java | 6 ------
.../pinot/segment/spi/index/IndexReaderFactory.java | 17 +++++++++--------
6 files changed, 13 insertions(+), 22 deletions(-)
diff --git a/pinot-core/src/test/java/org/apache/pinot/queries/FastHllQueriesTest.java b/pinot-core/src/test/java/org/apache/pinot/queries/FastHllQueriesTest.java
index fb18993faf..f2f1370c5b 100644
--- a/pinot-core/src/test/java/org/apache/pinot/queries/FastHllQueriesTest.java
+++ b/pinot-core/src/test/java/org/apache/pinot/queries/FastHllQueriesTest.java
@@ -38,8 +38,6 @@ import org.apache.pinot.segment.spi.ImmutableSegment;
import org.apache.pinot.segment.spi.IndexSegment;
import org.apache.pinot.segment.spi.creator.SegmentGeneratorConfig;
import org.apache.pinot.segment.spi.creator.SegmentIndexCreationDriver;
-import org.apache.pinot.segment.spi.index.StandardIndexes;
-import org.apache.pinot.spi.config.table.IndexConfig;
import org.apache.pinot.spi.config.table.TableConfig;
import org.apache.pinot.spi.config.table.TableType;
import org.apache.pinot.spi.config.table.ingestion.IngestionConfig;
@@ -195,8 +193,6 @@ public class FastHllQueriesTest extends BaseQueriesTest {
segmentGeneratorConfig.setInputFilePath(filePath);
segmentGeneratorConfig.setTableName("testTable");
segmentGeneratorConfig.setOutDir(INDEX_DIR.getAbsolutePath());
- segmentGeneratorConfig.setIndexOn(StandardIndexes.inverted(), IndexConfig.ENABLED,
- Arrays.asList("column6", "column7", "column11", "column17", "column18"));
// Build the index segment
SegmentIndexCreationDriver driver = new SegmentIndexCreationDriverImpl();
diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/column/PhysicalColumnIndexContainer.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/column/PhysicalColumnIndexContainer.java
index 8112d51046..35077a2956 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/column/PhysicalColumnIndexContainer.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/column/PhysicalColumnIndexContainer.java
@@ -47,6 +47,9 @@ public final class PhysicalColumnIndexContainer implements ColumnIndexContainer
String columnName = metadata.getColumnName();
FieldIndexConfigs fieldIndexConfigs = indexLoadingConfig.getFieldIndexConfig(columnName);
+ if (fieldIndexConfigs == null) {
+ fieldIndexConfigs = FieldIndexConfigs.EMPTY;
+ }
_readersByIndex = new HashMap<>();
for (IndexType<?, ?, ?> indexType : IndexService.getInstance().getAllIndexes()) {
diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/inverted/InvertedIndexType.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/inverted/InvertedIndexType.java
index 6845bb63b3..fce5097e06 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/inverted/InvertedIndexType.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/inverted/InvertedIndexType.java
@@ -158,7 +158,7 @@ public class InvertedIndexType
public InvertedIndexReader createIndexReader(SegmentDirectory.Reader segmentReader,
FieldIndexConfigs fieldIndexConfigs, ColumnMetadata metadata)
throws IOException, IndexReaderConstraintException {
- if (fieldIndexConfigs == null || !fieldIndexConfigs.getConfig(StandardIndexes.inverted()).isEnabled()) {
+ if (!segmentReader.hasIndexFor(metadata.getColumnName(), StandardIndexes.inverted())) {
return null;
}
if (!metadata.hasDictionary()) {
diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/nullvalue/NullValueIndexType.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/nullvalue/NullValueIndexType.java
index fdde62cf7a..1c6b5faea9 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/nullvalue/NullValueIndexType.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/nullvalue/NullValueIndexType.java
@@ -115,9 +115,6 @@ public class NullValueIndexType extends AbstractIndexType<IndexConfig, NullValue
public NullValueVectorReader createIndexReader(SegmentDirectory.Reader segmentReader,
FieldIndexConfigs fieldIndexConfigs, ColumnMetadata metadata)
throws IOException {
- // TODO: Change this behavior and make it closer to other indexes.
- // For historical and test reasons, NullValueIndexType doesn't really care about its config
- // if there is a buffer for this index, it is read even if the config explicitly ask to disable it.
if (!segmentReader.hasIndexFor(metadata.getColumnName(), StandardIndexes.nullValueVector())) {
return null;
}
diff --git a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/text/TextIndexType.java b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/text/TextIndexType.java
index 522bbd1c01..e01adb1927 100644
--- a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/text/TextIndexType.java
+++ b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/text/TextIndexType.java
@@ -162,9 +162,6 @@ public class TextIndexType extends AbstractIndexType<TextIndexConfig, TextIndexR
public TextIndexReader createIndexReader(SegmentDirectory.Reader segmentReader,
FieldIndexConfigs fieldIndexConfigs, ColumnMetadata metadata)
throws IndexReaderConstraintException {
- if (fieldIndexConfigs == null) {
- return null;
- }
if (metadata.getDataType() != FieldSpec.DataType.STRING) {
throw new IndexReaderConstraintException(metadata.getColumnName(), StandardIndexes.text(),
"Text index is currently only supported on STRING type columns");
@@ -176,9 +173,6 @@ public class TextIndexType extends AbstractIndexType<TextIndexConfig, TextIndexR
return new NativeTextIndexReader(metadata.getColumnName(), segmentDir);
}
TextIndexConfig indexConfig = fieldIndexConfigs.getConfig(StandardIndexes.text());
- if (!indexConfig.isEnabled()) {
- return null;
- }
return new LuceneTextIndexReader(metadata.getColumnName(), segmentDir, metadata.getTotalDocs(), indexConfig);
}
}
diff --git a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/IndexReaderFactory.java b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/IndexReaderFactory.java
index 7e3e2871ea..c8fb5d6b58 100644
--- a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/IndexReaderFactory.java
+++ b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/IndexReaderFactory.java
@@ -30,8 +30,15 @@ import org.apache.pinot.spi.config.table.IndexConfig;
public interface IndexReaderFactory<R extends IndexReader> {
/**
+ * Tries to create an index reader for the given column and segment.
+ *
+ * It may be the case that the configuration indicates that the index should be disabled but it is actually there.
+ * Also, it may be the case that the configuration says that the index should be there but it is not in the reader.
+ * In both cases the source of truth is the segment reader.
+ *
* @throws IndexReaderConstraintException if the constraints of the index reader are not matched. For example, some
* indexes may require the column to be dictionary based.
+ * @return the index reader or null if there is no index for that column
*/
@Nullable
R createIndexReader(SegmentDirectory.Reader segmentReader, FieldIndexConfigs fieldIndexConfigs,
@@ -50,20 +57,14 @@ public interface IndexReaderFactory<R extends IndexReader> {
ColumnMetadata metadata)
throws IOException, IndexReaderConstraintException {
IndexType<C, R, ?> indexType = getIndexType();
- C indexConf;
- if (fieldIndexConfigs == null) {
- indexConf = getIndexType().getDefaultConfig();
- } else {
- indexConf = fieldIndexConfigs.getConfig(indexType);
- }
- if (indexConf == null || !indexConf.isEnabled()) { //it is either not enabled or the default value is null
+ if (!segmentReader.hasIndexFor(metadata.getColumnName(), indexType)) { // there is no buffer for this index
return null;
}
PinotDataBuffer buffer = segmentReader.getIndexFor(metadata.getColumnName(), indexType);
try {
- return createIndexReader(buffer, metadata, indexConf);
+ return createIndexReader(buffer, metadata, fieldIndexConfigs.getConfig(indexType));
} catch (RuntimeException ex) {
throw new RuntimeException(
"Cannot read index " + indexType + " for column " + metadata.getColumnName(), ex);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org