[BugFix] Restore dedup pushdown when combined with WHERE clause (#5482)

Run PPLSimplifyDedupRule before FilterMergeRule in the HEP optimizer so the bucket-non-null filter PPL emits for dedup is matched as-is. With the previous order, an upstream user where filter sat adjacent to the bucket-non-null filter; FilterMergeRule fired first and merged them into a conjunction that no longer satisfied PPLSimplifyDedupRule's operand predicate, defeating dedup pushdown to the shard.

Use sequential addRuleInstance phases for explicit ordering rather than addRuleCollection, which is documented as non-deterministic in firing order. Adds two regression tests in CalcitePPLDedupTest: one that asserts LogicalDedup is produced under the fixed order, and one that pins the buggy behavior under the swapped order.

Signed-off-by: ryan-gh-bot <ryan-gh-bot@users.noreply.github.com>

Author: ryan-gh-bot

core/src/main/java/org/opensearch/sql/calcite/utils/CalciteToolsHelper.java

@@ -36,7 +36,6 @@
 import java.sql.PreparedStatement;
 import java.sql.SQLException;
 import java.time.Instant;
-import java.util.List;
 import java.util.Properties;
 import java.util.function.Consumer;
 import org.apache.calcite.adapter.enumerable.EnumerableConvention;
@@ -59,7 +58,6 @@
 import org.apache.calcite.plan.Convention;
 import org.apache.calcite.plan.RelOptCluster;
 import org.apache.calcite.plan.RelOptPlanner;
-import org.apache.calcite.plan.RelOptRule;
 import org.apache.calcite.plan.RelOptSchema;
 import org.apache.calcite.plan.RelOptTable;
 import org.apache.calcite.plan.RelOptTable.ViewExpander;
@@ -551,12 +549,25 @@ public RelNode visit(TableScan scan) {
     }
   }
 
-  /** Try to optimize the plan by using HepPlanner */
-  private static final List<RelOptRule> hepRuleList =
-      List.of(FilterMergeRule.Config.DEFAULT.toRule(), PPLSimplifyDedupRule.DEDUP_SIMPLIFY_RULE);
-
+  /**
+   * Try to optimize the plan by using HepPlanner.
+   *
+   * <p>Rules are added as separate {@code addRuleInstance} phases so each rule is run to a fixed
+   * point before the next phase begins. {@code PPLSimplifyDedupRule} must run before {@code
+   * FilterMergeRule}: PPL emits the dedup pattern with an {@code IS NOT NULL} bucket-non-null
+   * filter directly above the scan, and the simplify rule's operand requires that filter to remain
+   * a pure {@code IS NOT NULL} (or {@code AND} of {@code IS NOT NULL}s). When a user {@code where}
+   * clause precedes the dedup, the user's filter sits adjacent to the bucket-non-null filter; if
+   * {@code FilterMergeRule} fires first it folds the two into a single conjunction, the simplify
+   * rule's operand match fails, no {@link org.opensearch.sql.calcite.plan.rel.LogicalDedup} is
+   * produced, and dedup pushdown is silently disabled. See
+   * https://github.com/opensearch-project/sql/issues/5482.
+   */
   private static final HepProgram HEP_PROGRAM =
