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: [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]
