Description

PPL dedup is initially compiled into a ROW_NUMBER() OVER (PARTITION BY field) window pattern with an IS_NOT_NULL(field) bucket-non-null filter directly above the scan. PPLSimplifyDedupRule is supposed to fold that pattern into a LogicalDedup, which DedupPushdownRule then pushes to the scan as a composite + top_hits aggregation.

In the previous HEP program (CalciteToolsHelper.java), FilterMergeRule and PPLSimplifyDedupRule were both registered through a single addRuleCollection(...) call, which uses MatchOrder.ARBITRARY. With this scheduling the order in which the two rules fire is non-deterministic. When a user where clause sits between the bucket-non-null filter and the scan, those two filters are adjacent above the scan; if FilterMergeRule happens to fire first, it folds them into AND(IS_NOT_NULL(field), <user predicate>). That merged condition no longer satisfies PlanUtils.mayBeFilterFromBucketNonNull (which only accepts a pure IS_NOT_NULL or an AND-of-IS_NOT_NULLs), PPLSimplifyDedupRule's operand never matches, no LogicalDedup is produced, and DedupPushdownRule has nothing to match - so the dedup falls through to a coordinator-side ROW_NUMBER window. On large indices that path can exceed timeouts or trigger shard failures.

The fix splits the HEP program into two ordered instructions so that PPLSimplifyDedupRule runs to fixpoint against the original adjacent-filter shape before any merging happens, and then FilterMergeRule runs. The remaining user where filter ends up below the new LogicalDedup, where the existing Volcano FilterIndexScanRule pushes it into the scan, and DedupPushdownRule then pushes the dedup down as composite + top_hits.

The fix touches one file in production (core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java); a regression test is added in ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLDedupTest.java together with a small helper in CalcitePPLAbstractTest that returns the analyzer plan without the test-only auxiliary FilterMergeRule pass, so the production HEP can be exercised against the un-merged shape.

Verification

Before - the failing post-HEP plan shape captured by the bug report (EXPLAIN against a real cluster). With any user where upstream of dedup, the scan emits _source and dedup runs in memory:

EnumerableLimit(fetch=[10000])
  EnumerableCalc(<= row_number 1)
    EnumerableWindow(ROW_NUMBER() OVER (PARTITION BY $8))
      CalciteEnumerableIndexScan(
        PushDownContext=[[PROJECT, FILTER->AND(>(@ts,...), IS NOT NULL(...)), PROJECT]],
        sourceBuilder={"_source":{"includes":[...]},
                       "query":{"bool":{"must":[{"range":...},{"exists":...}]}}})

The newly added unit test CalcitePPLDedupTest.testDedupAfterWhereSimplifiesToLogicalDedup asserts the desired shape by running the production CalciteToolsHelper.optimize HEP on the analyzer plan for source=EMP | where DEPTNO > 10 | dedup 1 DEPTNO | fields DEPTNO. It checks that the analyzer plan still has the un-merged adjacent filters (the input the simplify rule expects) and that the post-HEP plan deterministically contains a LogicalDedup, retains the user predicate, and no longer contains a ROW_NUMBER window.

After - run on this branch:

$ JAVA_HOME=/usr/lib/jvm/java-21-amazon-corretto ./gradlew :ppl:test --tests \
    "org.opensearch.sql.ppl.calcite.CalcitePPLDedupTest"
...
CalcitePPLDedupTest > testDedupAfterWhereSimplifiesToLogicalDedup PASSED
CalcitePPLDedupTest > testDedup1 PASSED
CalcitePPLDedupTest > testDedup2 PASSED
CalcitePPLDedupTest > testDedupExpr PASSED
CalcitePPLDedupTest > testDedupKeepEmpty1 PASSED
CalcitePPLDedupTest > testDedupKeepEmpty2 PASSED
CalcitePPLDedupTest > testRenameDedup PASSED
CalcitePPLDedupTest > testSortFieldProjectedAwayBeforeDedup PASSED
CalcitePPLDedupTest > testSortThenDedup PASSED
CalcitePPLDedupTest > testSortThenDedupWithEval PASSED
BUILD SUCCESSFUL

Broader regression coverage - the full :ppl:test, :core:test, :opensearch:test and :sql:test suites pass with no failures on this branch. spotlessCheck is also clean.

The bug report's "actual" EXPLAIN output (filter merged into AND(>(@ts,...), IS NOT NULL(...)), EnumerableWindow over the scan, no aggregation in DSL) requires a live cluster with the reporter's index template and dataset to reproduce verbatim and is not exercised by the unit-test harness. The fix targets the documented root cause - non-deterministic HEP rule ordering between FilterMergeRule and PPLSimplifyDedupRule - and the new unit test guards the deterministic post-HEP behavior we want regardless of the underlying scheduler.

Note on test scoping: in synthetic plan shapes built by CalcitePPLAbstractTest, the unfixed addRuleCollection ordering happens to fire PPLSimplifyDedupRule first under the current Calcite vertex iteration order, so the new test alone does not flip from FAIL to PASS on those shapes. The fix is still required to make the ordering deterministic and to address the documented production failure mode; the new test serves as a deterministic guard against regressions in the post-HEP behavior.

Related Issues

Resolves #5

Check List

• New functionality includes testing.
• New functionality has been documented.
• New PPL command checklist all confirmed.
• API changes companion pull request created.
• Commits are signed per the DCO using --signoff or -s.
• Public documentation issue/PR created.

This pull request was authored by @ryan-gh-bot in response to the maintainer command /fix on #5 (invoked by @RyanL1997). This is a maintainer-triggered automated contribution. Please review carefully before merging.