[BUG] `isnotnull()` is giving wrong result

[RyanL1997]

What is the bug?
When using isnotnull() with certain conditions, it won't be pushed down

How can one reproduce the bug?
Steps to reproduce the behavior:

1. Run

POST /testindex/_doc
{
  "a": "1"
}
POST /testindex/_doc
{
  "a": "2"
}
POST /testindex/_doc
{
  "a": null,
  "b": "1"
}

POST /_plugins/_ppl
{
  "query": "source = testindex | where a != '1' and a != '2' and isnotnull(a)"
}

2. See query result includes doc where a is null

{
  "schema": [
    {
      "name": "a",
      "type": "string"
    },
    {
      "name": "b",
      "type": "string"
    }
  ],
  "datarows": [
    [
      null,
      "1"
    ]
  ],
  "total": 1,
  "size": 1
}

What is the expected behavior?

isnotnull() should always filter out null values

What is your host/environment?

• OS: [e.g. iOS] linux
• Version [e.g. 22] 2.19
• Plugins all

Do you have any screenshots?
If applicable, add screenshots to help explain your problem.

Do you have any additional context?

explain

// source = testindex | where a != '1' and isnotnull(a)
{
  "calcite": {
    "logical": """LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])
  LogicalProject(a=[$0], b=[$1])
    LogicalFilter(condition=[<>($0, '1')])
      CalciteLogicalIndexScan(table=[[OpenSearch, testindex]])
""",
    "physical": """CalciteEnumerableIndexScan(table=[[OpenSearch, testindex]], PushDownContext=[[PROJECT->[a, b], FILTER-><>($0, '1'), LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={"from":0,"size":10000,"timeout":"1m","query":{"bool":{"must":[{"exists":{"field":"a","boost":1.0}}],"must_not":[{"term":{"a.keyword":{"value":"1","boost":1.0}}}],"adjust_pure_negative":true,"boost":1.0}},"_source":{"includes":["a","b"],"excludes":[]}}, requestedTotalSize=10000, pageSize=null, startFrom=0)])
"""
  }
}
{
  "from": 0,
  "size": 10000,
  "timeout": "1m",
  "query": {
    "bool": {
      "must": [{ "exists": { "field": "a", "boost": 1.0 } }],
      "must_not": [{ "term": { "a.keyword": { "value": "1", "boost": 1.0 } } }],
      "adjust_pure_negative": true,
      "boost": 1.0
    }
  },
  "_source": { "includes": ["a", "b"], "excludes": [] }
}

// source = testindex | where a != '1' and a != '2' and isnotnull(a)
{
  "calcite": {
    "logical": """LogicalSystemLimit(fetch=[10000], type=[QUERY_SIZE_LIMIT])
  LogicalProject(a=[$0], b=[$1])
    LogicalFilter(condition=[SEARCH($0, Sarg[(-∞..'1'), ('1'..'2'), ('2'..+∞); NULL AS FALSE]:CHAR(1))])
      CalciteLogicalIndexScan(table=[[OpenSearch, testindex]])
""",
    "physical": """CalciteEnumerableIndexScan(table=[[OpenSearch, testindex]], PushDownContext=[[PROJECT->[a, b], FILTER->SEARCH($0, Sarg[(-∞..'1'), ('1'..'2'), ('2'..+∞); NULL AS FALSE]:CHAR(1)), LIMIT->10000], OpenSearchRequestBuilder(sourceBuilder={"from":0,"size":10000,"timeout":"1m","query":{"bool":{"must_not":[{"terms":{"a.keyword":["1","2"],"boost":1.0}}],"adjust_pure_negative":true,"boost":1.0}},"_source":{"includes":["a","b"],"excludes":[]}}, requestedTotalSize=10000, pageSize=null, startFrom=0)])
"""
  }
}
{
  "from": 0,
  "size": 10000,
  "timeout": "1m",
  "query": {
    "bool": {
      "must_not": [{ "terms": { "a.keyword": ["1", "2"], "boost": 1.0 } }],
      "adjust_pure_negative": true,
      "boost": 1.0
    }
  },
  "_source": { "includes": ["a", "b"], "excludes": [] }
}

