Skip to content

Keep no-match rows when pulling up a correlated aggregate subquery (bugfix to main) - #1933

Open
Alena0704 wants to merge 1 commit into
apache:mainfrom
Alena0704:main-correlated-agg-subquery-left-join
Open

Keep no-match rows when pulling up a correlated aggregate subquery (bugfix to main)#1933
Alena0704 wants to merge 1 commit into
apache:mainfrom
Alena0704:main-correlated-agg-subquery-left-join

Conversation

@Alena0704

@Alena0704 Alena0704 commented Aug 27, 2026

Copy link
Copy Markdown
Contributor

Keep no-match rows when pulling up a correlated aggregate subquery

With the Postgres planner (optimizer=off or an ORCA fallback), a correlated scalar subquery with an aggregate is pulled up into an INNER join with a grouped subquery (convert_EXPR_to_join), which drops outer rows that have no match. The original subquery keeps them: it computes the aggregate over empty input, so e.g. COUNT yields 0 there:

select ... from t1
	where t1.a > (select count(*) from t2 where t2.a = t1.d);

A row with no match in t2 must be compared as "t1.a > 0" and can pass, but the INNER join dropped it.

To fix this, pull the subquery up into a LEFT join, so no-match rows survive as null-extended rows, and rewrite the comparison to return the same value the subquery would:

outer OP CASE WHEN match_flag THEN expr ELSE empty_input_default END

match_flag is a constant TRUE column added to the subquery. For a matched row the CASE returns the real expression; for a null-extended row the flag is NULL and the CASE returns the empty-input default (0 for COUNT, NULL for other aggregates).

The comparison runs above the LEFT join as a filter, not as the join condition: as a join qual it would null-extend matched rows that fail it, and the default would let them back in.

The LEFT join is not always needed. If a no-match row cannot pass the comparison anyway -- e.g. "1 = (select count(*) ...)" turns into "1 = 0" for it -- dropping it is fine and the INNER join is kept as before. This is detected by substituting the empty-input default into the comparison and constant-folding it. Ordinary sum/avg/min/max comparisons fall into this group: their empty-input value is NULL, and a comparison with NULL does not pass, so those plans do not change.

