You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pinot.apache.org by "tarun11Mavani (via GitHub)" <gi...@apache.org> on 2026/07/22 04:33:13 UTC
[PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
tarun11Mavani opened a new pull request, #19040:
URL: https://github.com/apache/pinot/pull/19040
## Summary
This is **PR 3/4** in the OPEN_STRUCT stack — the query engine changes that make `DataType.OPEN_STRUCT` columns queryable. Without it, declaring OPEN_STRUCT in a schema and ingesting data works (PR 2), but all queries crash or fall to per-doc expression scan.
RFC: https://docs.google.com/document/d/14kPmjDTKbO8l0ql4rrN7I5Yki5pqMw6GeGmxxc9grsU/edit?tab=t.0
PR 1 (SPI + data model): #18368 — merged
PR 2a (SPI utilities, index registration): #18710 — merged
PR 2b (storage layer): #18643 — merged
### What this PR adds
**Projection path (`SELECT myStruct['key']`):**
- `ItemTransformFunction` — relaxes `instanceof MapDataSource` to also accept `OpenStructDataSource`. When `getDataSource(key)` returns null (absent key in mutable or fully-materialized segment), wraps in `NullDataSource`.
- `ProjectionBlock` — same pattern for path-based `[column, key]` value set access.
**Filter path (`WHERE myStruct['key'] = 'val'`):**
- `MapFilterOperator` — refactored from 2-way dispatch to 3-way:
1. **PER_KEY_INDEX** (new) — for `OpenStructDataSource` columns. Uses `isMaterialized(key)` / `isFullyMaterialized()` for sub-dispatch:
- Materialized key → rewrite predicate LHS, build `PredicateEvaluator`, delegate to `FilterOperatorUtils.getLeafFilterOperator` (picks inverted/range/sorted/scan automatically)
- IS_NULL/IS_NOT_NULL → read `NullValueVectorReader` bitmap directly → `BitmapBasedFilterOperator`
- Absent key (fully materialized) → IS_NULL → `MatchAllFilterOperator`, else → `EmptyFilterOperator`
- Sparse key (not fully materialized) → fall through to expression scan
2. **JSON_MATCH** (existing, unchanged) — MAP columns with JSON index
3. **EXPRESSION_FILTER** (existing, unchanged) — fallback scan
- Single `_delegate` field replaces the old dual-field pattern. EXPLAIN output reports `delegateTo:per_key_index|json_match|expression_filter`.
**Aggregation path (`SELECT COUNT(DISTINCT myStruct['key'])`):**
- `AggregationPlanNode` — `resolveDataSource()` + `tryResolveKeyedDataSource()` resolve `item(col, 'key')` FUNCTION expressions to per-key DataSources. Enables dictionary/metadata-based non-scan aggregation (COUNT DISTINCT, MIN, MAX) on dense OPEN_STRUCT keys. Sparse keys return null → scan path.
### Supported predicate types on per-key index path
EQ, NOT_EQ, IN, NOT_IN, RANGE, IS_NULL, IS_NOT_NULL. Other predicate types fall through to expression scan.
## Test plan
- [x] `MapFilterOperatorOpenStructTest` — 5 tests:
- EQ on materialized key → PER_KEY_INDEX delegate
- EQ on absent key (fully materialized) → EmptyFilterOperator (EOF)
- IS_NULL on absent key (fully materialized) → MatchAllFilterOperator (numMatchingDocs == numDocs)
- EQ on sparse key (not fully materialized) → EXPRESSION_FILTER fallback
- IS_NOT_NULL with null bitmap → PER_KEY_INDEX, correct doc count (numDocs − nullCount)
- [x] All existing pinot-core tests pass
- [x] spotless/checkstyle clean
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "tarun11Mavani (via GitHub)" <gi...@apache.org>.
tarun11Mavani commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3636796189
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/MapFilterOperator.java:
##########
@@ -67,104 +79,161 @@ public MapFilterOperator(IndexSegment indexSegment, Predicate predicate, QueryCo
_columnName = arguments.get(0).getIdentifier();
_keyName = arguments.get(1).getLiteral().getStringValue();
- JsonIndexReader jsonIndex = null;
- if (canUseJsonIndex(_predicate.getType())) {
- DataSource dataSource = indexSegment.getDataSourceNullable(_columnName);
- if (dataSource != null) {
- jsonIndex = dataSource.getJsonIndex();
- if (jsonIndex == null) {
- // Fallback to Composite JSON Index if standard JSON index is not available
- Optional<IndexType<?, ?, ?>> compositeIndex =
- IndexService.getInstance().getOptional("composite_json_index");
- if (compositeIndex.isPresent()) {
- jsonIndex = (JsonIndexReader) dataSource.getIndex(compositeIndex.get());
- }
- }
- }
+ // Try dispatch paths in priority order
+ DataSource columnDs = indexSegment.getDataSourceNullable(_columnName);
+
+ BaseFilterOperator perKey = tryPerKeyIndex(columnDs, queryContext, numDocs);
+ if (perKey != null) {
+ _delegate = perKey;
+ _delegateType = DelegateType.PER_KEY_INDEX;
+ return;
}
- if (jsonIndex != null) {
- FilterContext filterContext = createFilterContext();
- _jsonMatchOperator = new JsonMatchFilterOperator(jsonIndex, filterContext, numDocs);
- _expressionFilterOperator = null;
- } else {
- _jsonMatchOperator = null;
- _expressionFilterOperator = new ExpressionFilterOperator(indexSegment, queryContext, predicate, numDocs);
+
+ JsonMatchFilterOperator jsonOp = tryJsonIndex(columnDs, numDocs);
+ if (jsonOp != null) {
+ _delegate = jsonOp;
+ _delegateType = DelegateType.JSON_MATCH;
+ return;
}
+
+ _delegate = new ExpressionFilterOperator(indexSegment, queryContext, predicate, numDocs);
+ _delegateType = DelegateType.EXPRESSION_FILTER;
}
- /**
- * Creates a FilterContext based on the original predicate type
- */
- private FilterContext createFilterContext() {
- // Create identifier expression for the JSON column
- ExpressionContext keyLhs = ExpressionContext.forIdentifier(_keyName);
+ @Nullable
+ private BaseFilterOperator tryPerKeyIndex(@Nullable DataSource columnDs, QueryContext queryContext, int numDocs) {
+ if (!(columnDs instanceof OpenStructDataSource)) {
+ return null;
+ }
+ OpenStructDataSource osDs = (OpenStructDataSource) columnDs;
- // Create predicate based on type
- Predicate predicate;
+ if (osDs.isMaterialized(_keyName)) {
+ DataSource keyDs = osDs.getDataSource(_keyName);
+ return buildPerKeyFilterOperator(keyDs, queryContext, numDocs);
+ }
+
+ // Key not materialized
+ if (osDs.isFullyMaterialized()) {
+ // Fully materialized but key absent — definitive answer
+ if (_predicate.getType() == Predicate.Type.IS_NULL) {
+ return new MatchAllFilterOperator(numDocs);
+ }
+ return EmptyFilterOperator.getInstance();
+ }
+
+ // Sparse — can't be sure, fall through to JSON/expression
+ return null;
+ }
+
+ @Nullable
+ private BaseFilterOperator buildPerKeyFilterOperator(DataSource keyDs, QueryContext queryContext, int numDocs) {
switch (_predicate.getType()) {
+ case IS_NULL:
+ case IS_NOT_NULL: {
+ NullValueVectorReader nullReader = keyDs.getNullValueVector();
+ if (nullReader == null) {
Review Comment:
`MutableOpenStructDataSource.getDataSource(key)` now adds a `PresenceBasedNullValueVector` (backed by `MutableKeyColumn.getPresenceBitmap()` complement) to the index map. The bitmap is cloned before iteration to avoid races with the ingestion thread.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/ItemTransformFunction.java:
##########
@@ -58,9 +60,17 @@ public void init(List<TransformFunction> arguments, Map<String, ColumnContext> c
_keyPath = new String[]{column, key};
DataSource dataSource = columnContextMap.get(column).getDataSource();
- Preconditions.checkState(dataSource instanceof MapDataSource, "Column: %s must be a MAP column", column);
- MapDataSource mapDataSource = (MapDataSource) dataSource;
- DataSource valueDataSource = mapDataSource.getDataSource(key);
+ Preconditions.checkState(dataSource instanceof MapDataSource || dataSource instanceof OpenStructDataSource,
+ "Column: %s must be a MAP or OPEN_STRUCT column", column);
+ DataSource valueDataSource;
+ if (dataSource instanceof MapDataSource) {
+ valueDataSource = ((MapDataSource) dataSource).getDataSource(key);
+ } else {
+ valueDataSource = ((OpenStructDataSource) dataSource).getDataSource(key);
+ }
+ if (valueDataSource == null) {
+ valueDataSource = new NullDataSource(key);
Review Comment:
Introduced `OpenStructNullDataSource` which takes the correct FieldSpec and numDocs, provides a typed forward index and a full-segment null bitmap. `NullDataSource` is left untouched for MAP columns.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "tarun11Mavani (via GitHub)" <gi...@apache.org>.
tarun11Mavani commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3682635432
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/ProjectionBlock.java:
##########
@@ -59,15 +63,37 @@ public BlockValSet getBlockValueSet(ExpressionContext expression) {
@Override
public BlockValSet getBlockValueSet(String column) {
- return new ProjectionBlockValSet(_dataBlockCache, column, _dataSourceMap.get(column));
+ DataSource dataSource = _dataSourceMap.get(column);
+ // An OPEN_STRUCT parent is only a handle for per-key resolution — it has no forward index, so DataFetcher does
+ // not register it and it cannot be read as a column. Reject it here rather than letting the missing
+ // ColumnValueReader surface as an NPE.
+ if (dataSource instanceof OpenStructDataSource) {
+ throw new BadQueryRequestException(
+ "OPEN_STRUCT column: " + column + " cannot be selected directly; use " + column + "['key']");
+ }
+ return new ProjectionBlockValSet(_dataBlockCache, column, dataSource);
}
@Override
public BlockValSet getBlockValueSet(String[] paths) {
// TODO: only support one level of path for now, e.g. `map.key`
assert paths.length == 2;
- MapDataSource mapDataSource = (MapDataSource) _dataSourceMap.get(paths[0]);
- DataSource keyDataSource = mapDataSource.getDataSource(paths[1]);
+ DataSource columnDataSource = _dataSourceMap.get(paths[0]);
+ DataSource keyDataSource;
+ if (columnDataSource instanceof MapDataSource) {
+ keyDataSource = ((MapDataSource) columnDataSource).getDataSource(paths[1]);
+ if (keyDataSource == null) {
+ keyDataSource = new NullDataSource(paths[1]);
+ }
+ } else if (columnDataSource instanceof OpenStructDataSource) {
+ OpenStructDataSource osDs = (OpenStructDataSource) columnDataSource;
+ keyDataSource = osDs.getDataSource(paths[1]);
+ if (keyDataSource == null) {
+ keyDataSource = OpenStructNullDataSource.forAbsentKey(osDs, paths[1]);
Review Comment:
this PR scope if Materialized-only.
I am planning to support for sparse key query including json_index for those keys in upcoming PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "raghavyadav01 (via GitHub)" <gi...@apache.org>.
raghavyadav01 commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3659751232
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/ItemTransformFunction.java:
##########
@@ -87,6 +103,11 @@ public Dictionary getDictionary() {
return _dictionary;
}
+ @Override
+ public RoaringBitmap getNullBitmap(ValueBlock valueBlock) {
Review Comment:
This override now applies to MAP `item()` too, not just OPEN_STRUCT. On master `ItemTransformFunction` inherits `BaseTransformFunction.getNullBitmap`, which ORs the argument bitmaps (effectively never-null for a map key). Returning the per-key val-set bitmap silently changes `IS NULL` / null-aware results for existing `map['key']` queries. Was that MAP behavior change intended, and is it covered by a MAP-specific test?
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/MutableOpenStructDataSource.java:
##########
@@ -62,10 +63,13 @@ public ComplexFieldSpec getFieldSpec() {
@Override
@Nullable
public DataSource getDataSource(String key) {
- Map<IndexType, IndexReader> indexes = _index.getIndexes(key);
- if (indexes == null || indexes.isEmpty()) {
+ MutableKeyColumn col = _index.getKeyColumn(key);
+ if (col == null) {
return null;
}
+ Map<IndexType, IndexReader> indexes = new HashMap<>(_index.getIndexes(key));
Review Comment:
This allocates a fresh `HashMap` copy plus a new `PresenceBasedNullValueVector` on every `getDataSource(key)` call, and `getNullBitmap()` then rebuilds a full `[0,numDocs)` bitmap + clone per invocation. If this is hit per-predicate on the query path, is it worth caching the per-key DataSource, or is getDataSource already called once per query?
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java:
##########
@@ -204,6 +221,50 @@ private boolean isFitForNonScanBasedPlan() {
return true;
}
+ @Nullable
+ private DataSource resolveDataSource(ExpressionContext expression) {
+ return resolveDataSource(expression, _indexSegment, _queryContext.getSchema());
+ }
+
+ @Nullable
+ static DataSource resolveDataSource(ExpressionContext expression, IndexSegment segment,
+ @Nullable org.apache.pinot.spi.data.Schema schema) {
+ if (expression.getType() == ExpressionContext.Type.IDENTIFIER) {
+ return segment.getDataSource(expression.getIdentifier(), schema);
+ }
+ if (expression.getType() == ExpressionContext.Type.FUNCTION) {
+ return tryResolveKeyedDataSource(expression, segment, schema);
+ }
+ return null;
+ }
+
+ @Nullable
+ static DataSource tryResolveKeyedDataSource(ExpressionContext expression, IndexSegment segment,
+ @Nullable org.apache.pinot.spi.data.Schema schema) {
+ FunctionContext function = expression.getFunction();
+ if (function == null
+ || !ItemTransformFunction.FUNCTION_NAME.equals(function.getFunctionName())) {
+ return null;
+ }
+ List<ExpressionContext> args = function.getArguments();
+ if (args.size() != 2
+ || args.get(0).getType() != ExpressionContext.Type.IDENTIFIER
+ || args.get(1).getType() != ExpressionContext.Type.LITERAL) {
+ return null;
+ }
+ String columnName = args.get(0).getIdentifier();
+ String key = args.get(1).getLiteral().getStringValue();
+ DataSource columnDs = segment.getDataSource(columnName, schema);
+ if (columnDs instanceof MapDataSource) {
+ return ((MapDataSource) columnDs).getDataSource(key);
Review Comment:
This now makes `item(mapCol, 'k')` eligible for the non-scan aggregation path (previously any FUNCTION arg forced a scan). Two concerns for a key present in only some docs: (1) `BaseMapDataSource.getDataSource` returns a `NullDataSource` (INT, numDocs=0) for an absent key rather than null, so a non-scan COUNT/MIN/MAX could run over 0 docs; (2) the per-key MAP source may carry no null vector, so `hasNullValues` returns false and non-scan agg would ignore absent-doc nulls. Can you confirm the per-key MAP source models absent-doc nulls / full segment numDocs here?
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/PresenceBasedNullValueVector.java:
##########
@@ -0,0 +1,56 @@
+/**
+ * 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.local.segment.index.openstruct;
+
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
+
+
+/// {@link NullValueVectorReader} backed by an OPEN_STRUCT key's presence bitmap.
+///
+/// A document is null for a key when the key was never set on that document, i.e. when the
+/// document is absent from the presence bitmap. The null bitmap is computed on demand (not cached)
+/// because the presence bitmap is mutable during real-time consumption.
+public class PresenceBasedNullValueVector implements NullValueVectorReader {
+ private final ImmutableRoaringBitmap _presenceBitmap;
+ private final int _numDocs;
+
+ public PresenceBasedNullValueVector(ImmutableRoaringBitmap presenceBitmap, int numDocs) {
+ _presenceBitmap = presenceBitmap;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public boolean isNull(int docId) {
+ return !_presenceBitmap.contains(docId);
Review Comment:
`getNullBitmap()` below clones the presence bitmap before iterating to avoid racing the ingestion thread, but `isNull()` reads the live mutable bitmap directly via `contains()`. On a consuming segment, isn't this the same concurrent-mutation hazard the clone was added to guard against? Should isNull go through the same protection (or is contains() safe against a concurrent `add`)?
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/ImmutableOpenStructDataSource.java:
##########
@@ -84,8 +84,7 @@ public ComplexFieldSpec getFieldSpec() {
@Override
@Nullable
public DataSource getDataSource(String key) {
- DataSource ds = _perKeyDataSources.get(key);
- return ds != null ? ds : _sparseDataSource;
+ return _perKeyDataSources.get(key);
Review Comment:
This flips `getDataSource(key)` from returning the `$__sparse__` fallback to returning `null` for unmaterialized keys — a semantic change to a method other call sites may have assumed non-null. Have all existing callers been audited to handle null (vs. the old sparse fallback)?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "tarun11Mavani (via GitHub)" <gi...@apache.org>.
tarun11Mavani commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3664217154
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/MutableOpenStructDataSource.java:
##########
@@ -62,10 +63,13 @@ public ComplexFieldSpec getFieldSpec() {
@Override
@Nullable
public DataSource getDataSource(String key) {
- Map<IndexType, IndexReader> indexes = _index.getIndexes(key);
- if (indexes == null || indexes.isEmpty()) {
+ MutableKeyColumn col = _index.getKeyColumn(key);
+ if (col == null) {
return null;
}
+ Map<IndexType, IndexReader> indexes = new HashMap<>(_index.getIndexes(key));
Review Comment:
`getDataSource(key)` is called at operator setup, not per row. The three call sites — `ProjectionBlock.getBlockValueSet`, `ItemTransformFunction.init`, `MapFilterOperator.tryPerKeyIndex` — each resolve the key once per segment per query and hold the result. So the `HashMap` copy (shallow, ~2–3 entries) and the `PresenceBasedNullValueVector` (two fields) are setup-time costs, and `getNullBitmap()` fires at most once per key per segment per query.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "raghavyadav01 (via GitHub)" <gi...@apache.org>.
raghavyadav01 commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3706812016
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java:
##########
@@ -187,6 +191,15 @@ private boolean hasNullValues(AggregationFunction[] aggregationFunctions) {
}
break;
case FUNCTION:
+ DataSource resolvedDs = resolveDataSource(argument);
+ if (resolvedDs == null) {
+ return true;
+ }
+ NullValueVectorReader resolvedNullVector = resolvedDs.getNullValueVector();
+ if (resolvedNullVector != null && !resolvedNullVector.getNullBitmap().isEmpty()) {
Review Comment:
This narrows what used to be a blanket "any FUNCTION arg might be null" assumption. Now `item()` on a materialized key whose per-key null vector is empty (or null) falls through to `break`, so the aggregation is treated as null-free and takes the non-scan path. That's only correct if the per-key null vector is authoritative for absent docs — which is exactly the mutable-vs-sealed parity question raised earlier in the review.
Not blocking, but could we add a null-handling-**on** test that pins this: `COUNT`/`MIN`/`MAX` over a partially-present key, run on both a consuming and a sealed segment? The existing parity test only covers null handling off.
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/realtime/impl/invertedindex/RealtimeInvertedIndex.java:
##########
@@ -57,6 +57,19 @@ public void add(int dictId, int docId) {
}
}
+ /// Pre-creates an empty bitmap for the next dictionary id. Used by callers that reserve a
+ /// dictionary id upfront (e.g. OPEN_STRUCT key columns reserve dictId 0 for the default null
+ /// value) so that subsequent contiguous [#add] calls stay aligned with dictionary ids.
+ public void reserveNextDictId() {
Review Comment:
`reserveNextDictId()` appends unconditionally, unlike `add(dictId, docId)` which guards on `_bitmaps.size() == dictId`. It's safe today because `MutableKeyColumn` calls it exactly once on an empty index, but the method is public and generically named — a second call, or an interleave with `add()`, would silently misalign every dictId→docId bitmap after it. Worth a cheap guard, e.g. `Preconditions.checkState(_bitmaps.isEmpty())`, to make the "only on an empty index" contract enforced rather than assumed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on PR #19040:
URL: https://github.com/apache/pinot/pull/19040#issuecomment-5174630792
Documentation follow-up: https://github.com/pinot-contrib/pinot-docs/pull/961
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "tarun11Mavani (via GitHub)" <gi...@apache.org>.
tarun11Mavani commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3664081203
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/ItemTransformFunction.java:
##########
@@ -87,6 +103,11 @@ public Dictionary getDictionary() {
return _dictionary;
}
+ @Override
+ public RoaringBitmap getNullBitmap(ValueBlock valueBlock) {
Review Comment:
You're right that the base behavior is correct for MAP: there's no per-key null bitmap, so ORing the argument bitmaps yields the whole-map bitmap, a conservative over-estimate.
Removing the override entirely doesn't work either: for OPEN_STRUCT the parent DataSource is built with an empty index container (`ImmutableOpenStructDataSource:84`, `MutableOpenStructDataSource:51`), so the base impl would report "no nulls" there too. And `TransformBlockValSet.getNullBitmap()` delegates straight to the transform function (`:60`), so this is the only place per-key nulls can come from.
Scoped by column type instead: OPEN_STRUCT reads the per-key bitmap, MAP delegates to `super`. Added MAP tests including the absent-key case — both fail against the previous unconditional override.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "tarun11Mavani (via GitHub)" <gi...@apache.org>.
tarun11Mavani commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3682635432
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/ProjectionBlock.java:
##########
@@ -59,15 +63,37 @@ public BlockValSet getBlockValueSet(ExpressionContext expression) {
@Override
public BlockValSet getBlockValueSet(String column) {
- return new ProjectionBlockValSet(_dataBlockCache, column, _dataSourceMap.get(column));
+ DataSource dataSource = _dataSourceMap.get(column);
+ // An OPEN_STRUCT parent is only a handle for per-key resolution — it has no forward index, so DataFetcher does
+ // not register it and it cannot be read as a column. Reject it here rather than letting the missing
+ // ColumnValueReader surface as an NPE.
+ if (dataSource instanceof OpenStructDataSource) {
+ throw new BadQueryRequestException(
+ "OPEN_STRUCT column: " + column + " cannot be selected directly; use " + column + "['key']");
+ }
+ return new ProjectionBlockValSet(_dataBlockCache, column, dataSource);
}
@Override
public BlockValSet getBlockValueSet(String[] paths) {
// TODO: only support one level of path for now, e.g. `map.key`
assert paths.length == 2;
- MapDataSource mapDataSource = (MapDataSource) _dataSourceMap.get(paths[0]);
- DataSource keyDataSource = mapDataSource.getDataSource(paths[1]);
+ DataSource columnDataSource = _dataSourceMap.get(paths[0]);
+ DataSource keyDataSource;
+ if (columnDataSource instanceof MapDataSource) {
+ keyDataSource = ((MapDataSource) columnDataSource).getDataSource(paths[1]);
+ if (keyDataSource == null) {
+ keyDataSource = new NullDataSource(paths[1]);
+ }
+ } else if (columnDataSource instanceof OpenStructDataSource) {
+ OpenStructDataSource osDs = (OpenStructDataSource) columnDataSource;
+ keyDataSource = osDs.getDataSource(paths[1]);
+ if (keyDataSource == null) {
+ keyDataSource = OpenStructNullDataSource.forAbsentKey(osDs, paths[1]);
Review Comment:
this PR scope if Materialized-only.
I am planning to add support for sparse key query including json_index for those keys in upcoming PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "tarun11Mavani (via GitHub)" <gi...@apache.org>.
tarun11Mavani commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3664165753
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/PresenceBasedNullValueVector.java:
##########
@@ -0,0 +1,56 @@
+/**
+ * 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.local.segment.index.openstruct;
+
+import org.apache.pinot.segment.spi.index.reader.NullValueVectorReader;
+import org.roaringbitmap.buffer.ImmutableRoaringBitmap;
+import org.roaringbitmap.buffer.MutableRoaringBitmap;
+
+
+/// {@link NullValueVectorReader} backed by an OPEN_STRUCT key's presence bitmap.
+///
+/// A document is null for a key when the key was never set on that document, i.e. when the
+/// document is absent from the presence bitmap. The null bitmap is computed on demand (not cached)
+/// because the presence bitmap is mutable during real-time consumption.
+public class PresenceBasedNullValueVector implements NullValueVectorReader {
+ private final ImmutableRoaringBitmap _presenceBitmap;
+ private final int _numDocs;
+
+ public PresenceBasedNullValueVector(ImmutableRoaringBitmap presenceBitmap, int numDocs) {
+ _presenceBitmap = presenceBitmap;
+ _numDocs = numDocs;
+ }
+
+ @Override
+ public boolean isNull(int docId) {
+ return !_presenceBitmap.contains(docId);
Review Comment:
thanks for flagging this. Replaced ImmutableRoaringBitmap with ThreadSafeMutableRoaringBitmap.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "raghavyadav01 (via GitHub)" <gi...@apache.org>.
raghavyadav01 merged PR #19040:
URL: https://github.com/apache/pinot/pull/19040
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "raghavyadav01 (via GitHub)" <gi...@apache.org>.
raghavyadav01 commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3659796502
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/MutableOpenStructDataSource.java:
##########
@@ -62,10 +63,13 @@ public ComplexFieldSpec getFieldSpec() {
@Override
@Nullable
public DataSource getDataSource(String key) {
- Map<IndexType, IndexReader> indexes = _index.getIndexes(key);
- if (indexes == null || indexes.isEmpty()) {
+ MutableKeyColumn col = _index.getKeyColumn(key);
+ if (col == null) {
return null;
}
+ Map<IndexType, IndexReader> indexes = new HashMap<>(_index.getIndexes(key));
+ indexes.put(StandardIndexes.nullValueVector(),
+ new PresenceBasedNullValueVector(col.getPresenceBitmap(), _numDocs));
Review Comment:
The mutable path synthesizes a `PresenceBasedNullValueVector` here so per-key null info is exposed. On the sealed side, `ImmutableSegmentImpl` builds each materialized key as a plain `ImmutableDataSource`, so `getNullValueVector()` returns only what was written at seal time — there's no equivalent synthesis. For a **dense key that's materialized but present in only some docs**, does the segment build write a null vector for the absent docs? If not, on immutable segments `buildPerKeyFilterOperator`'s `nullReader == null` branch would treat those docs as non-null (IS_NULL → Empty, IS_NOT_NULL → MatchAll), and `hasNullValues` would return false and let a non-scan aggregation ignore them — both wrong. Worth a test with a partially-present dense key that survives seal, run on the immutable segment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #19040:
URL: https://github.com/apache/pinot/pull/19040#issuecomment-5043930363
## [Codecov](https://app.codecov.io/gh/apache/pinot/pull/19040?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:x: Patch coverage is `53.22581%` with `87 lines` in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 65.44%. Comparing base ([`385e38a`](https://app.codecov.io/gh/apache/pinot/commit/385e38a9c0fe7d09b9e6c549935c61e8e4c92c0c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`02bef8b`](https://app.codecov.io/gh/apache/pinot/commit/02bef8bb8d149b27633147fcf34f6a4f377d075c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 1 commits behind head on master.
| [Files with missing lines](https://app.codecov.io/gh/apache/pinot/pull/19040?dropdown=coverage&src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Patch % | Lines |
|---|---|---|
| [.../pinot/core/operator/filter/MapFilterOperator.java](https://app.codecov.io/gh/apache/pinot/pull/19040?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Ffilter%2FMapFilterOperator.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9maWx0ZXIvTWFwRmlsdGVyT3BlcmF0b3IuamF2YQ==) | 47.56% | [32 Missing and 11 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19040?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ndex/openstruct/ImmutableOpenStructDataSource.java](https://app.codecov.io/gh/apache/pinot/pull/19040?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Fsegment%2Findex%2Fopenstruct%2FImmutableOpenStructDataSource.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9zZWdtZW50L2luZGV4L29wZW5zdHJ1Y3QvSW1tdXRhYmxlT3BlblN0cnVjdERhdGFTb3VyY2UuamF2YQ==) | 59.09% | [12 Missing and 6 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19040?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...he/pinot/core/operator/blocks/ProjectionBlock.java](https://app.codecov.io/gh/apache/pinot/pull/19040?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Fblocks%2FProjectionBlock.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci9ibG9ja3MvUHJvamVjdGlvbkJsb2NrLmphdmE=) | 0.00% | [8 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19040?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...ator/transform/function/ItemTransformFunction.java](https://app.codecov.io/gh/apache/pinot/pull/19040?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Foperator%2Ftransform%2Ffunction%2FItemTransformFunction.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9vcGVyYXRvci90cmFuc2Zvcm0vZnVuY3Rpb24vSXRlbVRyYW5zZm9ybUZ1bmN0aW9uLmphdmE=) | 0.00% | [6 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19040?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...rg/apache/pinot/core/plan/AggregationPlanNode.java](https://app.codecov.io/gh/apache/pinot/pull/19040?src=pr&el=tree&filepath=pinot-core%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fcore%2Fplan%2FAggregationPlanNode.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3QtY29yZS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3QvY29yZS9wbGFuL0FnZ3JlZ2F0aW9uUGxhbk5vZGUuamF2YQ==) | 83.33% | [3 Missing and 3 partials :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19040?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...l/indexsegment/immutable/ImmutableSegmentImpl.java](https://app.codecov.io/gh/apache/pinot/pull/19040?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Findexsegment%2Fimmutable%2FImmutableSegmentImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvaW1tdXRhYmxlL0ltbXV0YWJsZVNlZ21lbnRJbXBsLmphdmE=) | 0.00% | [3 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19040?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...local/indexsegment/mutable/MutableSegmentImpl.java](https://app.codecov.io/gh/apache/pinot/pull/19040?src=pr&el=tree&filepath=pinot-segment-local%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Flocal%2Findexsegment%2Fmutable%2FMutableSegmentImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1sb2NhbC9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvcGlub3Qvc2VnbWVudC9sb2NhbC9pbmRleHNlZ21lbnQvbXV0YWJsZS9NdXRhYmxlU2VnbWVudEltcGwuamF2YQ==) | 0.00% | [2 Missing :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19040?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
| [...segment/spi/index/metadata/ColumnMetadataImpl.java](https://app.codecov.io/gh/apache/pinot/pull/19040?src=pr&el=tree&filepath=pinot-segment-spi%2Fsrc%2Fmain%2Fjava%2Forg%2Fapache%2Fpinot%2Fsegment%2Fspi%2Findex%2Fmetadata%2FColumnMetadataImpl.java&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#diff-cGlub3Qtc2VnbWVudC1zcGkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3Bpbm90L3NlZ21lbnQvc3BpL2luZGV4L21ldGFkYXRhL0NvbHVtbk1ldGFkYXRhSW1wbC5qYXZh) | 80.00% | [0 Missing and 1 partial :warning: ](https://app.codecov.io/gh/apache/pinot/pull/19040?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) |
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #19040 +/- ##
============================================
- Coverage 65.45% 65.44% -0.02%
Complexity 1421 1421
============================================
Files 3428 3428
Lines 216598 216717 +119
Branches 34311 34351 +40
============================================
+ Hits 141778 141832 +54
- Misses 63405 63440 +35
- Partials 11415 11445 +30
```
| [Flag](https://app.codecov.io/gh/apache/pinot/pull/19040/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | Coverage Δ | |
|---|---|---|
| [custom-integration1](https://app.codecov.io/gh/apache/pinot/pull/19040/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
| [integration](https://app.codecov.io/gh/apache/pinot/pull/19040/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
| [integration1](https://app.codecov.io/gh/apache/pinot/pull/19040/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `100.00% <ø> (ø)` | |
| [integration2](https://app.codecov.io/gh/apache/pinot/pull/19040/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `0.00% <ø> (ø)` | |
| [java-25](https://app.codecov.io/gh/apache/pinot/pull/19040/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `65.44% <53.22%> (-0.02%)` | :arrow_down: |
| [temurin](https://app.codecov.io/gh/apache/pinot/pull/19040/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `65.44% <53.22%> (-0.02%)` | :arrow_down: |
| [unittests](https://app.codecov.io/gh/apache/pinot/pull/19040/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `65.44% <53.22%> (-0.02%)` | :arrow_down: |
| [unittests1](https://app.codecov.io/gh/apache/pinot/pull/19040/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `56.88% <39.24%> (-0.03%)` | :arrow_down: |
| [unittests2](https://app.codecov.io/gh/apache/pinot/pull/19040/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) | `37.72% <13.97%> (-0.01%)` | :arrow_down: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache#carryforward-flags-in-the-pull-request-comment) to find out more.
</details>
[:umbrella: View full report in Codecov by Harness](https://app.codecov.io/gh/apache/pinot/pull/19040?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :package: [JS Bundle Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "tarun11Mavani (via GitHub)" <gi...@apache.org>.
tarun11Mavani commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3664217154
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/MutableOpenStructDataSource.java:
##########
@@ -62,10 +63,13 @@ public ComplexFieldSpec getFieldSpec() {
@Override
@Nullable
public DataSource getDataSource(String key) {
- Map<IndexType, IndexReader> indexes = _index.getIndexes(key);
- if (indexes == null || indexes.isEmpty()) {
+ MutableKeyColumn col = _index.getKeyColumn(key);
+ if (col == null) {
return null;
}
+ Map<IndexType, IndexReader> indexes = new HashMap<>(_index.getIndexes(key));
Review Comment:
make sense. Adding `_perKeyDataSources` cache here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "tarun11Mavani (via GitHub)" <gi...@apache.org>.
tarun11Mavani commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3682625364
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/MutableOpenStructDataSource.java:
##########
@@ -62,13 +69,19 @@ public ComplexFieldSpec getFieldSpec() {
@Override
@Nullable
public DataSource getDataSource(String key) {
- Map<IndexType, IndexReader> indexes = _index.getIndexes(key);
- if (indexes == null || indexes.isEmpty()) {
+ // Live lookup, outside the memo: a key not yet observed may still be created by the ingestion thread.
+ MutableKeyColumn col = _index.getKeyColumn(key);
+ if (col == null) {
return null;
}
- ColumnMetadata metadata = _index.getColumnMetadata(key);
- return new ImmutableDataSource(metadata,
- new ColumnIndexContainer.FromMap.Builder().withAll(indexes).build());
+ return _perKeyDataSourceCache.computeIfAbsent(key, k -> {
+ Map<IndexType, IndexReader> indexes = new HashMap<>(_index.getIndexes(k));
Review Comment:
Confirmed both failure modes: docIds past the last written chunk hit `_readers.get(bufferId)` OOB, and in-range holes deserialized as dictId 0 (first inserted value). Fixed by making dictId 0 *be* the answer: the dictionary now reserves it for the key's default null value at construction, so zero-initialized chunks read absent docs as the default (same value the sealed build folds in), and a last-written-docId watermark guards reads past the allocated tail.
Added `OpenStructConsumingSealedParityTest` covering a key absent from middle + tail rows, consuming vs sealed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "tarun11Mavani (via GitHub)" <gi...@apache.org>.
tarun11Mavani commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3664097757
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java:
##########
@@ -204,6 +221,50 @@ private boolean isFitForNonScanBasedPlan() {
return true;
}
+ @Nullable
+ private DataSource resolveDataSource(ExpressionContext expression) {
+ return resolveDataSource(expression, _indexSegment, _queryContext.getSchema());
+ }
+
+ @Nullable
+ static DataSource resolveDataSource(ExpressionContext expression, IndexSegment segment,
+ @Nullable org.apache.pinot.spi.data.Schema schema) {
+ if (expression.getType() == ExpressionContext.Type.IDENTIFIER) {
+ return segment.getDataSource(expression.getIdentifier(), schema);
+ }
+ if (expression.getType() == ExpressionContext.Type.FUNCTION) {
+ return tryResolveKeyedDataSource(expression, segment, schema);
+ }
+ return null;
+ }
+
+ @Nullable
+ static DataSource tryResolveKeyedDataSource(ExpressionContext expression, IndexSegment segment,
+ @Nullable org.apache.pinot.spi.data.Schema schema) {
+ FunctionContext function = expression.getFunction();
+ if (function == null
+ || !ItemTransformFunction.FUNCTION_NAME.equals(function.getFunctionName())) {
+ return null;
+ }
+ List<ExpressionContext> args = function.getArguments();
+ if (args.size() != 2
+ || args.get(0).getType() != ExpressionContext.Type.IDENTIFIER
+ || args.get(1).getType() != ExpressionContext.Type.LITERAL) {
+ return null;
+ }
+ String columnName = args.get(0).getIdentifier();
+ String key = args.get(1).getLiteral().getStringValue();
+ DataSource columnDs = segment.getDataSource(columnName, schema);
+ if (columnDs instanceof MapDataSource) {
+ return ((MapDataSource) columnDs).getDataSource(key);
Review Comment:
Checked both concerns. Neither can bite today, but I've added the guard.
(1) **COUNT** never reaches the per-key DataSource — `isFitForNonScanBasedPlan` skips COUNT before resolving it, and the operator counts from segment metadata. So `NullDataSource`'s `numDocs=0` is never read.
(2) **MIN/MAX** is the real hole, and it's min/max rather than the null vector: `NullDataSourceMetadata` returns `Integer.MIN_VALUE` for both (`NullDataSource:181-189`), so an absent MAP key would pass the metadata check and `MIN(mapCol['absentKey'])` would return `-2147483648` instead of NULL.
That path is unreachable on master, though. `BaseMapDataSource` only returns a `NullDataSource` when `getMapIndexReader().getIndexes(key)` is null, and the only `MapIndexReader` implementation returns a non-null map unconditionally. Absent keys land on `ImmutableDataSource` instead, whose min/max are null, so they fail the metadata check and fall back to a scan.
Added the guard regardless — `return keyDs instanceof NullDataSource ? null : keyDs`, mirroring the OPEN_STRUCT branch. `ItemTransformFunction` already null-checks the same call, so the nullable contract is clearly intended; this just makes the aggregation path honour it too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "tarun11Mavani (via GitHub)" <gi...@apache.org>.
tarun11Mavani commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3664362458
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/MapFilterOperator.java:
##########
@@ -67,104 +79,161 @@ public MapFilterOperator(IndexSegment indexSegment, Predicate predicate, QueryCo
_columnName = arguments.get(0).getIdentifier();
_keyName = arguments.get(1).getLiteral().getStringValue();
- JsonIndexReader jsonIndex = null;
- if (canUseJsonIndex(_predicate.getType())) {
- DataSource dataSource = indexSegment.getDataSourceNullable(_columnName);
- if (dataSource != null) {
- jsonIndex = dataSource.getJsonIndex();
- if (jsonIndex == null) {
- // Fallback to Composite JSON Index if standard JSON index is not available
- Optional<IndexType<?, ?, ?>> compositeIndex =
- IndexService.getInstance().getOptional("composite_json_index");
- if (compositeIndex.isPresent()) {
- jsonIndex = (JsonIndexReader) dataSource.getIndex(compositeIndex.get());
- }
- }
- }
+ // Try dispatch paths in priority order
+ DataSource columnDs = indexSegment.getDataSourceNullable(_columnName);
+
+ BaseFilterOperator perKey = tryPerKeyIndex(columnDs, queryContext, numDocs);
+ if (perKey != null) {
+ _delegate = perKey;
+ _delegateType = DelegateType.PER_KEY_INDEX;
+ return;
}
- if (jsonIndex != null) {
- FilterContext filterContext = createFilterContext();
- _jsonMatchOperator = new JsonMatchFilterOperator(jsonIndex, filterContext, numDocs);
- _expressionFilterOperator = null;
- } else {
- _jsonMatchOperator = null;
- _expressionFilterOperator = new ExpressionFilterOperator(indexSegment, queryContext, predicate, numDocs);
+
+ JsonMatchFilterOperator jsonOp = tryJsonIndex(columnDs, numDocs);
+ if (jsonOp != null) {
+ _delegate = jsonOp;
+ _delegateType = DelegateType.JSON_MATCH;
+ return;
}
+
+ _delegate = new ExpressionFilterOperator(indexSegment, queryContext, predicate, numDocs);
+ _delegateType = DelegateType.EXPRESSION_FILTER;
}
- /**
- * Creates a FilterContext based on the original predicate type
- */
- private FilterContext createFilterContext() {
- // Create identifier expression for the JSON column
- ExpressionContext keyLhs = ExpressionContext.forIdentifier(_keyName);
+ @Nullable
+ private BaseFilterOperator tryPerKeyIndex(@Nullable DataSource columnDs, QueryContext queryContext, int numDocs) {
+ if (!(columnDs instanceof OpenStructDataSource)) {
+ return null;
+ }
+ OpenStructDataSource osDs = (OpenStructDataSource) columnDs;
- // Create predicate based on type
- Predicate predicate;
+ if (osDs.isMaterialized(_keyName)) {
+ DataSource keyDs = osDs.getDataSource(_keyName);
+ return buildPerKeyFilterOperator(keyDs, queryContext, numDocs);
+ }
+
+ // Key not materialized
+ if (osDs.isFullyMaterialized()) {
+ // Fully materialized but key absent — definitive answer
+ if (_predicate.getType() == Predicate.Type.IS_NULL) {
+ return new MatchAllFilterOperator(numDocs);
+ }
+ return EmptyFilterOperator.getInstance();
Review Comment:
You're right — with null handling off an absent key reads as its type default (`"null"` for STRING),
so `NOT_EQ` / `NOT_IN` should match every doc. Fixed, routed through `forAbsentKey` as you suggested.
One deviation: delegating to `buildPerKeyFilterOperator` lands on a scan of all N docs for a constant
answer and loses `canOptimizeCount()`, so I apply the evaluator once and fold to `MatchAll`/`Empty`.
Same result, O(1).
Three other behavior changes fall out of this, all consistent with "absent key reads as its default":
`RANGE` is evaluated rather than always `Empty` (lexicographic for undeclared keys, which fall back
to STRING), `REGEXP_LIKE` falls through to the expression path instead of `Empty`, and a type
mismatch like `item(m,'absentLongKey') = 'abc'` now throws instead of returning 0 rows — same as a
materialized LONG key.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "raghavyadav01 (via GitHub)" <gi...@apache.org>.
raghavyadav01 commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3659774630
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/MapFilterOperator.java:
##########
@@ -67,104 +79,161 @@ public MapFilterOperator(IndexSegment indexSegment, Predicate predicate, QueryCo
_columnName = arguments.get(0).getIdentifier();
_keyName = arguments.get(1).getLiteral().getStringValue();
- JsonIndexReader jsonIndex = null;
- if (canUseJsonIndex(_predicate.getType())) {
- DataSource dataSource = indexSegment.getDataSourceNullable(_columnName);
- if (dataSource != null) {
- jsonIndex = dataSource.getJsonIndex();
- if (jsonIndex == null) {
- // Fallback to Composite JSON Index if standard JSON index is not available
- Optional<IndexType<?, ?, ?>> compositeIndex =
- IndexService.getInstance().getOptional("composite_json_index");
- if (compositeIndex.isPresent()) {
- jsonIndex = (JsonIndexReader) dataSource.getIndex(compositeIndex.get());
- }
- }
- }
+ // Try dispatch paths in priority order
+ DataSource columnDs = indexSegment.getDataSourceNullable(_columnName);
+
+ BaseFilterOperator perKey = tryPerKeyIndex(columnDs, queryContext, numDocs);
+ if (perKey != null) {
+ _delegate = perKey;
+ _delegateType = DelegateType.PER_KEY_INDEX;
+ return;
}
- if (jsonIndex != null) {
- FilterContext filterContext = createFilterContext();
- _jsonMatchOperator = new JsonMatchFilterOperator(jsonIndex, filterContext, numDocs);
- _expressionFilterOperator = null;
- } else {
- _jsonMatchOperator = null;
- _expressionFilterOperator = new ExpressionFilterOperator(indexSegment, queryContext, predicate, numDocs);
+
+ JsonMatchFilterOperator jsonOp = tryJsonIndex(columnDs, numDocs);
+ if (jsonOp != null) {
+ _delegate = jsonOp;
+ _delegateType = DelegateType.JSON_MATCH;
+ return;
}
+
+ _delegate = new ExpressionFilterOperator(indexSegment, queryContext, predicate, numDocs);
+ _delegateType = DelegateType.EXPRESSION_FILTER;
}
- /**
- * Creates a FilterContext based on the original predicate type
- */
- private FilterContext createFilterContext() {
- // Create identifier expression for the JSON column
- ExpressionContext keyLhs = ExpressionContext.forIdentifier(_keyName);
+ @Nullable
+ private BaseFilterOperator tryPerKeyIndex(@Nullable DataSource columnDs, QueryContext queryContext, int numDocs) {
+ if (!(columnDs instanceof OpenStructDataSource)) {
+ return null;
+ }
+ OpenStructDataSource osDs = (OpenStructDataSource) columnDs;
- // Create predicate based on type
- Predicate predicate;
+ if (osDs.isMaterialized(_keyName)) {
+ DataSource keyDs = osDs.getDataSource(_keyName);
+ return buildPerKeyFilterOperator(keyDs, queryContext, numDocs);
+ }
+
+ // Key not materialized
+ if (osDs.isFullyMaterialized()) {
+ // Fully materialized but key absent — definitive answer
+ if (_predicate.getType() == Predicate.Type.IS_NULL) {
+ return new MatchAllFilterOperator(numDocs);
+ }
+ return EmptyFilterOperator.getInstance();
Review Comment:
For a fully-materialized-but-absent key this hardcodes `IS_NULL → MatchAll` and everything else → `Empty`. With null handling disabled, `NOT_EQ` / `NOT_IN` against an all-default column should match *all* docs, so `Empty` looks wrong for those. Rather than special-casing here, could this delegate to `buildPerKeyFilterOperator(OpenStructNullDataSource.forAbsentKey(osDs, _keyName), ...)`? That's the typed all-null source (with a full null vector) this PR already builds for the projection path, and it'd keep EQ/NOT_EQ/IS_NULL semantics consistent across filter and projection.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "xiangfu0 (via GitHub)" <gi...@apache.org>.
xiangfu0 commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3632306646
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/ItemTransformFunction.java:
##########
@@ -58,9 +60,17 @@ public void init(List<TransformFunction> arguments, Map<String, ColumnContext> c
_keyPath = new String[]{column, key};
DataSource dataSource = columnContextMap.get(column).getDataSource();
- Preconditions.checkState(dataSource instanceof MapDataSource, "Column: %s must be a MAP column", column);
- MapDataSource mapDataSource = (MapDataSource) dataSource;
- DataSource valueDataSource = mapDataSource.getDataSource(key);
+ Preconditions.checkState(dataSource instanceof MapDataSource || dataSource instanceof OpenStructDataSource,
+ "Column: %s must be a MAP or OPEN_STRUCT column", column);
+ DataSource valueDataSource;
+ if (dataSource instanceof MapDataSource) {
+ valueDataSource = ((MapDataSource) dataSource).getDataSource(key);
+ } else {
+ valueDataSource = ((OpenStructDataSource) dataSource).getDataSource(key);
Review Comment:
Sparse OPEN_STRUCT keys do not have a per-key DataSource here. `ImmutableOpenStructDataSource#getDataSource()` returns the shared `$__sparse__` JSON column for every unmaterialized key, so the expression fallback compares or projects the entire JSON blob instead of the requested value. For example, `metrics['region'] = 'us'` compares `{"region":"us",...}` with `us` and silently returns no rows. Only use the direct DataSource path for `isMaterialized(key)` and add a key-aware sparse extractor or JSON delegate.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/filter/MapFilterOperator.java:
##########
@@ -67,104 +79,161 @@ public MapFilterOperator(IndexSegment indexSegment, Predicate predicate, QueryCo
_columnName = arguments.get(0).getIdentifier();
_keyName = arguments.get(1).getLiteral().getStringValue();
- JsonIndexReader jsonIndex = null;
- if (canUseJsonIndex(_predicate.getType())) {
- DataSource dataSource = indexSegment.getDataSourceNullable(_columnName);
- if (dataSource != null) {
- jsonIndex = dataSource.getJsonIndex();
- if (jsonIndex == null) {
- // Fallback to Composite JSON Index if standard JSON index is not available
- Optional<IndexType<?, ?, ?>> compositeIndex =
- IndexService.getInstance().getOptional("composite_json_index");
- if (compositeIndex.isPresent()) {
- jsonIndex = (JsonIndexReader) dataSource.getIndex(compositeIndex.get());
- }
- }
- }
+ // Try dispatch paths in priority order
+ DataSource columnDs = indexSegment.getDataSourceNullable(_columnName);
+
+ BaseFilterOperator perKey = tryPerKeyIndex(columnDs, queryContext, numDocs);
+ if (perKey != null) {
+ _delegate = perKey;
+ _delegateType = DelegateType.PER_KEY_INDEX;
+ return;
}
- if (jsonIndex != null) {
- FilterContext filterContext = createFilterContext();
- _jsonMatchOperator = new JsonMatchFilterOperator(jsonIndex, filterContext, numDocs);
- _expressionFilterOperator = null;
- } else {
- _jsonMatchOperator = null;
- _expressionFilterOperator = new ExpressionFilterOperator(indexSegment, queryContext, predicate, numDocs);
+
+ JsonMatchFilterOperator jsonOp = tryJsonIndex(columnDs, numDocs);
+ if (jsonOp != null) {
+ _delegate = jsonOp;
+ _delegateType = DelegateType.JSON_MATCH;
+ return;
}
+
+ _delegate = new ExpressionFilterOperator(indexSegment, queryContext, predicate, numDocs);
+ _delegateType = DelegateType.EXPRESSION_FILTER;
}
- /**
- * Creates a FilterContext based on the original predicate type
- */
- private FilterContext createFilterContext() {
- // Create identifier expression for the JSON column
- ExpressionContext keyLhs = ExpressionContext.forIdentifier(_keyName);
+ @Nullable
+ private BaseFilterOperator tryPerKeyIndex(@Nullable DataSource columnDs, QueryContext queryContext, int numDocs) {
+ if (!(columnDs instanceof OpenStructDataSource)) {
+ return null;
+ }
+ OpenStructDataSource osDs = (OpenStructDataSource) columnDs;
- // Create predicate based on type
- Predicate predicate;
+ if (osDs.isMaterialized(_keyName)) {
+ DataSource keyDs = osDs.getDataSource(_keyName);
+ return buildPerKeyFilterOperator(keyDs, queryContext, numDocs);
+ }
+
+ // Key not materialized
+ if (osDs.isFullyMaterialized()) {
+ // Fully materialized but key absent — definitive answer
+ if (_predicate.getType() == Predicate.Type.IS_NULL) {
+ return new MatchAllFilterOperator(numDocs);
+ }
+ return EmptyFilterOperator.getInstance();
+ }
+
+ // Sparse — can't be sure, fall through to JSON/expression
+ return null;
+ }
+
+ @Nullable
+ private BaseFilterOperator buildPerKeyFilterOperator(DataSource keyDs, QueryContext queryContext, int numDocs) {
switch (_predicate.getType()) {
+ case IS_NULL:
+ case IS_NOT_NULL: {
+ NullValueVectorReader nullReader = keyDs.getNullValueVector();
+ if (nullReader == null) {
Review Comment:
The assumption that a missing null vector means every value is non-null is false for consuming OPEN_STRUCT segments. `MutableKeyColumn` tracks per-key presence, but `MutableOpenStructIndex#getIndexes` exposes only forward, dictionary, and inverted indexes, so this null reader is absent even when the key exists in only some documents. `IS_NULL` consequently returns empty and `IS_NOT_NULL` matches every document. Expose a presence-backed `NullValueVectorReader` from mutable child DataSources before enabling this fast path.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/ItemTransformFunction.java:
##########
@@ -58,9 +60,17 @@ public void init(List<TransformFunction> arguments, Map<String, ColumnContext> c
_keyPath = new String[]{column, key};
DataSource dataSource = columnContextMap.get(column).getDataSource();
- Preconditions.checkState(dataSource instanceof MapDataSource, "Column: %s must be a MAP column", column);
- MapDataSource mapDataSource = (MapDataSource) dataSource;
- DataSource valueDataSource = mapDataSource.getDataSource(key);
+ Preconditions.checkState(dataSource instanceof MapDataSource || dataSource instanceof OpenStructDataSource,
Review Comment:
The new OPEN_STRUCT path inherits `BaseTransformFunction#getNullBitmap()`, which only combines the null bitmaps of the parent `metrics` argument and key literal. The parent OPEN_STRUCT DataSource has no null vector, so the selected dense child's null bitmap is lost. Rows where the key is absent are emitted as default values and treated as non-null, causing wrong projections, composed expressions, and aggregates such as `COUNT(metrics['views'])` with null handling enabled. Override `getNullBitmap()` to return the selected path's BlockValSet null bitmap.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/ItemTransformFunction.java:
##########
@@ -58,9 +60,17 @@ public void init(List<TransformFunction> arguments, Map<String, ColumnContext> c
_keyPath = new String[]{column, key};
DataSource dataSource = columnContextMap.get(column).getDataSource();
- Preconditions.checkState(dataSource instanceof MapDataSource, "Column: %s must be a MAP column", column);
- MapDataSource mapDataSource = (MapDataSource) dataSource;
- DataSource valueDataSource = mapDataSource.getDataSource(key);
+ Preconditions.checkState(dataSource instanceof MapDataSource || dataSource instanceof OpenStructDataSource,
+ "Column: %s must be a MAP or OPEN_STRUCT column", column);
+ DataSource valueDataSource;
+ if (dataSource instanceof MapDataSource) {
+ valueDataSource = ((MapDataSource) dataSource).getDataSource(key);
+ } else {
+ valueDataSource = ((OpenStructDataSource) dataSource).getDataSource(key);
+ }
+ if (valueDataSource == null) {
+ valueDataSource = new NullDataSource(key);
Review Comment:
`NullDataSource` is not a type-correct all-null representation for an absent OPEN_STRUCT key. It is hard-coded as INT, has no null vector, and reports zero documents, so a schema-declared STRING or DOUBLE key missing from one segment becomes INT/0 there while other segments use its actual type. This can corrupt `COUNT`/`MIN`/`MAX` and produce incompatible per-segment result schemas. Construct a typed all-null DataSource from the OPEN_STRUCT child FieldSpec with a null bitmap covering the segment.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "tarun11Mavani (via GitHub)" <gi...@apache.org>.
tarun11Mavani commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3636784319
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/ItemTransformFunction.java:
##########
@@ -58,9 +60,17 @@ public void init(List<TransformFunction> arguments, Map<String, ColumnContext> c
_keyPath = new String[]{column, key};
DataSource dataSource = columnContextMap.get(column).getDataSource();
- Preconditions.checkState(dataSource instanceof MapDataSource, "Column: %s must be a MAP column", column);
- MapDataSource mapDataSource = (MapDataSource) dataSource;
- DataSource valueDataSource = mapDataSource.getDataSource(key);
+ Preconditions.checkState(dataSource instanceof MapDataSource || dataSource instanceof OpenStructDataSource,
+ "Column: %s must be a MAP or OPEN_STRUCT column", column);
+ DataSource valueDataSource;
+ if (dataSource instanceof MapDataSource) {
+ valueDataSource = ((MapDataSource) dataSource).getDataSource(key);
+ } else {
+ valueDataSource = ((OpenStructDataSource) dataSource).getDataSource(key);
Review Comment:
`ImmutableOpenStructDataSource.getDataSource(key)` now returns null for unmaterialized keys instead of falling back to the sparse JSON column. Callers handle the null explicitly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "tarun11Mavani (via GitHub)" <gi...@apache.org>.
tarun11Mavani commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3636789799
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/transform/function/ItemTransformFunction.java:
##########
@@ -58,9 +60,17 @@ public void init(List<TransformFunction> arguments, Map<String, ColumnContext> c
_keyPath = new String[]{column, key};
DataSource dataSource = columnContextMap.get(column).getDataSource();
- Preconditions.checkState(dataSource instanceof MapDataSource, "Column: %s must be a MAP column", column);
- MapDataSource mapDataSource = (MapDataSource) dataSource;
- DataSource valueDataSource = mapDataSource.getDataSource(key);
+ Preconditions.checkState(dataSource instanceof MapDataSource || dataSource instanceof OpenStructDataSource,
Review Comment:
`ItemTransformFunction` now overrides `getNullBitmap()` to return the per-key DataSource's null bitmap directly.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "tarun11Mavani (via GitHub)" <gi...@apache.org>.
tarun11Mavani commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3664377271
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/MutableOpenStructDataSource.java:
##########
@@ -62,10 +63,13 @@ public ComplexFieldSpec getFieldSpec() {
@Override
@Nullable
public DataSource getDataSource(String key) {
- Map<IndexType, IndexReader> indexes = _index.getIndexes(key);
- if (indexes == null || indexes.isEmpty()) {
+ MutableKeyColumn col = _index.getKeyColumn(key);
+ if (col == null) {
return null;
}
+ Map<IndexType, IndexReader> indexes = new HashMap<>(_index.getIndexes(key));
+ indexes.put(StandardIndexes.nullValueVector(),
+ new PresenceBasedNullValueVector(col.getPresenceBitmap(), _numDocs));
Review Comment:
Good catch on the asymmetry. the two sides do get null info by different mechanisms — but the
sealed side is covered. `OpenStructColumnSplitter:326-336` runs a `NullValueVectorCreator` for every
materialized child column at seal time, so dense keys get a real on-disk null vector and the
`nullReader == null` branch isn't reached.
No existing test has a partially-present dense key. I plan to add a dedicated pr with integration tests. I will cover this case as well.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "raghavyadav01 (via GitHub)" <gi...@apache.org>.
raghavyadav01 commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3678929512
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java:
##########
@@ -204,6 +222,55 @@ private boolean isFitForNonScanBasedPlan() {
return true;
}
+ @Nullable
+ private DataSource resolveDataSource(ExpressionContext expression) {
+ return resolveDataSource(expression, _indexSegment, _queryContext.getSchema());
+ }
+
+ @Nullable
+ static DataSource resolveDataSource(ExpressionContext expression, IndexSegment segment,
+ @Nullable org.apache.pinot.spi.data.Schema schema) {
+ if (expression.getType() == ExpressionContext.Type.IDENTIFIER) {
+ return segment.getDataSource(expression.getIdentifier(), schema);
+ }
+ if (expression.getType() == ExpressionContext.Type.FUNCTION) {
+ return tryResolveKeyedDataSource(expression, segment, schema);
+ }
+ return null;
+ }
+
+ @Nullable
+ static DataSource tryResolveKeyedDataSource(ExpressionContext expression, IndexSegment segment,
+ @Nullable org.apache.pinot.spi.data.Schema schema) {
+ FunctionContext function = expression.getFunction();
+ if (function == null
+ || !ItemTransformFunction.FUNCTION_NAME.equals(function.getFunctionName())) {
+ return null;
+ }
+ List<ExpressionContext> args = function.getArguments();
+ if (args.size() != 2
+ || args.get(0).getType() != ExpressionContext.Type.IDENTIFIER
+ || args.get(1).getType() != ExpressionContext.Type.LITERAL) {
+ return null;
+ }
+ String columnName = args.get(0).getIdentifier();
+ String key = args.get(1).getLiteral().getStringValue();
+ DataSource columnDs = segment.getDataSource(columnName, schema);
+ if (columnDs instanceof MapDataSource) {
+ DataSource keyDs = ((MapDataSource) columnDs).getDataSource(key);
+ // An absent MAP key yields a NullDataSource whose metadata reports non-null min/max (the INT
+ // default) and which carries no null vector, so it would wrongly satisfy the metadata-based
+ // non-scan check and report hasNullValues=false. Fall back to a scan, mirroring the
+ // OPEN_STRUCT guard below.
+ return keyDs instanceof NullDataSource ? null : keyDs;
+ }
+ if (columnDs instanceof OpenStructDataSource) {
+ OpenStructDataSource osDs = (OpenStructDataSource) columnDs;
+ return osDs.isMaterialized(key) ? osDs.getDataSource(key) : null;
Review Comment:
For a key present in only some docs, this routes MIN/MAX/DISTINCTCOUNT to the non-scan path, which reads the dictionary — and the mutable dictionary holds present values only. With null handling off, doesn't that diverge from the scan path (and from sealed segments, which fold the default null in at build time)? Might be worth a consuming-vs-sealed test to pin the expected value.
##########
pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/ProjectionBlock.java:
##########
@@ -59,15 +63,37 @@ public BlockValSet getBlockValueSet(ExpressionContext expression) {
@Override
public BlockValSet getBlockValueSet(String column) {
- return new ProjectionBlockValSet(_dataBlockCache, column, _dataSourceMap.get(column));
+ DataSource dataSource = _dataSourceMap.get(column);
+ // An OPEN_STRUCT parent is only a handle for per-key resolution — it has no forward index, so DataFetcher does
+ // not register it and it cannot be read as a column. Reject it here rather than letting the missing
+ // ColumnValueReader surface as an NPE.
+ if (dataSource instanceof OpenStructDataSource) {
+ throw new BadQueryRequestException(
+ "OPEN_STRUCT column: " + column + " cannot be selected directly; use " + column + "['key']");
+ }
+ return new ProjectionBlockValSet(_dataBlockCache, column, dataSource);
}
@Override
public BlockValSet getBlockValueSet(String[] paths) {
// TODO: only support one level of path for now, e.g. `map.key`
assert paths.length == 2;
- MapDataSource mapDataSource = (MapDataSource) _dataSourceMap.get(paths[0]);
- DataSource keyDataSource = mapDataSource.getDataSource(paths[1]);
+ DataSource columnDataSource = _dataSourceMap.get(paths[0]);
+ DataSource keyDataSource;
+ if (columnDataSource instanceof MapDataSource) {
+ keyDataSource = ((MapDataSource) columnDataSource).getDataSource(paths[1]);
+ if (keyDataSource == null) {
+ keyDataSource = new NullDataSource(paths[1]);
+ }
+ } else if (columnDataSource instanceof OpenStructDataSource) {
+ OpenStructDataSource osDs = (OpenStructDataSource) columnDataSource;
+ keyDataSource = osDs.getDataSource(paths[1]);
+ if (keyDataSource == null) {
+ keyDataSource = OpenStructNullDataSource.forAbsentKey(osDs, paths[1]);
Review Comment:
For a sparse (not-yet-materialized) key, `getDataSource` returns null even though the value may still live in the sparse blob, so this projects all-NULL for every row. Should sparse keys fall back to the sparse/JSON read path here, or is materialized-only the intended scope for this PR?
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/MutableOpenStructDataSource.java:
##########
@@ -62,13 +69,19 @@ public ComplexFieldSpec getFieldSpec() {
@Override
@Nullable
public DataSource getDataSource(String key) {
- Map<IndexType, IndexReader> indexes = _index.getIndexes(key);
- if (indexes == null || indexes.isEmpty()) {
+ // Live lookup, outside the memo: a key not yet observed may still be created by the ingestion thread.
+ MutableKeyColumn col = _index.getKeyColumn(key);
+ if (col == null) {
return null;
}
- ColumnMetadata metadata = _index.getColumnMetadata(key);
- return new ImmutableDataSource(metadata,
- new ColumnIndexContainer.FromMap.Builder().withAll(indexes).build());
+ return _perKeyDataSourceCache.computeIfAbsent(key, k -> {
+ Map<IndexType, IndexReader> indexes = new HashMap<>(_index.getIndexes(k));
Review Comment:
This hands the engine `MutableKeyColumn`'s raw forward index, which only has slots written at present docIds (1000-row chunks). A full `[0, numDocs)` projection/scan reads via `_readers.get(docId / 1000)` with no presence check — so wouldn't an absent doc past the last present chunk hit `_readers.get(bufferId)` out-of-bounds, and an in-range absent doc return dictId `0` (the first inserted value)? Should the per-key read be presence-aware (default for absent docs) here, and do we have a consuming-segment test that projects/scans a key absent from the tail rows?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "tarun11Mavani (via GitHub)" <gi...@apache.org>.
tarun11Mavani commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3682629475
##########
pinot-core/src/main/java/org/apache/pinot/core/plan/AggregationPlanNode.java:
##########
@@ -204,6 +222,55 @@ private boolean isFitForNonScanBasedPlan() {
return true;
}
+ @Nullable
+ private DataSource resolveDataSource(ExpressionContext expression) {
+ return resolveDataSource(expression, _indexSegment, _queryContext.getSchema());
+ }
+
+ @Nullable
+ static DataSource resolveDataSource(ExpressionContext expression, IndexSegment segment,
+ @Nullable org.apache.pinot.spi.data.Schema schema) {
+ if (expression.getType() == ExpressionContext.Type.IDENTIFIER) {
+ return segment.getDataSource(expression.getIdentifier(), schema);
+ }
+ if (expression.getType() == ExpressionContext.Type.FUNCTION) {
+ return tryResolveKeyedDataSource(expression, segment, schema);
+ }
+ return null;
+ }
+
+ @Nullable
+ static DataSource tryResolveKeyedDataSource(ExpressionContext expression, IndexSegment segment,
+ @Nullable org.apache.pinot.spi.data.Schema schema) {
+ FunctionContext function = expression.getFunction();
+ if (function == null
+ || !ItemTransformFunction.FUNCTION_NAME.equals(function.getFunctionName())) {
+ return null;
+ }
+ List<ExpressionContext> args = function.getArguments();
+ if (args.size() != 2
+ || args.get(0).getType() != ExpressionContext.Type.IDENTIFIER
+ || args.get(1).getType() != ExpressionContext.Type.LITERAL) {
+ return null;
+ }
+ String columnName = args.get(0).getIdentifier();
+ String key = args.get(1).getLiteral().getStringValue();
+ DataSource columnDs = segment.getDataSource(columnName, schema);
+ if (columnDs instanceof MapDataSource) {
+ DataSource keyDs = ((MapDataSource) columnDs).getDataSource(key);
+ // An absent MAP key yields a NullDataSource whose metadata reports non-null min/max (the INT
+ // default) and which carries no null vector, so it would wrongly satisfy the metadata-based
+ // non-scan check and report hasNullValues=false. Fall back to a scan, mirroring the
+ // OPEN_STRUCT guard below.
+ return keyDs instanceof NullDataSource ? null : keyDs;
+ }
+ if (columnDs instanceof OpenStructDataSource) {
+ OpenStructDataSource osDs = (OpenStructDataSource) columnDs;
+ return osDs.isMaterialized(key) ? osDs.getDataSource(key) : null;
Review Comment:
Confirmed. With the reserved default in the mutable dictionary, a partially-present key's dictionary now has identical contents to the sealed one (default + observed values), so the dictionary-based path is correct there.
The one divergent case — key present in every doc with the default never observed (phantom entry) forces the scan path via a new `OpenStructDataSource.isKeyDictionaryExact`
The parity test pins MIN/MAX/DISTINCTCOUNT consuming vs sealed with null handling off.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "tarun11Mavani (via GitHub)" <gi...@apache.org>.
tarun11Mavani commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3664217154
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/MutableOpenStructDataSource.java:
##########
@@ -62,10 +63,13 @@ public ComplexFieldSpec getFieldSpec() {
@Override
@Nullable
public DataSource getDataSource(String key) {
- Map<IndexType, IndexReader> indexes = _index.getIndexes(key);
- if (indexes == null || indexes.isEmpty()) {
+ MutableKeyColumn col = _index.getKeyColumn(key);
+ if (col == null) {
return null;
}
+ Map<IndexType, IndexReader> indexes = new HashMap<>(_index.getIndexes(key));
Review Comment:
Cached both — the per-key DataSource here, and the derived bitmap in PresenceBasedNullValueVector, since the memo alone wouldn't have helped while the vector didn't cache its own bitmap. Safe because [0, numDocs) is frozen; absent keys are deliberately not memoised so a key created later is still picked up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "tarun11Mavani (via GitHub)" <gi...@apache.org>.
tarun11Mavani commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3664217154
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/MutableOpenStructDataSource.java:
##########
@@ -62,10 +63,13 @@ public ComplexFieldSpec getFieldSpec() {
@Override
@Nullable
public DataSource getDataSource(String key) {
- Map<IndexType, IndexReader> indexes = _index.getIndexes(key);
- if (indexes == null || indexes.isEmpty()) {
+ MutableKeyColumn col = _index.getKeyColumn(key);
+ if (col == null) {
return null;
}
+ Map<IndexType, IndexReader> indexes = new HashMap<>(_index.getIndexes(key));
Review Comment:
Cached both — the per-key DataSource here, and the derived bitmap in PresenceBasedNullValueVector, since the memo alone wouldn't have helped while the vector didn't cache its own bitmap. Safe because [0, numDocs) is frozen; absent keys are deliberately not memoised so a key created later is still picked up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org
Re: [PR] feat(open_struct): query-layer wiring for OPEN_STRUCT columns (PR 3/4) [pinot]
Posted by "tarun11Mavani (via GitHub)" <gi...@apache.org>.
tarun11Mavani commented on code in PR #19040:
URL: https://github.com/apache/pinot/pull/19040#discussion_r3664283222
##########
pinot-segment-local/src/main/java/org/apache/pinot/segment/local/segment/index/openstruct/ImmutableOpenStructDataSource.java:
##########
@@ -84,8 +84,7 @@ public ComplexFieldSpec getFieldSpec() {
@Override
@Nullable
public DataSource getDataSource(String key) {
- DataSource ds = _perKeyDataSources.get(key);
- return ds != null ? ds : _sparseDataSource;
+ return _perKeyDataSources.get(key);
Review Comment:
Yes. This is an intentional change.
The reason for the flip: `_sparseDataSource` is one JSON blob shared by every sparse key, not the
key's values, and it has the wrong type. Returning it meant a caller couldn't distinguish "key
absent" from "key lives in sparse", so it couldn't choose a fallback.
I have updated the SPI method declaration and the java doc for more clarity.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pinot.apache.org
For additional commands, e-mail: commits-help@pinot.apache.org