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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
