feat(columnar_map): SPI and data model for COLUMNAR_MAP index #18368
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -25,28 +25,28 @@ | |
|
|
||
| public interface MapDataSource extends DataSource { | ||
|
|
||
| /// Returns the map FieldSpec. | ||
| ComplexFieldSpec.MapFieldSpec getFieldSpec(); | ||
|
|
||
| /// Returns the DataSource for the given map key. For absent keys, returns a [NullDataSource] | ||
| /// that produces the column default value for every document. Callers need not null-check. | ||
| DataSource getKeyDataSource(String key); | ||
|
|
||
| /// Returns whether this segment MAY contain the given key. Implementations are allowed to return | ||
| /// `true` conservatively (i.e., when it is not possible to determine key presence without a | ||
| /// full scan). Callers must handle the case where the key is absent even when this returns | ||
| /// `true` — [#getKeyDataSource(String)] will return a DataSource for an absent key | ||
| /// (forward-index reads return the column default value; null-value bitmap marks all rows as null). | ||
| default boolean containsKey(String key) { | ||
|
Contributor
There was a problem hiding this comment. (optional) This is probably not needed if we make
Contributor
Author
There was a problem hiding this comment. replied above.
|
||
| return getKeyDataSources().containsKey(key); | ||
| } | ||
|
|
||
| /// Returns DataSources for all keys present in this segment. | ||
| Map<String, DataSource> getKeyDataSources(); | ||
|
|
||
| /// Returns the DataSourceMetadata for the given key. | ||
| DataSourceMetadata getKeyDataSourceMetadata(String key); | ||
|
|
||
| /// Returns the ColumnIndexContainer for the given key. | ||
| ColumnIndexContainer getKeyIndexContainer(String key); | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,51 @@ | ||
| /** | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
| package org.apache.pinot.segment.spi.index.creator; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.Map; | ||
| import org.apache.commons.configuration2.PropertiesConfiguration; | ||
| import org.apache.pinot.segment.spi.index.IndexCreator; | ||
|
|
||
|
|
||
| /// Creator for the COLUMNAR_MAP index. Accepts one map per document during segment creation and | ||
| /// decomposes it into per-key columnar storage on `seal()`. | ||
| /// | ||
| /// Implementations are not thread-safe; callers must serialize `add` calls per creator instance. | ||
| /// | ||
| /// The inherited `add(Object, int)` method from `IndexCreator` treats the first argument as the | ||
| /// map and the second as the docId, matching the column-major creator path. | ||
| public interface ColumnarMapIndexCreator extends IndexCreator { | ||
|
|
||
| /// Adds one document's map. Keys are routed to per-key columnar storage; declared-type keys are | ||
| /// coerced to those types, others use the configured default value type. An empty map is valid. | ||
| /// Callers must pass an empty map rather than `null`. | ||
| /// | ||
| /// @param mapValue the document's map (non-null, may be empty) | ||
| /// @param docId document id, must be monotonically non-decreasing across calls | ||
| void add(Map<String, Object> mapValue, int docId) | ||
| throws IOException; | ||
|
|
||
| /// Returns metadata properties for the materialized columns this creator produced during `seal()`. | ||
| /// The framework merges the returned properties into the segment metadata. | ||
| /// Returns an empty map for creators that produce no materialized columns. Call after `seal()`. | ||
| default Map<String, PropertiesConfiguration> getMaterializedColumnMetadata() { | ||
| return Map.of(); | ||
| } | ||
| } |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| /** | ||
| * Licensed to the Apache Software Foundation (ASF) under one | ||
| * or more contributor license agreements. See the NOTICE file | ||
| * distributed with this work for additional information | ||
| * regarding copyright ownership. The ASF licenses this file | ||
| * to you under the Apache License, Version 2.0 (the | ||
| * "License"); you may not use this file except in compliance | ||
| * with the License. You may obtain a copy of the License at | ||
| * | ||
| * http://www.apache.org/licenses/LICENSE-2.0 | ||
| * | ||
| * Unless required by applicable law or agreed to in writing, | ||
| * software distributed under the License is distributed on an | ||
| * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
| * KIND, either express or implied. See the License for the | ||
| * specific language governing permissions and limitations | ||
| * under the License. | ||
| */ | ||
| package org.apache.pinot.segment.spi.index.reader; | ||
|
|
||
| import java.util.Map; | ||
| import java.util.Set; | ||
| import javax.annotation.Nullable; | ||
| import org.apache.pinot.segment.spi.index.IndexReader; | ||
| import org.apache.pinot.spi.data.FieldSpec.DataType; | ||
| import org.roaringbitmap.buffer.ImmutableRoaringBitmap; | ||
|
|
||
|
|
||
| /// Reader for the COLUMNAR_MAP index. Each indexed key is materialized as its own per-key | ||
| /// forward index plus a presence bitmap. | ||
| /// | ||
| /// Implementations must be safe for concurrent reads. Mutable implementations may impose | ||
| /// a single-writer constraint; refer to the concrete implementation's Javadoc for details. | ||
| /// | ||
| /// Per-key `DataSource` construction is the responsibility of the surrounding | ||
| /// `ColumnarMapDataSource` wrappers, not this reader. This interface exposes only | ||
| /// the primitives a wrapper needs (key set, type, presence bitmap, per-doc map view). | ||
| public interface ColumnarMapIndexReader extends IndexReader { | ||
|
|
||
| /// Returns the set of all indexed key names. Never null; empty if no keys are indexed. | ||
| Set<String> getKeys(); | ||
|
|
||
| /// Returns the value DataType for the given key, or null if the key is not indexed. | ||
| @Nullable | ||
| DataType getValueType(String key); | ||
|
|
||
| /// Returns the number of documents that have a non-null value for the given key. | ||
| /// Returns 0 if the key is not indexed. | ||
| int getNumDocsWithKey(String key); | ||
|
|
||
| /// Returns the presence bitmap for the given key (docIds with non-null values). | ||
| /// Returns an empty bitmap if the key is not indexed. The returned bitmap must not be mutated. | ||
| ImmutableRoaringBitmap getPresenceBitmap(String key); | ||
|
|
||
| /// Reconstructs the full map for a single document from per-key data. Only keys with a | ||
| /// non-null value at `docId` appear in the result. Returns an empty map if the document has | ||
| /// no values; behavior for an out-of-range `docId` is implementation-defined. | ||
| Map<String, Object> getMap(int docId); | ||
|
|
||
| /// Returns whether the given key has an inverted index available. False if the key is not indexed. | ||
| default boolean hasInvertedIndex(String key) { | ||
| return false; | ||
| } | ||
|
|
||
| /// Returns sorted distinct values for the key from the inverted index, or null if no | ||
| /// inverted index is available (or the key is not indexed). | ||
| @Nullable | ||
| default String[] getDistinctValuesForKey(String key) { | ||
| return null; | ||
| } | ||
| } |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not introduced in this PR, but let's rename it to:
It is very confusing now because the data source is for value, not key.
Suggest letting it return
@Nullableto represent key not existThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed on the naming — getDataSource(key) is cleaner. I'll do the full getKeyXXX → getXXX rename across the SPI in a separate refactoring PR so it's one clean sweep rather than scattered across the stack. Will raise a new PR for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For @nullable: I'd prefer to keep the current non-null contract backed by NullDataSource. The callers in ProjectionBlock and ItemTransformFunction dereference the result immediately without a null-check — NullDataSource lets them do that safely by returning the column's default value for every doc.
If we switch to @nullable, those two callers need null-guards, and so does any future caller. NullDataSource gives the same semantic (absent key → null/default for all rows) without pushing null-handling into every call site.
Happy to revisit this if you strognly feel we should add @nullable.
Let me know your thoughts.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the API names here - #18437