fix: preserve scalar aggregate cardinality in EXISTS unnest - #3683
Open
vishnujayvel wants to merge 1 commit into
Open
fix: preserve scalar aggregate cardinality in EXISTS unnest#3683vishnujayvel wants to merge 1 commit into
vishnujayvel wants to merge 1 commit into
Conversation
Correlated EXISTS whose body is a scalar aggregate (implicit grouping, no GROUP BY keys) must be TRUE even when the match set is empty: aggregates emit exactly one row over empty input. simplifyPartialJoinParents stripped *plan.GroupBy unconditionally, turning empty-input cardinality from 1 to 0 and dropping every outer row (NOT EXISTS kept every row). Refuse unnest when GroupByExprs is empty so ExistsSubquery evaluates the aggregate correctly. Explicit GROUP BY keys still strip as before. Add enginetest coverage for SUM/COUNT(*) EXISTS and NOT EXISTS, a bare- column control, explicit GROUP BY empty-match, and uncorrelated guards.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Correlated
EXISTSwhose body is a scalar aggregate (implicit grouping — noGROUP BYkeys) should be TRUE for every outer row, including rows with an empty match set. SQL aggregates emit exactly one row over empty input (SUM→NULL,COUNT(*)→0), so existence never fails for that shape.On current
main, those queries return no outer rows:The same bug also hit uncorrelated scalar-aggregate
EXISTSin FILTER position when the match set is empty (e.g.WHERE EXISTS (SELECT SUM(u.x) FROM u WHERE u.x = 999)orEXISTS (SELECT SUM(e.y) FROM e)on an empty table). Those go through thelen(joinFilters)==0Limit-CrossJoin path after the sameGroupBystrip and were wrong onmain; this patch fixes them. Projection-position uncorrelated forms (SELECT EXISTS(...)) were already correct and are unchanged.Root cause
simplifyPartialJoinParentsinsql/analyzer/unnest_exists_subqueries.gostrips*plan.GroupBywhen building the right side of a decorrelated EXISTS semi-join. Forlen(GroupByExprs) == 0(scalar/implicit grouping) that changes empty-input cardinality from 1 → 0, so the semi-join sees no right-side rows and drops every outer row.Fix
When the
GroupBygrouping-key list is empty, refuse unnest (return nil, false) so theExistsSubqueryevaluation path keeps the aggregate and preserves the one empty-input row. ExplicitGROUP BYcolumns still strip as before (empty groups produce no rows, so bare-match EXISTS semantics are correct).This is the same strip list that has bitten related shapes (see dolthub/dolt#10493 for Limit, and historical empty-input existence issues around dolthub/dolt#6898). Window stripping is intentionally out of scope here (row-preserving; handled separately).
Performance
The fix converts SemiJoin unnest into one
ExistsSubqueryexecution per outer row for scalar-aggregate bodies. Correctness outranks the wrong fast plan: an unnested semi-join that strips the scalar aggregate is not a free win when it changes empty-input cardinality.A strictly faster follow-up would constant-fold
EXISTS(scalar aggregate with no HAVING/LIMIT)toTRUE(andNOT EXISTS(...)toFALSE) without executing the subquery at all. That was deliberately not done here — minimal refusal keeps the change small, reviewable, and confined to the strip that causes the bug, rather than adding a new optimizer rewrite with its own edge cases.Related / rebase notes
EXISTS (... LIMIT 0)remains wrong before and after this patch; that is partial overlap, not a full fix for #10493.join_queries.goaftervar JoinScriptTestsandunnest_exists_subqueries.goaftercase plan.Having). Textual conflict is certain on rebase; merged semantics are compatible (Window strips then scalar GroupBy refuses).EXISTSinside a scalar-aggregate body is no longer unnested (perf-only side effect). Results were verified correct.Test plan
go test ./sql/analyzer/ -count=1go test ./enginetest/ -run TestJoinQueries -count=1go test ./enginetest/... -count=1JoinScriptTestsscript covering:EXISTS (SELECT SUM(...))empty-match → all outer rowsNOT EXISTS (SELECT SUM(...))→ noneEXISTS (SELECT COUNT(*))sameEXISTS (SELECT SUM(...) GROUP BY col)empty-match → match-onlyWHERE u.x = 999) → all / nonee→ all outer rows