Skip to content

[BUG] isnotnull() is giving wrong result #2

Open

Description

@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)"
}
  1. 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": [] }
}

Activity

  1. Repository owner deleted a comment from ryan-gh-bot on May 4, 2026
  2. Repository owner deleted a comment from ryan-gh-bot on May 4, 2026
  3. Repository owner deleted a comment from ryan-gh-bot on May 4, 2026
  4. Repository owner deleted a comment from ryan-gh-bot on May 4, 2026
  5. Repository owner deleted a comment from ryan-gh-bot on May 4, 2026
  6. Repository owner deleted a comment from ryan-gh-bot on May 4, 2026
  7. Repository owner deleted a comment from ryan-gh-bot on May 4, 2026
  8. Repository owner deleted a comment from ryan-gh-bot on May 4, 2026
  9. RyanL1997 commented on May 4, 2026

    @RyanL1997
    OwnerAuthor
  10. ryan-gh-bot commented on May 4, 2026

    @ryan-gh-bot

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

  11. ryan-gh-bot commented on May 4, 2026

    @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

    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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions