Skip to content

fix(optimizer): Stop dropping correlated subquery filters - #1802

Closed
PingLiuPing wants to merge 2 commits into
facebookincubator:mainfrom
PingLiuPing:lp_fix_tpch_q2
Closed

fix(optimizer): Stop dropping correlated subquery filters#1802
PingLiuPing wants to merge 2 commits into
facebookincubator:mainfrom
PingLiuPing:lp_fix_tpch_q2

Conversation

@PingLiuPing

Copy link
Copy Markdown
Contributor

A correlated scalar subquery predicate could be silently dropped by the v1 optimizer.

For example, this query (simplified from tpch Q2) must keep only the cheapest ps row for each key:

SELECT ps.k, ps.cost
FROM (VALUES (1), (2)) AS p(k),
     (VALUES (1, 100.0), (1, 200.0), (2, 50.0), (2, 30.0)) AS ps(k, cost)
WHERE p.k = ps.k
  AND ps.cost = (
      SELECT min(ps2.cost)
      FROM (VALUES (1, 100.0), (1, 200.0), (2, 50.0), (2, 30.0)) AS ps2(k, cost)
      WHERE ps2.k = p.k)
ORDER BY ps.k

Before this change, v1 returned all 4 ps rows. The ps.cost = (...) predicate was lost, so only p.k = ps.k remained effective. But the correct result should be:

k | cost
1 | 100
2 |  30

Root cause

Decorrelation rewrites the scalar subquery into an aggregate relation, conceptually:

MINS(k, min_cost)

and attaches it to the correlated outer table with a LEFT join:

p LEFT JOIN MINS ON p.k = MINS.k

After decorrelation, the original comparison is an ordinary filter evaluated after both inputs are available:

Filter ps.cost = MINS.min_cost
└─ p ⋈ ps ON p.k = ps.k
   LEFT JOIN MINS ON p.k = MINS.k

distributeConjuncts incorrectly recognized this filter as a reorderable join equality simply because it references two tables. It promoted ps.cost = MINS.min_cost into a join edge between ps and MINS.

When join search places p and ps first, it later adds MINS through the required non-commutative LEFT join. Extra inner edges are merged only when the candidate being added is itself an inner join. In this join order, the candidate that adds MINS is the LEFT join, so the promoted ps.cost = MINS.min_cost edge is not incorporated. The original conjunct had already been removed, so no filter remains to evaluate it.

Fix

Do not promote a two-table equality into a join edge when either endpoint is the null-producing side of an outer join. The conjunct remains a filter above the outer join, where both ps.cost and MINS.min_cost are available and the comparison is correctly evaluated.

Only v1 is affected. v2 uses a separate decorrelation pipeline and returns the correct result before and after this change. The new SQL regression runs under both versions to guard against a future v2 regression.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Aug 28, 2026
@PingLiuPing

Copy link
Copy Markdown
Contributor Author

@mbasmanova Could you please take a look when you get a chance? Thank you!

@mbasmanova

Copy link
Copy Markdown
Contributor

@PingLiuPing Thank you for the fix. We are actively migrating to v2 and I hope to "delete" v1 within a month or so.

@mbasmanova mbasmanova left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the detailed root cause.

Note that we are actively migrating to v2, which already returns the correct
result for this query.

That said, the bug looks like it is elsewhere. distributeConjuncts() promotes
the equality into a join edge and erases the conjunct, which is only sound if the
edge is always applied. It is not: in nextJoins(),

candidates.emplace_back(join, joined, fanout);
if (join->isInner()) {
  addExtraEdges(state, candidates.back());
}

addExtraEdges() runs only when the candidate join is inner
(Optimization.cpp:973), so when MINS is brought in by the LEFT join the extra
ps.cost = MINS.min_cost edge is never picked up, and the conjunct that would
have evaluated it is already gone. Your guard stops one way of producing such an
edge; the path that discards it is untouched, and anything else that puts an
inner edge on a table placed by a non-commutative join drops predicates the same
way.

@PingLiuPing

Copy link
Copy Markdown
Contributor Author

@mbasmanova Thank you for the review. I will close it and move to v2 in my test.

@PingLiuPing PingLiuPing closed this Sep 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants