[BUG] Dedup pushdown disabled when combined with WHERE clause (FilterMergeRule defeats PPLSimplifyDedupRule) #7

Activity

1. RyanL1997

   added

   bugSomething isn't workingSomething isn't working

   untriaged

   on May 28, 2026
2. RyanL1997 commented on May 28, 2026

   

   RyanL1997

   on May 28, 2026

   OwnerAuthor

   More actions

   @ryan-gh-bot, can you root cause this?
3. ryan-gh-bot commented on May 28, 2026

   

   ryan-gh-bot

   on May 28, 2026

   More actions

   Working on @triage (invoked by @RyanL1997) — posting result when done.
4. ryan-gh-bot commented on May 28, 2026

   

   ryan-gh-bot

   on May 28, 2026

   More actions

   [RCA] FilterMergeRule runs before PPLSimplifyDedupRule and merges the user WHERE into the bucket-non-null filter, so the dedup-simplify pattern fails to match and LogicalDedup is never produced

   The HEP program in CalciteToolsHelper registers FilterMergeRule ahead of PPLSimplifyDedupRule. When a user WHERE sits below the synthetic IS NOT NULL filter that PPL emits as part of dedup (keepempty=false), the two adjacent filters are merged into one whose condition is AND(IS_NOT_NULL(field), <user predicate>). The simplify rule's bottom operand requires that filter to satisfy mayBeFilterFromBucketNonNull, which only accepts pure IS_NOT_NULL (or AND-of-IS_NOT_NULLs); the merged condition fails the predicate, the rule never fires, no LogicalDedup is produced, and the Volcano-time DedupPushdownRule has nothing to match — dedup falls back to the row-number window form executed on the coordinator.

   Environment

   • Tenant: sql (opensearch-project/sql)
   • Refs checked:
     • target/main @ d099d3eff

   Suspected location

   • core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java L555-L559 — HEP rule order: FilterMergeRule then PPLSimplifyDedupRule.DEDUP_SIMPLIFY_RULE.
   • core/src/main/java/org/opensearch/sql/calcite/utils/PlanUtils.java L788-L799 — mayBeFilterFromBucketNonNull + isNotNullOnRef: only accepts a pure IS_NOT_NULL on a RexInputRef, or an AND whose every operand is such an IS_NOT_NULL.
   • core/src/main/java/org/opensearch/sql/calcite/plan/rule/PPLSimplifyDedupRule.java L158-L184 — operand pattern: the bottom LogicalFilter (immediate child of the window LogicalProject) must satisfy PlanUtils::mayBeFilterFromBucketNonNull.
   • core/src/main/java/org/opensearch/sql/calcite/plan/rule/PPLDedupConvertRule.java L151-L188 — buildDedupNotNull emits the IS NOT NULL(...) filter directly above the existing input; if the user added | where ..., that user filter is the input — the two filters are adjacent.
   • opensearch/src/main/java/org/opensearch/sql/opensearch/planner/rules/DedupPushdownRule.java — Volcano substitution rule whose operand is rooted at LogicalDedup; it cannot fire unless the simplify rule has produced one.

   Analysis

   PPL | dedup field (default keepempty=false) is compiled by PPLDedupConvertRule.buildDedupNotNull into the following bottom-up shape (the comment in that method documents it explicitly):

   LogicalProject(...)                                              -- finalProject (drop _row_number_dedup_)
   +- LogicalFilter(condition=[<=(_row_number_dedup_, n)])          -- numOfDedupFilter
      +- LogicalProject(..., _row_number_dedup_=ROW_NUMBER() ...)   -- projectWithWindow
         +- LogicalFilter(condition=[AND(IS NOT NULL(a), ...)])     -- bucketNonNullFilter
            +- <input with Sort stripped>
   

   PPLSimplifyDedupRule is a HEP rule whose 4-level operand matches that exact pattern and rewrites it into a LogicalDedup, which DedupPushdownRule later pushes to the OpenSearch index scan as composite + top_hits.

   The bug is in the interaction between the two rules registered in the HEP program:

   1. Without | where, the bucket-non-null filter's input is the bare scan. PPLSimplifyDedupRule matches, produces LogicalDedup, and DedupPushdownRule pushes it. This is the "good" EXPLAIN the reporter showed (composite aggregation + top_hits).
   2. With | where ts > X, the user predicate is already a LogicalFilter directly under the bucket-non-null filter when buildDedupNotNull runs. The two adjacent LogicalFilters are exactly the shape FilterMergeRule is designed to collapse. Because FilterMergeRule is listed first in hepRuleList, the HEP planner fires it first and rewrites the subtree to a single filter whose condition is AND(IS_NOT_NULL(field), ts > X).
   3. PPLSimplifyDedupRule's operand b3 then evaluates mayBeFilterFromBucketNonNull against that merged filter. The predicate's AND branch requires rexCall.getOperands().stream().allMatch(PlanUtils::isNotNullOnRef); the > operand is not an IS_NOT_NULL, so the predicate returns false. The operand never matches, no transform is applied, no LogicalDedup is produced.
   4. Volcano then plans the un-simplified subtree: the ROW_NUMBER OVER (PARTITION BY ...) window survives, DedupPushdownRule finds nothing matching its LogicalDedup root, and the scan emits _source rather than the composite aggregation. This is the "bad" EXPLAIN with EnumerableLimit -> EnumerableCalc -> EnumerableWindow -> CalciteEnumerableIndexScan that the reporter observed.

   This is consistent with the symptom: dedup pushdown works on source=logs-* | dedup field but breaks the moment any WHERE lands between the dedup compilation output and the scan.

   Evidence

   • Grep/read: confirmed the HEP rule list at CalciteToolsHelper.java:555-556 is literally List.of(FilterMergeRule.Config.DEFAULT.toRule(), PPLSimplifyDedupRule.DEDUP_SIMPLIFY_RULE) — FilterMergeRule is first.
   • Read: PlanUtils.mayBeFilterFromBucketNonNull at lines 788-794 only returns true for a pure IS_NOT_NULL on a RexInputRef or an AND whose operands are all such; mixing in a non-IS_NOT_NULL (e.g., a range comparison from a user WHERE) makes it return false. Confirmed by reading isNotNullOnRef at lines 796-799.
   • Read: PPLSimplifyDedupRule.Config.DEFAULT at lines 158-184 anchors the bottom of its operand on a LogicalFilter predicated on PlanUtils::mayBeFilterFromBucketNonNull. There is no fallback path that accepts a wider filter.
   • Read: PPLDedupConvertRule.buildDedupNotNull at lines 151-188 emits the IS NOT NULL filter as the immediate child of whatever input was on the rel-builder stack — guaranteeing adjacency to a user WHERE if one is present.
   • Read: DedupPushdownRule.Config.DEFAULT operand is rooted at LogicalDedup (b0.operand(LogicalDedup.class)...), so it depends on the simplify rule having succeeded.
   • The reporter's EXPLAIN output is consistent with this hypothesis on both sides — composite + top_hits when the simplify rule fires, EnumerableWindow form when it doesn't.
   • Regression check: not performed — the reporter does not claim a prior version worked; this appears to be a long-standing limitation surfaced by a pushdown change rather than a regression.
   • Live verification: not performed — four converging code-reading signals (HEP order, predicate shape, simplify operand shape, pushdown rule depending on LogicalDedup) yield high confidence without needing runtime confirmation.

   Confidence: High
   Four independent signals from reading the actual files at the checked-out sha (HEP rule order, mayBeFilterFromBucketNonNull shape, PPLSimplifyDedupRule operand definition, DedupPushdownRule operand depending on LogicalDedup) all converge on the same explanation, and the EXPLAIN evidence quoted in the issue body matches both paths exactly.

   Next steps

   • Two viable fix directions, both already noted by the reporter; the second is mechanically simpler and should be considered first:
     • Reorder the HEP program so PPLSimplifyDedupRule.DEDUP_SIMPLIFY_RULE is registered before FilterMergeRule.Config.DEFAULT.toRule() in CalciteToolsHelper.java L555-L556. The dedup pattern matcher then sees the original adjacent-filter shape; FilterMerge can run after and the user filter remains pushable normally. Lowest risk because it doesn't change the simplify rule's invariants — it only fixes the rule-order race.
     • Loosen mayBeFilterFromBucketNonNull in PlanUtils.java L788-L794 to accept any AND containing an IS_NOT_NULL on the partition column(s), and split the matched filter so the non-IS_NOT_NULL conjuncts are re-attached as a separate filter that can be pushed normally. More invasive — PPLSimplifyDedupRule.apply would also need to thread the residual predicate above the dedup it produces.
   • Unverified suggestion: a unit test in core/src/test/java/.../calcite/plan/rule/ that constructs a LogicalProject(window) -> LogicalFilter(IS_NOT_NULL) -> LogicalFilter(user predicate) -> Scan plan, runs the current HEP_PROGRAM, and asserts that a LogicalDedup was produced would prevent regressions on either fix.
   • If a maintainer wants empirical confirmation before committing to a fix, follow up with @ryan-gh-bot /reproduce so the reporter's EXPLAIN can be replayed against the local server.

   ---

   Automated response from @ryan-gh-bot (maintainer-triggered agent). About this agent.
5. RyanL1997 commented on May 28, 2026

   

   RyanL1997

   on May 28, 2026

   OwnerAuthor

   More actions

   @ryan-gh-bot, fix it based on your rca
6. ryan-gh-bot commented on May 28, 2026

   

   ryan-gh-bot

   on May 28, 2026

   More actions

   Working on @triage (invoked by @RyanL1997) — posting result when done.
7. ryan-gh-bot commented on May 28, 2026

   

   ryan-gh-bot

   on May 28, 2026

   More actions

   Opened pull request #8

   This was generated by /fix invoked by @RyanL1997. The PR body includes Before/After verification output. Please review.