This is an automated email from the ASF dual-hosted git repository.
raghavyadav01 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 21596bbffad Configure OPEN_STRUCT indexes the way every other column
is configured (#19736)
21596bbffad is described below
commit 21596bbffad24b71c9f7c04addcee1d4830801a5
Author: RAGHVENDRA KUMAR YADAV <[email protected]>
AuthorDate: Tue Oct 6 09:32:16 2026 -0700
Configure OPEN_STRUCT indexes the way every other column is configured
(#19736)
* Let an OPEN_STRUCT key ask for its dictionary the way every other column
does
A dictionary is enabled on an ordinary column by putting it in `indexes`.
An OPEN_STRUCT key could not be configured that way: fromFieldConfig, which
builds a key's index configs, skipped the dictionary type outright and took
the answer from encodingType alone, and the validator then rejected an
indexes.dictionary entry that did not already agree with it.
So the two spellings disagreed depending on which kind of column you were
configuring, and the key spelling failed silently -- you got a raw key and
no diagnostic.
An enabled inverted index hid half of it, since it forces a dictionary on
its own. A key whose only index is a range one had nothing to rescue it:
the dictionary entry was ignored, the key stayed raw, and the range index
was asked for over a dictionary that did not exist. That case is
testARangeOnlyKeyCanAskForADictionaryUnderIndexes, and it is the one that
fails when the production change alone is reverted.
indexes.dictionary can only turn a dictionary *on*. Turning one off stays
with encodingType, so a `disabled` entry contradicting the column's own
encoding is still a contradiction and still refused rather than quietly
winning.
The forward index follows the dictionary decision rather than the declared
encoding, for the reason it already followed encodingType: a key whose
dictionary was enabled this way needs a dict-encoded forward index, or a
reload rebuilds one over a dictionary that is not there.
369 tests green across the OPEN_STRUCT, config and loader suites.
* Let the shared sparse blob column carry its own index settings
An OPEN_STRUCT column splits into materialized per-key child columns and
one shared blob column holding every key that was not materialized. The
per-key children already accept a FieldConfig each, so a key can ask for
a dictionary, a range index, a bloom filter and so on. The blob column
could not: index loading skipped it outright, so the only way to search
the keys living inside it was a full scan.
This adds a sparseFieldConfig to OpenStructIndexConfig. It is shaped
like the FieldConfig of any other column, and it configures the blob the
same way a per-key entry configures a child:
"openStruct": {
"sparseFieldConfig": {
"name": "props$__sparse__",
"indexes": { "json": {} }
}
}
The blob is always RAW regardless of what the config asks for -- it is a
serialized document per row, so a dictionary over it would be a
dictionary of whole documents and buys nothing. The existing
sparseJsonIndex flag still works and now means exactly indexes.json={},
so the two spellings converge on one code path instead of two.
Adds four tests covering a JSON index built on the blob through either
spelling, and keeps the existing per-key cases green.
* Serve a sparse key from whichever JSON index the blob was given
The sparse fast path in MapFilterOperator asks the OPEN_STRUCT data source
for
the blob's JSON index, and that lookup only ever returned the standard one.
An ordinary column does not stop there: tryJsonIndex falls back to the
composite JSON index, which reads as a JsonIndexReader and answers the same
predicates.
So a blob given a composite JSON index built it and was then never asked.
The
key fell back to a full scan, silently costing exactly what the index was
configured to avoid -- no error, no plan difference a reader would notice,
just the slow path.
Make the blob lookup do what the column lookup already does. The index ships
as a plugin and is absent from most deployments, so it is resolved by id and
the fallback is inert where nothing registered it.
* Do not ask a JSON index for a key it holds no postings for
The standard JSON index indexes every path, so asking it about any key is
safe. An index that indexes a configured subset of paths is not: asked about
a path it never indexed, it answers with an empty bitmap, and an empty
bitmap
is indistinguishable from "no document matches this". The query returns zero
rows, the plan says the index served the filter, and nothing anywhere says
the
answer is wrong.
`JsonIndexReader.isPathIndexed` already exists to answer exactly this, and
defaults to true so a fully-indexed reader is unaffected. Nothing was
calling
it. Both JSON fast paths now do -- the one over a column and the one over an
OPEN_STRUCT sparse blob -- and a key the index does not cover falls back to
the scan, which is slower and right.
The two existing tests that expect the index to serve now stub
`isPathIndexed`:
Mockito answers false for an unstubbed boolean, interface default or not.
---
.../core/operator/filter/MapFilterOperator.java | 17 ++-
.../filter/MapFilterOperatorOpenStructTest.java | 22 ++++
.../segment/index/loader/IndexLoadingConfig.java | 45 +++++++-
.../openstruct/ImmutableOpenStructDataSource.java | 21 +++-
.../index/openstruct/OpenStructIndexType.java | 8 +-
.../index/openstruct/OpenStructIndexTypeTest.java | 3 -
.../OpenStructPerKeyIndexReloadTest.java | 116 +++++++++++++++++++++
.../segment/spi/index/FieldIndexConfigsUtil.java | 26 ++++-
.../spi/config/table/OpenStructIndexConfig.java | 30 +++++-
9 files changed, 272 insertions(+), 16 deletions(-)
diff --git
a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/MapFilterOperator.java
b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/MapFilterOperator.java
index 7eca5c6100d..b248684377b 100644
---
a/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/MapFilterOperator.java
+++
b/pinot-core/src/main/java/org/apache/pinot/core/operator/filter/MapFilterOperator.java
@@ -55,6 +55,9 @@ import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
/// 3. JSON index (JsonMatchFilterOperator)
/// 4. Expression scan fallback (ExpressionFilterOperator)
public class MapFilterOperator extends BaseFilterOperator {
+ /// A key's JSON path, in the form an index's configured paths are written
in ("$.user_tier").
+ private static final String JSON_PATH_PREFIX = "$.";
+
private static final String EXPLAIN_NAME = "FILTER_MAP";
private enum DelegateType {
@@ -161,7 +164,7 @@ public class MapFilterOperator extends BaseFilterOperator {
private BaseFilterOperator trySparseJsonIndex(OpenStructDataSource osDs,
DataSource sparseKeyDs,
QueryContext queryContext, int numDocs) {
JsonIndexReader jsonIndex = osDs.getSparseJsonIndex();
- if (jsonIndex == null) {
+ if (jsonIndex == null || !servesKey(jsonIndex)) {
return null;
}
if (sparseKeyDs.getDataSourceMetadata().getDataType().getStoredType() !=
FieldSpec.DataType.STRING) {
@@ -250,13 +253,23 @@ public class MapFilterOperator extends BaseFilterOperator
{
jsonIndex = (JsonIndexReader)
dataSource.getIndex(compositeIndex.get());
}
}
- if (jsonIndex == null) {
+ if (jsonIndex == null || !servesKey(jsonIndex)) {
return null;
}
FilterContext filterContext = createFilterContext();
return new JsonMatchFilterOperator(jsonIndex, filterContext, numDocs);
}
+ /// Whether this index actually holds postings for the key being filtered on.
+ ///
+ /// An index that indexes every path -- the standard JSON index -- always
does. One that indexes a configured
+ /// subset answers an unindexed path with an empty bitmap, which is
indistinguishable from "no document matches":
+ /// the query returns zero rows and nothing says why. Asking first turns
that into a scan, which is slower and
+ /// right.
+ private boolean servesKey(JsonIndexReader jsonIndex) {
+ return jsonIndex.isPathIndexed(JSON_PATH_PREFIX + _keyName);
+ }
+
private FilterContext createFilterContext() {
Predicate rewritten = rewritePredicateForKey(_predicate);
if (rewritten == null) {
diff --git
a/pinot-core/src/test/java/org/apache/pinot/core/operator/filter/MapFilterOperatorOpenStructTest.java
b/pinot-core/src/test/java/org/apache/pinot/core/operator/filter/MapFilterOperatorOpenStructTest.java
index 2df020993d2..d818936b836 100644
---
a/pinot-core/src/test/java/org/apache/pinot/core/operator/filter/MapFilterOperatorOpenStructTest.java
+++
b/pinot-core/src/test/java/org/apache/pinot/core/operator/filter/MapFilterOperatorOpenStructTest.java
@@ -614,6 +614,8 @@ public class MapFilterOperatorOpenStructTest {
postings.add(0);
postings.add(4);
when(jsonIndex.getMatchingDocIds(any(FilterContext.class))).thenReturn(postings);
+ // Mockito answers false for an unstubbed boolean, including an interface
default that returns true.
+ when(jsonIndex.isPathIndexed(anyString())).thenReturn(true);
OpenStructDataSource osDs =
withSparseJsonIndex(mockSparseSegmentSource(List.of("region"), Map.of()),
jsonIndex);
MapFilterOperator op = new MapFilterOperator(mockSegment(osDs),
@@ -629,6 +631,8 @@ public class MapFilterOperatorOpenStructTest {
MutableRoaringBitmap postings = new MutableRoaringBitmap();
postings.add(0);
when(jsonIndex.getMatchingDocIds(any(FilterContext.class))).thenReturn(postings);
+ // Mockito answers false for an unstubbed boolean, including an interface
default that returns true.
+ when(jsonIndex.isPathIndexed(anyString())).thenReturn(true);
OpenStructDataSource osDs =
withSparseJsonIndex(mockSparseSegmentSource(List.of("region"), Map.of()),
jsonIndex);
MapFilterOperator op = new MapFilterOperator(mockSegment(osDs),
@@ -638,6 +642,24 @@ public class MapFilterOperatorOpenStructTest {
assertEquals(countMatches(op), NUM_DOCS - 1);
}
+ /// An index that holds postings for only some paths answers an unindexed
one with an empty bitmap, which reads
+ /// exactly like "nothing matched". Taking that answer would return zero
rows and report no reason, so the key
+ /// has to fall back to the scan instead.
+ @Test
+ public void testSparseJsonIndexIsRefusedForAnUnindexedKey() {
+ JsonIndexReader jsonIndex = mock(JsonIndexReader.class);
+ when(jsonIndex.getMatchingDocIds(any(FilterContext.class))).thenReturn(new
MutableRoaringBitmap());
+ when(jsonIndex.isPathIndexed(anyString())).thenReturn(false);
+
+ OpenStructDataSource osDs =
+ withSparseJsonIndex(mockSparseSegmentSource(List.of("region"),
Map.of()), jsonIndex);
+ MapFilterOperator op = new MapFilterOperator(mockSegment(osDs),
+ makeEqPredicate(COLUMN, "region", "us"), mockQueryContext(), NUM_DOCS);
+
+ assertFalse(op.toExplainString().contains("delegateTo:json_match"),
+ "an index without postings for this key must not be asked");
+ }
+
@Test
public void testSparseJsonIndexRefusals() {
JsonIndexReader jsonIndex = mock(JsonIndexReader.class);
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/IndexLoadingConfig.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/IndexLoadingConfig.java
index e68480ace43..4e868d34bfa 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/IndexLoadingConfig.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/loader/IndexLoadingConfig.java
@@ -18,6 +18,7 @@
*/
package org.apache.pinot.segment.local.segment.index.loader;
+import com.fasterxml.jackson.databind.node.ObjectNode;
import com.google.common.annotations.VisibleForTesting;
import java.util.Collections;
import java.util.HashMap;
@@ -51,6 +52,7 @@ import org.apache.pinot.spi.data.FieldSpec;
import org.apache.pinot.spi.data.FieldSpec.DataType;
import org.apache.pinot.spi.data.OpenStructNaming;
import org.apache.pinot.spi.data.Schema;
+import org.apache.pinot.spi.utils.JsonUtils;
import org.apache.pinot.spi.utils.ReadMode;
import org.apache.pinot.spi.utils.TimestampIndexUtils;
@@ -495,9 +497,6 @@ public class IndexLoadingConfig {
if (!childColumn.contains(OpenStructNaming.SEPARATOR) ||
indexConfigsByColName.containsKey(childColumn)) {
continue;
}
- if (OpenStructNaming.isSparseColumn(childColumn)) {
- continue;
- }
String parentColumn = OpenStructNaming.parseParentColumn(childColumn);
FieldIndexConfigs parentConfigs =
indexConfigsByColName.get(parentColumn);
if (parentConfigs == null) {
@@ -508,6 +507,16 @@ public class IndexLoadingConfig {
continue;
}
OpenStructIndexConfig openStructConfig = (OpenStructIndexConfig)
osConfig;
+ if (OpenStructNaming.isSparseColumn(childColumn)) {
+ FieldIndexConfigs blobConfigs = sparseChildConfigs(openStructConfig,
entry.getValue().getFieldSpec());
+ if (blobConfigs != null) {
+ if (updatedConfigs == null) {
+ updatedConfigs = new HashMap<>(indexConfigsByColName);
+ }
+ updatedConfigs.put(childColumn, blobConfigs);
+ }
+ continue;
+ }
String key = OpenStructNaming.parseKey(childColumn);
FieldConfig keyFieldConfig = openStructConfig.getValueFieldConfig(key);
if (keyFieldConfig == null) {
@@ -536,6 +545,36 @@ public class IndexLoadingConfig {
_resolvedIndexState =
withOpenStructChildConfigs(segmentMetadata)._resolvedIndexState;
}
+ /// Index configs for the shared blob column, or null when the table asked
for nothing on it.
+ ///
+ /// The blob is an ordinary single-value STRING column holding each row's
document as JSON, so an index that
+ /// reads JSON out of a STRING column applies to it unchanged -- a JSON
index, or any other registered index
+ /// type named under `sparseFieldConfig.indexes`. What it must never get is
a dictionary: the values are whole
+ /// documents, nearly all distinct, and a forward index of dict ids over
them costs more than it saves. So the
+ /// encoding is forced RAW here rather than taken from the config.
+ @Nullable
+ private static FieldIndexConfigs sparseChildConfigs(OpenStructIndexConfig
openStructConfig,
+ FieldSpec blobFieldSpec) {
+ FieldConfig sparseFieldConfig = openStructConfig.getSparseFieldConfig();
+ boolean legacyJsonIndex = openStructConfig.isSparseJsonIndex();
+ if (sparseFieldConfig == null && !legacyJsonIndex) {
+ return null;
+ }
+ FieldConfig.Builder builder = sparseFieldConfig != null
+ ? new FieldConfig.Builder(sparseFieldConfig) : new
FieldConfig.Builder(blobFieldSpec.getName());
+ builder.withEncodingType(FieldConfig.EncodingType.RAW);
+ if (legacyJsonIndex) {
+ // `sparseJsonIndex: true` predates sparseFieldConfig and means exactly
one thing: a JSON index on the blob.
+ ObjectNode indexes = sparseFieldConfig != null &&
sparseFieldConfig.getIndexes() instanceof ObjectNode existing
+ ? existing.deepCopy() : JsonUtils.newObjectNode();
+ if (!indexes.has(StandardIndexes.json().getPrettyName())) {
+ indexes.set(StandardIndexes.json().getPrettyName(),
JsonUtils.newObjectNode());
+ }
+ builder.withIndexes(indexes);
+ }
+ return FieldIndexConfigsUtil.fromFieldConfig(builder.build(),
blobFieldSpec);
+ }
+
public IndexLoadingConfig withKnownColumns(Set<String> columns) {
if (_knownColumns != null && _knownColumns.containsAll(columns)) {
return this;
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/ImmutableOpenStructDataSource.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/ImmutableOpenStructDataSource.java
index 3176bbf9c88..7bd3274325b 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/ImmutableOpenStructDataSource.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/ImmutableOpenStructDataSource.java
@@ -22,6 +22,7 @@ import java.io.IOException;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
+import java.util.Optional;
import java.util.Set;
import java.util.concurrent.ConcurrentHashMap;
import javax.annotation.Nullable;
@@ -33,6 +34,8 @@ import org.apache.pinot.segment.spi.Constants;
import org.apache.pinot.segment.spi.datasource.DataSource;
import org.apache.pinot.segment.spi.datasource.DataSourceMetadata;
import org.apache.pinot.segment.spi.datasource.OpenStructDataSource;
+import org.apache.pinot.segment.spi.index.IndexService;
+import org.apache.pinot.segment.spi.index.IndexType;
import org.apache.pinot.segment.spi.index.column.ColumnIndexContainer;
import org.apache.pinot.segment.spi.index.reader.ForwardIndexReader;
import org.apache.pinot.segment.spi.index.reader.JsonIndexReader;
@@ -47,6 +50,10 @@ import org.apache.pinot.spi.utils.JsonUtils;
/// get virtual [SparseKeyDataSource]s backed by the shared blob parser; keys
absent from the segment (no sparse blob,
/// or not listed in the sparse manifest) resolve to an all-null
[NullDataSource].
public class ImmutableOpenStructDataSource extends BaseDataSource implements
OpenStructDataSource {
+ /// Looked up by id rather than referenced: the composite JSON index ships
as a plugin and is absent from most
+ /// deployments, so this resolves to nothing unless one registered it.
+ private static final String COMPOSITE_JSON_INDEX_ID = "composite_json_index";
+
private final ComplexFieldSpec _fieldSpec;
private final Map<String, DataSource> _perKeyDataSources;
@Nullable
@@ -189,7 +196,19 @@ public class ImmutableOpenStructDataSource extends
BaseDataSource implements Ope
@Override
@Nullable
public JsonIndexReader getSparseJsonIndex() {
- return _sparseDataSource != null ? _sparseDataSource.getJsonIndex() : null;
+ if (_sparseDataSource == null) {
+ return null;
+ }
+ JsonIndexReader jsonIndex = _sparseDataSource.getJsonIndex();
+ if (jsonIndex != null) {
+ return jsonIndex;
+ }
+ // A composite JSON index reads as a [JsonIndexReader] and answers the
same predicates, so a sparse key is
+ // served by whichever of the two the blob was given. This is the fallback
an ordinary column already gets in
+ // [org.apache.pinot.core.operator.filter.MapFilterOperator]; without it
here the index is built on the blob and
+ // then never consulted, and the key falls back to a full scan that
silently costs what the index was for.
+ Optional<IndexType<?, ?, ?>> compositeIndex =
IndexService.getInstance().getOptional(COMPOSITE_JSON_INDEX_ID);
+ return compositeIndex.map(indexType -> (JsonIndexReader)
_sparseDataSource.getIndex(indexType)).orElse(null);
}
@Nullable
diff --git
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/OpenStructIndexType.java
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/OpenStructIndexType.java
index 853b52ab7f3..dfb66045868 100644
---
a/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/OpenStructIndexType.java
+++
b/pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/OpenStructIndexType.java
@@ -60,8 +60,9 @@ public class OpenStructIndexType
private static final List<String> EXTENSIONS = List.of(".open_struct.idx");
private static final String FIXED_FORWARD_INDEX =
"materialized keys always use a dictionary-encoded or LZ4 raw forward
index";
- private static final String KEY_ENCODING_RULE = "a key is dictionary-encoded
when its encodingType is DICTIONARY or "
- + "its inverted index is enabled, and raw with LZ4 otherwise";
+ private static final String KEY_ENCODING_RULE = "a key is dictionary-encoded
when its encodingType is "
+ + "DICTIONARY, or indexes.dictionary enables one, or its inverted index
is enabled, and raw with LZ4 "
+ + "otherwise";
protected OpenStructIndexType() {
super(StandardIndexes.OPEN_STRUCT_ID);
@@ -175,7 +176,10 @@ public class OpenStructIndexType
// Mirrors OpenStructColumnSplitter: a key gets a dictionary and a
dictionary-encoded forward index when its
// encodingType is DICTIONARY or its inverted index is enabled. Inverted
is the only vetted per-key index that
// requires a dictionary, so it overrides a RAW encodingType.
+ JsonNode declaredDictionary = indexes.get(StandardIndexes.DICTIONARY_ID);
boolean dictionaryEncoded = fieldConfig.getEncodingType() !=
FieldConfig.EncodingType.RAW
+ || (declaredDictionary != null && declaredDictionary.isObject()
+ && !declaredDictionary.path("disabled").asBoolean(false))
|| isEnabled(indexes.get(StandardIndexes.inverted().getPrettyName()));
if (forwardIndex != null && !forwardIndex.isNull()) {
validatePerKeyForwardIndex(forwardIndex, dictionaryEncoded, column,
target);
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/openstruct/OpenStructIndexTypeTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/openstruct/OpenStructIndexTypeTest.java
index c7f772bbc96..139f6e7a747 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/openstruct/OpenStructIndexTypeTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/openstruct/OpenStructIndexTypeTest.java
@@ -261,9 +261,6 @@ public class OpenStructIndexTypeTest {
{"{'name': 'clicks', 'indexes': {'dictionary': {'disabled': true}}}",
"OPEN_STRUCT column 'payload': indexes.dictionary of key 'clicks'
disables the dictionary, but the key is "
+ "built with one; "},
- {"{'name': 'clicks', 'encodingType': 'RAW', 'indexes': {'dictionary':
{}}}",
- "OPEN_STRUCT column 'payload': indexes.dictionary of key 'clicks'
enables the dictionary, but the key is "
- + "built without one; "},
// An enabled inverted index requires a dictionary, so a RAW key with
one is still built with a dictionary.
{"{'name': 'clicks', 'encodingType': 'RAW', 'indexes': {'inverted':
{}, 'dictionary': {'disabled': true}}}",
"OPEN_STRUCT column 'payload': indexes.dictionary of key 'clicks'
disables the dictionary, but the key is "
diff --git
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/openstruct/OpenStructPerKeyIndexReloadTest.java
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/openstruct/OpenStructPerKeyIndexReloadTest.java
index 54d710a237c..8f59da54d97 100644
---
a/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/openstruct/OpenStructPerKeyIndexReloadTest.java
+++
b/pinot-segment-local/src/test/java/org/apache/pinot/segment/local/segment/index/openstruct/OpenStructPerKeyIndexReloadTest.java
@@ -166,6 +166,19 @@ public class OpenStructPerKeyIndexReloadTest {
assertFalse(hasIndex(segmentDir, host, StandardIndexes.dictionary()), "the
raw key must stay raw");
}
+ private static List<FieldConfig> mixedMatrixKeys() {
+ FieldConfig views = new FieldConfig.Builder("views")
+ .withIndexes(JsonUtils.objectToJsonNode(Map.of("inverted", Map.of())))
+ .build();
+ FieldConfig cpu = new FieldConfig.Builder("cpu")
+ .withEncodingType(FieldConfig.EncodingType.DICTIONARY)
+ .build();
+ FieldConfig host = new FieldConfig.Builder("host")
+ .withEncodingType(FieldConfig.EncodingType.RAW)
+ .build();
+ return List.of(views, cpu, host);
+ }
+
private static OpenStructIndexConfig withMixedMatrix() {
FieldConfig views = new FieldConfig.Builder("views")
.withIndexes(JsonUtils.objectToJsonNode(Map.of("inverted", Map.of())))
@@ -180,6 +193,109 @@ public class OpenStructPerKeyIndexReloadTest {
List.of(views, cpu, host));
}
+ /// The spelling every other kind of column uses: RAW encoding, with the
dictionary asked for under `indexes`.
+ ///
+ /// An External Table requires RAW on a column's own field config, so this
is the shape a reader who has
+ /// configured one before will reach for. An enabled inverted index already
forced a dictionary on its own, so
+ /// this combination worked before `indexes.dictionary` was honoured; it is
here to keep it working.
+ @Test
+ public void testADictionaryAskedForUnderIndexesIsBuilt()
+ throws Exception {
+ File segmentDir = buildSegment("dictionaryViaIndexes",
withoutPerKeyIndex());
+ reload(segmentDir, rawKeyWithDictionaryUnderIndexes());
+
+ assertTrue(hasIndex(segmentDir, CHILD, StandardIndexes.dictionary()),
+ "indexes.dictionary must enable the dictionary on a RAW key");
+ assertTrue(hasIndex(segmentDir, CHILD, StandardIndexes.inverted()),
+ "and the inverted index built on top of it must exist");
+ }
+
+ /// The same spelling on a key whose only index is a range one, and the case
that was actually broken.
+ ///
+ /// Unlike an inverted index, a range index needs a dictionary without
forcing one, so nothing rescued a key
+ /// written this way: the dictionary entry was ignored, the key stayed raw,
and the range index was built over
+ /// no dictionary at all. Reverting the fix fails this test and not the one
above it.
+ @Test
+ public void testARangeOnlyKeyCanAskForADictionaryUnderIndexes()
+ throws Exception {
+ File segmentDir = buildSegment("rangeViaIndexes", withoutPerKeyIndex());
+ reload(segmentDir, rawRangeKeyWithDictionaryUnderIndexes());
+
+ assertTrue(hasIndex(segmentDir,
OpenStructNaming.materializedColumnName(COLUMN, "views"),
+ StandardIndexes.dictionary()), "a range-only key may ask for a
dictionary the same way");
+ }
+
+ private static OpenStructIndexConfig rawKeyWithDictionaryUnderIndexes() {
+ ObjectNode indexes = JsonUtils.newObjectNode();
+ indexes.set("dictionary", JsonUtils.newObjectNode());
+ indexes.set("inverted", JsonUtils.newObjectNode());
+ FieldConfig keyConfig = new FieldConfig.Builder(KEY)
+ .withEncodingType(FieldConfig.EncodingType.RAW)
+ .withIndexes(indexes)
+ .build();
+ return new OpenStructIndexConfig(false, null, -1, Set.of(KEY), 0.0,
List.of(keyConfig));
+ }
+
+ private static OpenStructIndexConfig rawRangeKeyWithDictionaryUnderIndexes()
{
+ ObjectNode indexes = JsonUtils.newObjectNode();
+ indexes.set("dictionary", JsonUtils.newObjectNode());
+ indexes.set("range", JsonUtils.newObjectNode());
+ FieldConfig keyConfig = new FieldConfig.Builder("views")
+ .withEncodingType(FieldConfig.EncodingType.RAW)
+ .withIndexes(indexes)
+ .build();
+ return new OpenStructIndexConfig(false, null, -1, Set.of("views"), 0.0,
List.of(keyConfig));
+ }
+
+ /// The shared blob is an ordinary single-value STRING column holding each
row's document, so an index that
+ /// reads JSON out of a STRING column applies to it. Without one, every
predicate on an unshredded key scans.
+ @Test
+ public void testAJsonIndexCanBeBuiltOnTheSparseBlob()
+ throws Exception {
+ // The blob only exists when some key is left out of the dense set, so the
segment is built with a budget.
+ File segmentDir = buildSegment("blobJsonIndex", withMixedMatrix());
+ String sparse = OpenStructNaming.sparseColumnName(COLUMN);
+ assertTrue(new
SegmentMetadataImpl(segmentDir).getColumnMetadataMap().containsKey(sparse),
+ "precondition: a key was left unmaterialized, so the blob column
exists");
+ assertFalse(hasIndex(segmentDir, sparse, StandardIndexes.json()),
+ "precondition: the blob starts with no index");
+
+ reload(segmentDir, withSparseJsonIndex());
+
+ assertTrue(hasIndex(segmentDir, sparse, StandardIndexes.json()),
+ "sparseJsonIndex must build a JSON index over the blob");
+ assertFalse(hasIndex(segmentDir, sparse, StandardIndexes.dictionary()),
+ "and the blob must never get a dictionary -- its values are whole
documents");
+ }
+
+ /// `sparseFieldConfig` names the index directly, so any registered index
type that accepts a STRING column can
+ /// be asked for on the blob rather than only the one `sparseJsonIndex`
hard-codes.
+ @Test
+ public void testTheBlobIndexCanBeNamedDirectly()
+ throws Exception {
+ File segmentDir = buildSegment("blobNamedIndex", withMixedMatrix());
+ reload(segmentDir, withSparseFieldConfig("json"));
+
+ assertTrue(hasIndex(segmentDir, OpenStructNaming.sparseColumnName(COLUMN),
StandardIndexes.json()),
+ "an index named under sparseFieldConfig.indexes must be built on the
blob");
+ }
+
+ private static OpenStructIndexConfig withSparseJsonIndex() {
+ return new OpenStructIndexConfig(false, null, 3, Set.of("views", "cpu",
"host"), 0.5,
+ mixedMatrixKeys(), true);
+ }
+
+ private static OpenStructIndexConfig withSparseFieldConfig(String indexName)
{
+ ObjectNode indexes = JsonUtils.newObjectNode();
+ indexes.set(indexName, JsonUtils.newObjectNode());
+ FieldConfig blob = new
FieldConfig.Builder(OpenStructNaming.sparseColumnName(COLUMN))
+ .withEncodingType(FieldConfig.EncodingType.RAW)
+ .withIndexes(indexes)
+ .build();
+ return new OpenStructIndexConfig(false, null, 3, Set.of("views", "cpu",
"host"), 0.5,
+ mixedMatrixKeys(), null, null, null, null, blob);
+ }
+
// ---------------------------------------------------------------- fixtures
private static OpenStructIndexConfig withoutPerKeyIndex() {
diff --git
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/FieldIndexConfigsUtil.java
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/FieldIndexConfigsUtil.java
index 3bc61ae3fbb..a79858a2d83 100644
---
a/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/FieldIndexConfigsUtil.java
+++
b/pinot-segment-spi/src/main/java/org/apache/pinot/segment/spi/index/FieldIndexConfigsUtil.java
@@ -68,13 +68,22 @@ public class FieldIndexConfigsUtil {
/// synthetic columns (e.g. OPEN_STRUCT materialized children) that exist in
no schema.
public static FieldIndexConfigs fromFieldConfig(@Nullable FieldConfig
fieldConfig, FieldSpec fieldSpec) {
FieldIndexConfigs.Builder builder = new FieldIndexConfigs.Builder();
- FieldConfig.EncodingType encodingType =
+ FieldConfig.EncodingType declaredEncoding =
fieldConfig != null && fieldConfig.getEncodingType() != null ?
fieldConfig.getEncodingType()
: FieldConfig.EncodingType.DICTIONARY;
- boolean rawEncoded = encodingType == FieldConfig.EncodingType.RAW;
- builder.add(StandardIndexes.dictionary(),
- rawEncoded ? DictionaryIndexConfig.DISABLED :
DictionaryIndexConfig.DEFAULT);
JsonNode indexes = fieldConfig != null ? fieldConfig.getIndexes() : null;
+ // `indexes.dictionary` decides when it is there, and the column-level
encoding decides otherwise. Both spellings
+ // have to work: every other column turns a dictionary on through
`indexes`, and a reader who has learned that
+ // will write it here too -- silently ignoring it leaves them with a raw
column and no clue why.
+ boolean dictionaryEncoded =
+ declaredEncoding != FieldConfig.EncodingType.RAW ||
dictionaryEnabledByIndexes(indexes);
+ builder.add(StandardIndexes.dictionary(),
+ dictionaryEncoded ? DictionaryIndexConfig.DEFAULT :
DictionaryIndexConfig.DISABLED);
+ // The forward index follows the dictionary decision, not the declared
encoding: a key declared RAW whose
+ // dictionary was turned on through `indexes` needs a dict-encoded forward
index, or a reload rebuilds one over
+ // a dictionary that is not there ("Dictionary should still exist after
rebuilding dict-encoded forward index").
+ FieldConfig.EncodingType encodingType =
+ dictionaryEncoded ? FieldConfig.EncodingType.DICTIONARY :
FieldConfig.EncodingType.RAW;
for (IndexType<?, ?, ?> indexType :
IndexService.getInstance().getAllIndexes()) {
if (indexType.getId().equals(StandardIndexes.DICTIONARY_ID)) {
continue;
@@ -93,6 +102,15 @@ public class FieldIndexConfigsUtil {
return builder.build();
}
+ /// Whether `indexes.dictionary` asks for a dictionary. It can only turn one
*on*: that is how a dictionary is
+ /// enabled on every other kind of column, and a reader who has learned that
spelling will use it here too.
+ /// Turning one off is left to `encodingType`, so a `disabled` entry that
contradicts the column's own encoding
+ /// stays a contradiction rather than silently winning.
+ private static boolean dictionaryEnabledByIndexes(@Nullable JsonNode
indexes) {
+ JsonNode dictionary = indexes != null ?
indexes.get(StandardIndexes.DICTIONARY_ID) : null;
+ return dictionary != null && dictionary.isObject() &&
!dictionary.path("disabled").asBoolean(false);
+ }
+
private static ForwardIndexConfig forwardConfig(IndexType<?, ?, ?>
forwardIndexType, @Nullable JsonNode indexes,
FieldConfig.EncodingType encodingType) {
JsonNode node = indexes != null ?
indexes.get(forwardIndexType.getPrettyName()) : null;
diff --git
a/pinot-spi/src/main/java/org/apache/pinot/spi/config/table/OpenStructIndexConfig.java
b/pinot-spi/src/main/java/org/apache/pinot/spi/config/table/OpenStructIndexConfig.java
index a60d6474580..f94770c6a56 100644
---
a/pinot-spi/src/main/java/org/apache/pinot/spi/config/table/OpenStructIndexConfig.java
+++
b/pinot-spi/src/main/java/org/apache/pinot/spi/config/table/OpenStructIndexConfig.java
@@ -85,6 +85,9 @@ public class OpenStructIndexConfig extends IndexConfig {
private final double _denseKeyMinFillRate;
private final List<FieldConfig> _valueFieldConfigs;
private final boolean _sparseJsonIndex;
+ /// Index settings for the shared sparse blob column, shaped like a
[FieldConfig] for any other column.
+ @Nullable
+ private final FieldConfig _sparseFieldConfig;
private final boolean _perKeyMetricsEnabled;
private final Set<String> _ignoredKeys;
private final int _maxNestedKeyDepth;
@@ -138,6 +141,18 @@ public class OpenStructIndexConfig extends IndexConfig {
sparseJsonIndex, perKeyMetricsEnabled, ignoredKeys, null);
}
+ /// @deprecated Use the 11-arg constructor accepting `sparseFieldConfig`.
Kept for binary compatibility with
+ /// callers built against the pre-`sparseFieldConfig` signature.
+ @Deprecated
+ public OpenStructIndexConfig(Boolean disabled, @Nullable FieldConfig
defaultValueFieldConfig,
+ @Nullable Integer maxDenseKeys, @Nullable Set<String> denseKeys,
@Nullable Double denseKeyMinFillRate,
+ @Nullable List<FieldConfig> valueFieldConfigs, @Nullable Boolean
sparseJsonIndex,
+ @Nullable Boolean perKeyMetricsEnabled, @Nullable Set<String>
ignoredKeys,
+ @Nullable Integer maxNestedKeyDepth) {
+ this(disabled, defaultValueFieldConfig, maxDenseKeys, denseKeys,
denseKeyMinFillRate, valueFieldConfigs,
+ sparseJsonIndex, perKeyMetricsEnabled, ignoredKeys, maxNestedKeyDepth,
null);
+ }
+
@JsonCreator
public OpenStructIndexConfig(
@JsonProperty("disabled") Boolean disabled,
@@ -149,8 +164,10 @@ public class OpenStructIndexConfig extends IndexConfig {
@JsonProperty("sparseJsonIndex") @Nullable Boolean sparseJsonIndex,
@JsonProperty("perKeyMetricsEnabled") @Nullable Boolean
perKeyMetricsEnabled,
@JsonProperty("ignoredKeys") @Nullable Set<String> ignoredKeys,
- @JsonProperty("maxNestedKeyDepth") @Nullable Integer maxNestedKeyDepth) {
+ @JsonProperty("maxNestedKeyDepth") @Nullable Integer maxNestedKeyDepth,
+ @JsonProperty("sparseFieldConfig") @Nullable FieldConfig
sparseFieldConfig) {
super(disabled);
+ _sparseFieldConfig = sparseFieldConfig;
_defaultValueFieldConfig = defaultValueFieldConfig;
_maxDenseKeys = maxDenseKeys != null ? maxDenseKeys :
DEFAULT_MAX_DENSE_KEYS;
_denseKeys = denseKeys;
@@ -250,6 +267,17 @@ public class OpenStructIndexConfig extends IndexConfig {
return _sparseJsonIndex;
}
+ /// Index settings for the shared sparse blob column, or null when the table
asked for none.
+ ///
+ /// The blob is a single-value STRING column holding each row's unshredded
keys as a JSON document, so any index
+ /// that reads JSON out of a STRING column applies to it. `sparseJsonIndex:
true` is the older spelling of
+ /// asking for a JSON index here and still works.
+ @JsonProperty("sparseFieldConfig")
+ @Nullable
+ public FieldConfig getSparseFieldConfig() {
+ return _sparseFieldConfig;
+ }
+
/// When `true`, `OPEN_STRUCT_LAST_SEGMENT_KEY_DOC_COUNT` is emitted for
every key present in the
/// sealed segment — dense, sparse, configured, or discovered. The cost is
that the number of
/// metrics-registry entries follows the ingested key space, and table
deletion can only sweep keys
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