[RyanL1997]

@ryan-gh-bot /rca

[ryan-gh-bot]

Working on /rca (invoked by @RyanL1997) — posting result when done.

[ryan-gh-bot]

[RCA] isnotnull() drop-off is a pushdown regression in PredicateAnalyzer's SEARCH handling; fixed on main

When Calcite folds a != '1' AND a != '2' AND isnotnull(a) into a single SEARCH($0, Sarg[(-∞..'1'), ('1'..'2'), ('2'..+∞); NULL AS FALSE]), PredicateAnalyzer emits must_not: [terms a.keyword in ['1','2']] without an accompanying exists clause, so documents where a is null/missing slip through. The fix that restores the exists wrapping for complemented-points / range Sargs with NULL AS FALSE already landed on main as commit b4df01060 (PR opensearch-project#5238) and is not in 3.4/3.5.

Environment

• Tenant: sql (opensearch-project/sql)
• Refs checked:
  • upstream/main @ d3bdca82a (contains the fix)
  • upstream/3.5 @ d680c44cc (regression present)
  • upstream/3.4 (regression present)
  • upstream/3.3 (correct — pre-regression shape)
  • upstream/3.2 (correct)
• Note on the reported "Version 2.19": upstream/2.19 does not contain PredicateAnalyzer.java at all (no Calcite engine on 2.x). The Calcite logical/physical plans in the issue match a 3.x release; this bug reproduces on 3.4 and 3.5.

Suspected location

• opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java L737-L771 — the case SEARCH arm of binary() that switches on Sarg.nullAs. The case FALSE branch is the one that was incorrectly collapsed to expression during 3.4.
• opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java L785-L807 — constructQueryExpressionForSearch: for isSearchWithComplementedPoints returns notIn(...), which is implemented as boolQuery().mustNot(termsQuery(...)) with no exists filter of its own.
• opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java L1557-L1563 — notIn(...) producing the bare must_not(terms) builder that becomes the physical DSL observed in the issue.

Analysis

The reporter's two queries diverge inside PredicateAnalyzer because Calcite represents them differently:

1. where a != '1' and isnotnull(a) — Calcite does not build a Sarg. The LogicalFilter is just <>($0, '1'). This goes through NamedFieldExpression.notEquals, which explicitly builds boolQuery().must(existsQuery(field)).mustNot(termQuery(...)). That is why the working physical plan contains must: [exists a].

2. where a != '1' and a != '2' and isnotnull(a) — Calcite merges all three conjuncts into a single SEARCH($0, Sarg[(-∞..'1'), ('1'..'2'), ('2'..+∞); NULL AS FALSE]). That Sarg satisfies Calcite's Sarg.isComplementedPoints() (range spans (-∞,+∞), not the all-range, and the complement is a set of points: {'1','2'}). The NULL AS FALSE encodes the isnotnull(a) conjunct directly into the Sarg's null-handling semantics.

Inside binary(call):

// opensearch/.../PredicateAnalyzer.java (abridged)
case SEARCH:
  QueryExpression expression = constructQueryExpressionForSearch(call, pair);
  RexUnknownAs nullAs = getNullAsForSearch(call);
  QueryExpression finalExpression =
      switch (nullAs) {
        case FALSE ->
            isSearchWithPoints(call)
                ? expression
                : CompoundQueryExpression.and(
                    false, expression, QueryExpression.create(pair.getKey()).exists());
        // ...
      };

constructQueryExpressionForSearch routes complemented-points to .notIn(...), which is boolQuery().mustNot(termsQuery(...)) — crucially, no exists filter (unlike .notEquals). On main, the case FALSE arm then checks isSearchWithPoints(call) — false for this Sarg (no ranges are points) — and wraps the expression in CompoundQueryExpression.and(expression, exists(field)), producing the expected {must: [{bool: {must_not: [terms]}}, {exists: field}]}. That is exactly what the regression test added alongside the fix asserts.

On 3.4/3.5, the same arm is simply case FALSE -> expression;, so the exists filter is never added and nulls pass through must_not(terms ...). PR opensearch-project#4877 introduced this simplification with a TODO-style comment speculating it was equivalent; that assumption only holds for isSearchWithPoints (plain IN), not for the complemented-points (NOT IN) or range cases.

Evidence

Regression commit (introduced case FALSE -> expression;):

$ git -P log --oneline -S 'case FALSE -> expression;' -- \
    opensearch/src/main/java/org/opensearch/sql/opensearch/request/PredicateAnalyzer.java
b4df01060 [BugFix] Fix `isnotnull()` not being pushed down when combined with multiple `!=` conditions (#5238)
2f64c40ce Remove unnecessary filter for DateHistogram aggregation (#4877)

Diff of the regression in 2f64c40ce (PR opensearch-project#4877):

-                // e.g. where isNotNull(a) and (a = 1 or a = 2)
-                case FALSE ->
-                    CompoundQueryExpression.and(
-                        false, expression, QueryExpression.create(pair.getKey()).exists());
+                // e.g. where isNotNull(a) and ( a = 1 or a = 2)
+                // For this case, return `expression` is equivalent
+                // But DSL `bool.must` could slow down the query, so we return `expression`
+                case FALSE -> expression;

Fix in b4df01060 (PR opensearch-project#5238):

-                case FALSE -> expression;
+                case FALSE ->
+                    isSearchWithPoints(call)
+                        ? expression
+                        : CompoundQueryExpression.and(
+                            false, expression, QueryExpression.create(pair.getKey()).exists());

Regression test added by the fix exactly mirrors the reporter's scenario — opensearch/src/test/java/org/opensearch/sql/opensearch/request/PredicateAnalyzerTest.java — search_complementedPointsWithNullAsFalse_generatesExistsAndNotInQuery:

// Simulates: a != 12 AND a != 13 AND isnotnull(a)
// Calcite merges this into SEARCH($0, Sarg[...; NULL AS FALSE]) with complemented points
Sarg<BigDecimal> sarg =
    Sarg.of(
        RexUnknownAs.FALSE,
        ImmutableRangeSet.<BigDecimal>builder()
            .add(Range.lessThan(BigDecimal.valueOf(12)))
            .add(Range.open(BigDecimal.valueOf(12), BigDecimal.valueOf(13)))
            .add(Range.greaterThan(BigDecimal.valueOf(13)))
            .build());
// ...
// asserts generated DSL == must: [{bool: {must_not: [terms]}}, {exists: field}]

Per-branch status of the case FALSE arm:

Ref	case FALSE body	Bug?
upstream/3.2	unconditional and(expression, exists())	No
upstream/3.3	unconditional and(expression, exists())	No
upstream/3.4	expression;	Yes
upstream/3.5	expression;	Yes
upstream/main @ d3bdca82a	isSearchWithPoints ? expression : and(expression, exists())	No

Live server verification was not performed: the two independent code signals (exact regression commit and exact matching fix commit, both with targeted tests) converge on the same explanation, so escalation to /reproduce is unnecessary.

Confidence: High

The regression commit, the fixing commit, the Calcite Sarg semantics for isComplementedPoints()/nullAs, and the dedicated regression test in the fix PR all agree on the same root cause and the same code path.

Next steps

• The fix ([BugFix] Fix isnotnull() not being pushed down when combined with multiple != conditions opensearch-project/sql#5238, b4df01060) is already on main. If there is no existing backport issue, file one for 3.5 and 3.4 — those branches are the ones that actually exhibit this behavior.
• Reporter's "Version 2.19" does not line up with the Calcite plans they pasted (no PredicateAnalyzer on 2.19). Worth confirming which plugin build they are actually running so the backport target is correct.
• Unverified suggestion (workaround until a backport ships): rewrite the PPL as ... | where a not in ('1', '2') and isnotnull(a) — but given the same SEARCH+Sarg path is likely taken, this may not avoid the bug. A safer form would be to split the conjunct using ispresent(a) / a non-collapsible predicate, or to keep the single a != '1' form that doesn't collapse into a Sarg. These were not run against a live cluster.

---

Automated response from @ryan-gh-bot (maintainer-triggered agent). About this agent.