If the comparison cannot be placed above the join (the sublink is in an outer join's ON clause) or the subquery's targetlist is correlated, the pull-up bails out and the sublink runs as a SubPlan, as before.

Adapted from open-gpdb open-gpdb/gpdb#397 and Greengage GreengageDB/greengage#546.
Co-Authored-By: excaliiibur excaliiibur@foxmail.com

The same #1928 but rebased up to Postgres v.16.
The bug reproduction

postgres=# drop table t_out;
DROP TABLE
postgres=# drop table t_in;
DROP TABLE
postgres=# create table t_out as select 1 as a distributed by (a);                                          
SELECT 1
postgres=# create table t_in  as select 2 as a distributed by (a);                                          
SELECT 1
postgres=# set optimizer=0;                                                                                 
SET
postgres=# select * from t_out where a > (select count(*) from t_in where t_in.a = t_out.a); --incorrect result
 a 
---
(0 rows)

postgres=# set optimizer=1;
SET
postgres=# select * from t_outwhere a > (select count(*) from t_in where t_in.a = t_out.a);
 a 
---
 1
(1 row)

Type of Change

  • Bug fix (non-breaking change)
  • New feature (non-breaking change)
  • Breaking change (fix or feature with breaking changes)
  • Documentation update

Breaking Changes

Test Plan

  • Unit tests added/updated
  • Integration tests added/updated
  • Passed make installcheck
  • Passed make -C src/test installcheck-cbdb-parallel

Impact

Performance:

User-facing changes:

Dependencies:

Checklist

Additional Context

CI Skip Instructions


@Alena0704 Alena0704 changed the title Keep no-match rows when pulling up a correlated aggregate subquery Keep no-match rows when pulling up a correlated aggregate subquery (bugfix to main) Aug 27, 2026
@Alena0704
Alena0704 force-pushed the main-correlated-agg-subquery-left-join branch from b8ffb75 to 844ed55 Compare August 27, 2026 11:54
@leborchuk
leborchuk requested a review from yjhjstz August 27, 2026 16:01
-> Seq Scan on smallt (cost=0.00..1.33 rows=33 width=4)
Optimizer: Postgres query optimizer
(12 rows)
(11 rows)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

test sql: select * from t1 where t1.a > (select count(*) + count(*) over () from t2 where t2.a = t1.b);

got ERROR: WindowFunc found in non-WindowAgg plan node (execExpr.c:1188).

@Alena0704 Alena0704 Aug 28, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I'm looking at this and I'll check everything again just in case. Thank you for feedback.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

About the WindowAgg bug: without the pull-up the subquery has a plain aggregate, so it returns exactly one row and the window runs over that single row; the pulled-up subquery is grouped by the correlation columns, so the same window runs over all groups at once.

I found no safe way to keep the pull-up here, so safe_to_convert_EXPR() now bails out on subselect->hasWindowFuncs and the sublink stays a SubPlan.

I have found another bug - division by zero at plan time. Unlike the one above, this one is introduced by this patch: on main the query below runs fine and returns no rows. Reproduction:

    set optimizer = off;

    create table e1(a int, d int);
    create table k1(a int);  insert into k1 values (1), (2);

    select * from e1 where e1.a > (select 1/count(*) from k1 where k1.a = e1.d);

e1 is empty, so the correct answer is no rows and no error. Before the fix:

ERROR: division by zero

It failed in EXPLAIN too — nothing was ever executed. For a row with no match the patch replaces the aggregate with the value it returns over empty input — 0 for count(), NULL for the others — and puts that expression into the outer WHERE. So 1/count() becomes 1/0.
Nothing variable is left in it, and the planner evaluates it right away instead of at run time. Only the 0 can do this: an expression built on NULL is simplified to NULL without evaluating anything.

replace_agg_with_empty_default_mutator() now records whether it invented a non-NULL value (only COUNT does), and convert_EXPR_to_join() refuses the pull-up when it did and the default is not a plain Const. The sublink stays a SubPlan:

     Gather Motion 3:1  (slice1; segments: 3)
       ->  Seq Scan on e1
             Filter: (a > (SubPlan 1))
             SubPlan 1
               ->  Aggregate
                     ->  Result
                           Filter: (k1.a = e1.d)
                           ->  Materialize
                                 ->  Broadcast Motion 3:3  (slice2; segments: 3)
                                       ->  Seq Scan on k1

     a | d
    ---+---
    (0 rows)

@Alena0704
Alena0704 force-pushed the main-correlated-agg-subquery-left-join branch 3 times, most recently from 69b42d8 to 6ef3f63 Compare September 4, 2026 12:59
With the Postgres planner (optimizer=off or an ORCA fallback), a
correlated scalar subquery with an aggregate is pulled up into an INNER
join with a grouped subquery (convert_EXPR_to_join), which drops outer
rows that have no match. The original subquery keeps them: it computes
the aggregate over empty input, so e.g. COUNT yields 0 there:

	select ... from t1
		where t1.a > (select count(*) from t2 where t2.a = t1.d);

A row with no match in t2 must be compared as "t1.a > 0" and can pass,
but the INNER join dropped it.

To fix this, pull the subquery up into a LEFT join, so no-match rows
survive as null-extended rows, and rewrite the comparison to return the
same value the subquery would:

    outer OP CASE WHEN match_flag THEN expr ELSE empty_input_default END

match_flag is a constant TRUE column added to the subquery. For a matched
row the CASE returns the real expression; for a null-extended row the flag
is NULL and the CASE returns the empty-input default (0 for COUNT, NULL for
other aggregates).

The comparison runs above the LEFT join as a filter, not as the join
condition: as a join qual it would null-extend matched rows that fail it,
and the default would let them back in.

The LEFT join is not always needed. If a no-match row cannot pass the
comparison anyway -- e.g. "1 = (select count(*) ...)" turns into "1 = 0"
for it -- dropping it is fine and the INNER join is kept as before. This is
detected by substituting the empty-input default into the comparison and
constant-folding it. Ordinary sum/avg/min/max comparisons fall into this
group: their empty-input value is NULL, and a comparison with NULL does not
pass, so those plans do not change.

If the comparison cannot be placed above the join (the sublink is in an
outer join's ON clause) or the subquery's targetlist is correlated, the
pull-up bails out and the sublink runs as a SubPlan, as before.

Adapted from open-gpdb open-gpdb/gpdb#397 and Greengage GreengageDB/greengage#546.
Co-Authored-By: excaliiibur excaliiibur@foxmail.com
@Alena0704
Alena0704 force-pushed the main-correlated-agg-subquery-left-join branch from 6ef3f63 to 04d815b Compare September 4, 2026 13:00

@leborchuk leborchuk 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.

LGTM

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants