[CORE] Add distinct FILTER boundary coverage for two-phase aggregate merge

[Smallfu666]

What changes are proposed in this pull request?

This records a non-merge boundary exposed while reviewing PR #12653.

PR #12653 added FILTER-clause support to MergeTwoPhasesHashBaseAggregate. Its code comment used the single-distinct case as rationale for positional alignment over resultId matching. However, single-distinct + FILTER is outside this merge rule's scope: planAggregateWithOneDistinct inserts an exchange between the partial and final stages, breaking the direct parent-child relationship the rule requires.

This PR:

1. Corrects the comment — removes the resultId rationale, keeps only the positional-alignment invariant.
2. Adds two boundary characterization tests verifying that distinct + FILTER is not merged into a single aggregate and that FILTER is preserved through the unmerged stages (results match vanilla Spark).

These tests would pass on main before #12653 was merged — distinct aggregates were never merged because of the exchange. This PR records a non-merge boundary, not a correctness regression fixed by #12653.

How was this patch tested?

Verified on Spark 3.4 + Scala 2.12 (AEOn + AEOff, 6/6 tests pass) on the existing MergeTwoPhasesHashBaseAggregateSuite. Both new cases use compareResultsAgainstVanillaSpark (which checks Gluten result against vanilla Spark oracle and verifies no fallback).

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Opus 4.8 GPT 5.6 sol

[github-actions]

Run Gluten Clickhouse CI on x86

[Copilot]

Pull request overview

This PR clarifies the intended scope/behavior of MergeTwoPhasesHashBaseAggregate around DISTINCT + FILTER by (1) correcting an in-code rationale comment and (2) adding characterization tests that record the non-merge boundary caused by Spark’s planAggregateWithOneDistinct exchange insertion, while also validating results against vanilla Spark.

Changes:

• Update the merge-rule comment to rely only on Spark’s positional alignment invariant (not resultId matching) when restoring FILTER predicates.
• Add two new test cases asserting DISTINCT + FILTER plans are not merged into a single aggregate and that results match vanilla Spark (no fallback).

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File	Description
gluten-ut/test/src/test/scala/org/apache/gluten/execution/MergeTwoPhasesHashBaseAggregateSuite.scala	Adds boundary tests for DISTINCT + FILTER ensuring no merge occurs and results match vanilla Spark.
gluten-substrait/src/main/scala/org/apache/gluten/extension/columnar/MergeTwoPhasesHashBaseAggregate.scala	Refines explanatory comment about how FILTER restoration is matched (positional alignment).

Suppressed comments (1)

gluten-ut/test/src/test/scala/org/apache/gluten/execution/MergeTwoPhasesHashBaseAggregateSuite.scala:149

• compareResultsAgainstVanillaSpark already runs the query before calling customCheck; the extra df.collect() here executes the SQL an additional time. The executed plan can be inspected without re-running the query.

        df => {
          df.collect()
          val plans = collect(df.queryExecution.executedPlan) {
            case agg: HashAggregateExecBaseTransformer => agg
          }

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.