Prevents pushing down isnull(nested) in PredicateAnalyzer to correct pushdown of complex & partial filters containing isnull(nested)

Signed-off-by: Yuanchun Shen <yuanchu@amazon.com>

Author: yuancu

opensearch/src/main/java/org/opensearch/sql/opensearch/planner/physical/OpenSearchFilterIndexScanRule.java

@@ -54,7 +54,6 @@ public interface Config extends RelRule.Config {
             .withOperandSupplier(
                 b0 ->
                     b0.operand(LogicalFilter.class)
-                        .predicate(Predicate.not(OpenSearchIndexScanRule::isConditionExcluded))
                         .oneInput(
                             b1 ->
                                 b1.operand(CalciteLogicalIndexScan.class)

opensearch/src/main/java/org/opensearch/sql/opensearch/planner/physical/OpenSearchIndexScanRule.java

@@ -6,20 +6,13 @@
 package org.opensearch.sql.opensearch.planner.physical;
 
 import java.util.HashSet;
-import java.util.List;
 import java.util.Set;
 import org.apache.calcite.plan.RelOptTable;
-import org.apache.calcite.rel.core.Filter;
 import org.apache.calcite.rel.core.Sort;
 import org.apache.calcite.rel.logical.LogicalProject;
 import org.apache.calcite.rel.logical.LogicalSort;
-import org.apache.calcite.rel.type.RelDataTypeField;
-import org.apache.calcite.rex.RexCall;
-import org.apache.calcite.rex.RexInputRef;
 import org.apache.calcite.rex.RexNode;
 import org.apache.calcite.rex.RexOver;
-import org.apache.calcite.sql.SqlKind;
-import org.apache.calcite.sql.type.ArraySqlType;
 import org.opensearch.sql.opensearch.storage.OpenSearchIndex;
 import org.opensearch.sql.opensearch.storage.scan.AbstractCalciteIndexScan;
 
@@ -69,39 +62,4 @@ static boolean isLogicalSortLimit(LogicalSort sort) {
   static boolean sortByFieldsOnly(Sort sort) {
     return !sort.getCollation().getFieldCollations().isEmpty() && sort.fetch == null;
   }
-
-  /**
-   * Filters with conditions of these kinds should not be pushed down since DSL does not handle such
-   * cases correctly
-   */
-  Set<SqlKind> FILTER_PUSHDOWN_EXCLUDED_FUNCTION = Set.of(SqlKind.IS_NULL, SqlKind.IS_NOT_NULL);
-
-  /**
-   * Determines whether a filter condition should be excluded from pushdown to OpenSearch.
-   * Currently, this method only excludes IS NULL and IS NOT NULL operations on nested fields, as
-   * OpenSearch DSL does not handle these conditions correctly.
-   *
-   * @param filter The filter operation to check
-   * @return true if the condition should be excluded from pushdown, false otherwise
-   */
-  static boolean isConditionExcluded(Filter filter) {
-    RexNode condition = filter.getCondition();
-    if (condition instanceof RexCall conditionCall) {
-      return FILTER_PUSHDOWN_EXCLUDED_FUNCTION.contains(conditionCall.getKind())
-          && conditionContainsNestedField(condition, filter.getRowType().getFieldList());
-    }
-    return false;
-  }
-
-  private static boolean conditionContainsNestedField(
-      RexNode condition, List<RelDataTypeField> fields) {
-    if (condition instanceof RexCall conditionCall) {
-      return conditionCall.getOperands().stream()
-          .filter(operand -> operand instanceof RexInputRef)
-          .map(operand -> (RexInputRef) operand)
-          // Nested fields are of type ArraySqlType
-          .anyMatch(inputRef -> fields.get(inputRef.getIndex()).getType() instanceof ArraySqlType);
-    }
-    return false;
-  }
 }

opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java

@@ -72,6 +72,7 @@
 import org.apache.calcite.sql.SqlOperator;
 import org.apache.calcite.sql.SqlSyntax;
 import org.apache.calcite.sql.fun.SqlStdOperatorTable;
+import org.apache.calcite.sql.type.ArraySqlType;
 import org.apache.calcite.sql.type.SqlTypeFamily;
 import org.apache.calcite.sql.type.SqlTypeName;
 import org.apache.calcite.util.NlsString;
@@ -485,6 +486,10 @@ private QueryExpression postfix(RexCall call) {
         String message = format(Locale.ROOT, "Unsupported operator: [%s]", call);
         throw new PredicateAnalyzerException(message);
       }
+
+      // OpenSearch DSL does not handle IS_NULL / IS_NOT_NULL on nested fields correctly
+      checkForNestedFieldOperands(call);
+
       Expression a = call.getOperands().get(0).accept(this);
       // OpenSearch does not want is null/is not null (exists query)
       // for _id and _index, although it supports for all other metadata column
@@ -1357,6 +1362,11 @@ public ScriptQueryExpression(
         RelDataType rowType,
         Map<String, ExprType> fieldTypes,
         RelOptCluster cluster) {
+      if (rexNode instanceof RexCall
+          && (rexNode.getKind().equals(SqlKind.IS_NULL)
+              || rexNode.getKind().equals(SqlKind.IS_NOT_NULL))) {
+        checkForNestedFieldOperands((RexCall) rexNode);
+      }
       ReferenceFieldVisitor validator = new ReferenceFieldVisitor(rowType, fieldTypes, true);
       // Dry run visitInputRef to make sure the input reference ExprType is valid for script
       // pushdown
@@ -1645,4 +1655,29 @@ private static void checkForIncompatibleDateTimeOperands(RexCall call) {
           "Cannot handle " + call.getKind() + " expression for _id field, " + call);
     }
   }
+
+  /**
+   * Examines the operands of a RexCall to check for nested fields and throws an exception if any
+   * are found.
+   *
+   * <p>This check prevents operations (IS_NULL, IS_NOT_NULL) that would produce incorrect results
+   * in OpenSearch when pushed down as either DSL queries or scripts.
+   *
+   * @param call The RexCall to check for nested field operands
+   * @throws PredicateAnalyzerException if any nested fields are detected in the operands
+   */
+  private static void checkForNestedFieldOperands(RexCall call) throws PredicateAnalyzerException {
+    boolean conditionContainsNestedField =
+        call.getOperands().stream()
+            .map(RexNode::getType)
+            // Nested fields are of type ArraySqlType
+            .anyMatch(type -> type instanceof ArraySqlType);
+    if (conditionContainsNestedField) {
+      throw new PredicateAnalyzerException(
+          format(
+              Locale.ROOT,
+              "OpenSearch DSL does not handle %s on nested fields correctly",
+              call.getKind()));
+    }
+  }
 }