-      new HepProgramBuilder().addRuleCollection(hepRuleList).build();
+      new HepProgramBuilder()
+          .addRuleInstance(PPLSimplifyDedupRule.DEDUP_SIMPLIFY_RULE)
+          .addRuleInstance(FilterMergeRule.Config.DEFAULT.toRule())
+          .build();
 
   public static RelNode optimize(RelNode plan, CalcitePlanContext context) {
     Util.discard(context);

ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLAbstractTest.java

@@ -110,6 +110,20 @@ public RelNode getRelNode(String ppl) {
     return root;
   }
 
+  /**
+   * Get the root RelNode of the given PPL query without merging adjacent filters. Useful for
+   * regression tests that need to exercise rule ordering against the un-merged shape that PPL
+   * actually emits to the production HEP optimizer.
+   */
+  public RelNode getRelNodeRaw(String ppl) {
+    CalcitePlanContext context = createBuilderContext();
+    Query query = (Query) plan(pplParser, ppl);
+    planTransformer.analyze(query.getPlan(), context);
+    RelNode root = context.relBuilder.build();
+    System.out.println(root.explain());
+    return root;
+  }
+
   private RelNode mergeAdjacentFilters(RelNode relNode) {
     HepProgram program =
         new HepProgramBuilder().addRuleInstance(FilterMergeRule.Config.DEFAULT.toRule()).build();

ppl/src/test/java/org/opensearch/sql/ppl/calcite/CalcitePPLDedupTest.java

@@ -5,9 +5,15 @@
 
 package org.opensearch.sql.ppl.calcite;
 
+import org.apache.calcite.plan.hep.HepPlanner;
+import org.apache.calcite.plan.hep.HepProgram;
+import org.apache.calcite.plan.hep.HepProgramBuilder;
 import org.apache.calcite.rel.RelNode;
+import org.apache.calcite.rel.rules.FilterMergeRule;
 import org.apache.calcite.test.CalciteAssert;
+import org.junit.Assert;
 import org.junit.Test;
+import org.opensearch.sql.calcite.plan.rule.PPLSimplifyDedupRule;
 
 public class CalcitePPLDedupTest extends CalcitePPLAbstractTest {
 
@@ -353,4 +359,89 @@ public void testSortFieldProjectedAwayBeforeDedup() {
             + "            LogicalTableScan(table=[[scott, EMP]])\n";
     verifyLogical(root, expectedLogical);
   }
+
+  /**
+   * Regression test for https://github.com/opensearch-project/sql/issues/5482
+   *
+   * <p>When a user {@code where} precedes {@code dedup}, the user filter sits adjacent to the
+   * bucket-non-null filter that PPL emits for the dedup pattern. The HEP optimizer must run {@link
+   * PPLSimplifyDedupRule} before {@link FilterMergeRule}; otherwise the merged condition breaks the
+   * simplify-rule's operand match and dedup falls through to the in-memory {@code ROW_NUMBER}
+   * window form, defeating dedup pushdown to the shard.
+   *
+   * <p>This test mirrors the production HEP rule sequence in {@code CalciteToolsHelper#HEP_PROGRAM}
+   * and asserts that a {@code LogicalDedup} is produced in the presence of a user {@code where}.
+   */
+  @Test
+  public void testDedupAfterWhereProducesLogicalDedup() {
+    String ppl = "source=EMP | where SAL > 1000 | dedup DEPTNO";
+    RelNode raw = getRelNodeRaw(ppl);
+
+    // Sanity: the un-merged plan has both the bucket-non-null filter and the user where filter
+    // adjacent to each other above the scan — exactly the shape that triggers the bug.
+    String rawExplain = raw.explain();
+    Assert.assertTrue(
+        "Raw plan should contain the bucket-non-null filter:\n" + rawExplain,
+        rawExplain.contains("IS NOT NULL"));
+    Assert.assertTrue(
+        "Raw plan should contain the user where filter:\n" + rawExplain,
+        rawExplain.contains("1000"));
+    Assert.assertTrue(
+        "Raw plan should contain ROW_NUMBER prior to simplification:\n" + rawExplain,
+        rawExplain.contains("ROW_NUMBER"));
+
+    // Apply the production HEP rule sequence: PPLSimplifyDedupRule first, then FilterMergeRule.
+    HepProgram program =
+        new HepProgramBuilder()
+            .addRuleInstance(PPLSimplifyDedupRule.DEDUP_SIMPLIFY_RULE)
+            .addRuleInstance(FilterMergeRule.Config.DEFAULT.toRule())
+            .build();
+    HepPlanner planner = new HepPlanner(program);
+    planner.setRoot(raw);
+    RelNode optimized = planner.findBestExp();
+
+    String optimizedExplain = optimized.explain();
+    Assert.assertTrue(
+        "Optimized plan should contain LogicalDedup so DedupPushdownRule can match it:\n"
+            + optimizedExplain,
+        optimizedExplain.contains("LogicalDedup"));
+    Assert.assertFalse(
+        "Optimized plan should not retain ROW_NUMBER after simplification:\n" + optimizedExplain,
+        optimizedExplain.contains("ROW_NUMBER"));
+  }
+
+  /**
+   * Companion to {@link #testDedupAfterWhereProducesLogicalDedup} that pins the buggy behavior:
+   * when {@link FilterMergeRule} runs before {@link PPLSimplifyDedupRule}, the merged conjunction
+   * defeats the simplify-rule's operand match and no {@code LogicalDedup} is produced. This guards
+   * against accidentally swapping the rule order in {@code CalciteToolsHelper#HEP_PROGRAM} back to
+   * the buggy ordering.
+   */
+  @Test
+  public void testDedupAfterWhereWithFilterMergeFirstFailsToSimplify() {
+    String ppl = "source=EMP | where SAL > 1000 | dedup DEPTNO";
+    RelNode raw = getRelNodeRaw(ppl);
+
+    // Buggy order: FilterMergeRule first, then PPLSimplifyDedupRule.
+    HepProgram program =
+        new HepProgramBuilder()
+            .addRuleInstance(FilterMergeRule.Config.DEFAULT.toRule())
+            .addRuleInstance(PPLSimplifyDedupRule.DEDUP_SIMPLIFY_RULE)
+            .build();
+    HepPlanner planner = new HepPlanner(program);
+    planner.setRoot(raw);
+    RelNode optimized = planner.findBestExp();
+
+    String optimizedExplain = optimized.explain();
+    Assert.assertFalse(
+        "Buggy order: PPLSimplifyDedupRule should not match after FilterMergeRule consolidates"
+            + " the bucket-non-null filter with the user where filter, so no LogicalDedup is"
+            + " produced:\n"
+            + optimizedExplain,
+        optimizedExplain.contains("LogicalDedup"));
+    Assert.assertTrue(
+        "Buggy order: ROW_NUMBER window form should remain since simplification was skipped:\n"
+            + optimizedExplain,
+        optimizedExplain.contains("ROW_NUMBER"));
+  }
 }