Keep no-match rows when pulling up a correlated aggregate subquery - #546
Keep no-match rows when pulling up a correlated aggregate subquery#546Alena0704 wants to merge 1 commit into
Conversation
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/gpdb#397.
Co-Authored-By: excaliiibur <excaliiibur@foxmail.com>
|
An issue without this commit is: As you see the Postgres planner returns incorrect result. |
|
Thanks for contributing! There is error at the test: diff -I HINT: -I CONTEXT: -I GP_IGNORE: -U3 /home/gpadmin/gpdb_src/src/test/regress/expected/subselect_gp.out /home/gpadmin/gpdb_src/src/test/regress/results/subselect_gp.out
--- /home/gpadmin/gpdb_src/src/test/regress/expected/subselect_gp.out 2026-07-30 06:50:37.480701798 +0000
+++ /home/gpadmin/gpdb_src/src/test/regress/results/subselect_gp.out 2026-07-30 06:50:37.583698198 +0000
@@ -953,22 +957,22 @@
on t1.a > (select count(*) from t_in t2 where t2.a = t1.a);
QUERY PLAN
___________
- Aggregate
+ Finalize Aggregate
-> Gather Motion 3:1
- -> Aggregate
+ -> Partial Aggregate
-> Nested Loop Left Join
- Join Filter: (t1.a > (SubPlan 1))
+ Join Filter: (t1.a > ((SubPlan 1)))
-> Seq Scan on t_out t1
+ SubPlan 1
+ -> Aggregate
+ -> Result
+ Filter: (t2.a = t1.a)
+ -> Materialize
+ -> Broadcast Motion 3:3
+ -> Seq Scan on t_in t2
-> Materialize
-> Broadcast Motion 3:3
-> Seq Scan on t_out t3
- SubPlan 1
- -> Aggregate
- -> Result
- Filter: (t2.a = t1.a)
- -> Materialize
- -> Broadcast Motion 3:3
- -> Seq Scan on t_in t2
GP_IGNORE:(17 rows)
select count(*) from t_out t1 left join t_out t3diff -I HINT: -I CONTEXT: -I GP_IGNORE: -U3 /home/gpadmin/gpdb_src/src/test/regress/expected/subselect_gp_optimizer.out /home/gpadmin/gpdb_src/src/test/regress/results/subselect_gp.out
--- /home/gpadmin/gpdb_src/src/test/regress/expected/subselect_gp_optimizer.out 2026-07-30 06:50:57.060763829 +0000
+++ /home/gpadmin/gpdb_src/src/test/regress/results/subselect_gp.out 2026-07-30 06:50:57.202761658 +0000
@@ -924,22 +928,22 @@
on t1.a > (select count(*) from t_in t2 where t2.a = t1.a);
QUERY PLAN
___________
- Aggregate
+ Finalize Aggregate
-> Gather Motion 3:1
- -> Aggregate
+ -> Partial Aggregate
-> Nested Loop Left Join
- Join Filter: (t1.a > (SubPlan 1))
+ Join Filter: (t1.a > ((SubPlan 1)))
-> Seq Scan on t_out t1
+ SubPlan 1
+ -> Aggregate
+ -> Result
+ Filter: (t2.a = t1.a)
+ -> Materialize
+ -> Broadcast Motion 3:3
+ -> Seq Scan on t_in t2
-> Materialize
-> Broadcast Motion 3:3
-> Seq Scan on t_out t3
- SubPlan 1
- -> Aggregate
- -> Result
- Filter: (t2.a = t1.a)
- -> Materialize
- -> Broadcast Motion 3:3
- -> Seq Scan on t_in t2
GP_IGNORE:(17 rows)
select count(*) from t_out t1 left join t_out t3 |
Thanks for the review! The original fix was written for open-gpdb, where the planner already pulls this subquery into a join, so it was done as a LEFT join + CASE. When porting to Greengage I only verified that the wrong results were gone, not the ORCA plan - so this question didn't come up on my side. I've found Greengage's commit 2ef1914 (#1658), which disables ORCA's decorrelation of such COUNT subqueries and keeps them as a SubPlan for the same reason; it took me a while to understand. Rethinking it, I now think keeping the correlated SubPlan is the safer and more consistent choice here, so I'm reworking the planner path to keep the SubPlan too. Thanks! |
|
I actually think your current solution is better than the one I made in 2ef1914. In case of ORCA, the decorrelation logic was broken in other ways too, so as a temporary solution we decided to disable it for this case. However, decorrelation is useful for performance, and while your solution is not complete, it does allow decorrelation in more cases than completely disabling it. (For a more complete solution search "Magic Decorrelation", for example.) I wasn't able to find queries where the current patch gives incorrect results, so I think it is logically correct. Are there other reasons to change it, other than consistency with ORCA? I think it would be better to port this solution to ORCA in the future instead. |
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:
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:
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/gpdb#397.
Co-Authored-By: excaliiibur excaliiibur@foxmail.com