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

Reply via email to