Skip to content

branch-4.1:[fix](subquery): avoid join amplification in WinMagic rewrite rule - #67288

Open
starocean999 wants to merge 2 commits into
apache:branch-4.1from
starocean999:b41_63763
Open

branch-4.1:[fix](subquery): avoid join amplification in WinMagic rewrite rule#67288
starocean999 wants to merge 2 commits into
apache:branch-4.1from
starocean999:b41_63763

Conversation

@starocean999

@starocean999 starocean999 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

pick #63763

Related PR: #xxx

Problem Summary:

Release note

None

Check List (For Author)

  • Test

    • Regression test
    • Unit Test
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
      • This is a refactor/code format and no logic has been changed.
      • Previous test can cover this change.
      • No code files have been changed.
      • Other reason

  • Behavior changed:

    • No.
    • Yes.

  • Does this need documentation?

    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

@starocean999
[fix](subquery): avoid join amplification in WinMagic rewrite rule (a…
8d6c5d1 
…pache#63763)

Problem Summary:

The `AggScalarSubQueryToWindowFunction` rewrite rule (WinMagic) rewrites
a correlated scalar subquery like:

```sql
SELECT d.did, f.id, f.k, f.v
FROM fact f, dim d
WHERE f.k = d.k
  AND f.v * 2 > (
    SELECT SUM(f2.v)
    FROM fact f2
    WHERE f2.k = d.k
  )
```

into a window-function plan:

```
Filter(f.v * 2 > SUM(v) OVER (PARTITION BY d.k))
  Window(SUM(v) OVER (PARTITION BY d.k))
    CrossJoin(Scan(fact f), Scan(dim d))
```

This rewrite had three correctness issues:

---

**Bug 1 — Join amplification when the correlated key is not unique**

```sql
-- dim has DUPLICATE KEY(did), so k is NOT unique and can be NULL
CREATE TABLE dim (did INT, k INT, tag INT)
ENGINE=OLAP DUPLICATE KEY(did) ...;

-- data: two rows with the same k=1
INSERT INTO dim VALUES (10, 1, 1), (11, 1, 1);

-- fact and dim are joined on f.k = d.k
SELECT d.did, f.id, f.k, f.v
FROM fact f, dim d
WHERE f.k = d.k
  AND f.v * 2 > (
    SELECT SUM(f2.v) FROM fact f2 WHERE f2.k = d.k
  );
```

Before the fix, the rule **did not check** whether the correlated
outer-only table's key (`d.k`) is unique. Since `dim` is a DUPLICATE KEY
table, `k=1` has two rows. After the CrossJoin, each fact row with `k=1`
joins with both dim rows, producing 2× more output rows than the
original correlated scalar subquery (which is evaluated per outer row).
The window function then sees this inflated row set, producing wrong
aggregate values.

**Fix:** Added `checkUniqueCorrelatedTable()` which verifies the
correlated outer-only table's key is unique & non-null via
`DataTrait.isUniqueAndNotNull()`. This covers OLAP UNIQUE_KEYS metadata,
explicit `UNIQUE` constraints, and rejects nullable keys. Queries
against non-unique-keyed tables are left as original scalar subqueries.

---

**Bug 2 — Inner filter split by PushDownFilterThroughProject causes
rewrite to fail**

```sql
-- inner subquery has both a correlated predicate and a non-correlated predicate
SELECT d.did, f.id, f.k, f.v
FROM fact f, dim_unique d
WHERE f.k = d.k
  AND f.v < 10
  AND f.v * 2 > (
    SELECT SUM(f2.v)
    FROM fact f2
    WHERE f2.k = d.k    -- correlated, cannot push through project
      AND f2.v < 10     -- non-correlated, can push through project
  );
```

The optimizer's `PushDownFilterThroughProject` splits the inner `WHERE
f2.k = d.k AND f2.v < 10` into two separate `LogicalFilter` nodes:
- `Filter(k = d.k)` stays above the project (references correlated slot
`d.k`, not in project output)
- `Filter(v < 10)` is pushed below the project (only references `v`,
which is in the project output)

The old `checkFilter()` required **exactly one** inner `LogicalFilter`
and returned false when seeing two, causing the rule to miss this valid
rewrite.

**Fix:** `checkFilter()` now collects conjuncts from **all** inner
`LogicalFilter` nodes rather than requiring exactly one.

---

**Bug 3 — Matched inner-filter predicates incorrectly placed above the
window**

```sql
-- f.v < 10 appears both in the outer WHERE and the inner WHERE
SELECT d.did, f.id, f.k, f.v
FROM fact f, dim_unique d
WHERE f.k = d.k
  AND f.v < 10
  AND f.v * 2 > (
    SELECT SUM(f2.v)
    FROM fact f2
    WHERE f2.k = d.k
      AND f2.v < 10
  );
```

The predicate `f.v < 10` is **semantically part of the inner aggregate's
filter** — the original scalar subquery only sums `f2.v` where `v < 10`.
However, the old rewrite placed this conjunct **above** the window:

```
-- Wrong: f.v < 10 above the window lets the window see ALL rows
Filter(f.v < 10 AND f.v * 2 > SUM(v) OVER (...))
  Window(SUM(v) OVER (PARTITION BY d.k))   -- aggregates all rows, not just v<10
    CrossJoin(Scan(fact f), Scan(dim_unique d))
```

This means the window function computes `SUM(v)` over **all** fact rows
per key (e.g., `v=5+7+6+4+10+6+8`), instead of only rows where `v < 10`
(e.g., `v=5+7+6+4+6+8`). The comparison `f.v * 2 > sum` then uses the
wrong aggregate, potentially returning incorrect rows.

**Fix:** `checkFilter()` now tracks which outer conjuncts were matched
against inner subquery filter conjuncts. `rewrite()` places these
matched conjuncts **below** the window, preserving the inner aggregate's
filter semantic:

```
-- Correct: f.v < 10 below the window restricts the window's input
Filter(f.v * 2 > SUM(v) OVER (...))
  Window(SUM(v) OVER (PARTITION BY d.k))
    Filter(f.v < 10)     -- restricts rows seen by the window
      CrossJoin(Scan(fact f), Scan(dim_unique d))
```

---

- `checkUniqueCorrelatedTable()` — validates correlated key uniqueness
via `DataTrait.isUniqueAndNotNull()`
- Fix `checkFilter()` — collect conjuncts from ALL inner filters, track
matched conjuncts
- Enhance `rewrite()` — classify conjuncts: matched inner-filter → below
window, shared-table → above, volatile → above, outer-only → below
- Add `stripOuterFilters()` and `ensureProjectOutput()` helpers
- 13 unit tests + 6 regression cases with result and plan-shape
verification

Scalar correlated subquery to window function rewrite is now correct:
non-unique keys are rejected, split inner filters are handled, and
inner-filter predicates are placed below the window.
@starocean999
fix case
19fc0a7
@starocean999
starocean999 requested a review from yiguolei as a code owner August 28, 2026 09:01
@hello-stephen

hello-stephen commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@starocean999

starocean999 commented Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen

hello-stephen commented Aug 28, 2026

Copy link
Copy Markdown
Contributor

FE UT Coverage Report

Increment line coverage 81.57% (177/217) 🎉
Increment coverage report
Complete coverage report

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

Reviewers

@yiguolei yiguolei Awaiting requested review from yiguolei yiguolei is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@starocean999 @hello-stephen