Skip to content

Commit e640835

Browse files
Alena0704excaliiibur
andcommitted
Fix COUNT bug when pulling up scalar subqueries (fallback planner)
A correlated subquery like select ... from t1 where t1.a > (select count(*) from t2 where t2.a = t1.d) must return COUNT = 0 (not NULL) for outer rows that have no match. The fallback planner pulled such subqueries into an INNER join, which dropped every no-match row -- the classic COUNT bug. PR #376 (df87bf0) turned the join into a LEFT join and wrapped the aggregate in COALESCE(agg, default) with COUNT -> 0. But the comparison stayed on the join itself (a "Join Filter"), so a matched row that failed the comparison was treated as unmatched, null-extended, and let back in by the COALESCE default -- e.g. count(*) returned 100 instead of 99. Fix: when the pulled-up join is LEFT, evaluate the comparison ABOVE the join (wrap it in a FromExpr) instead of as the join condition. The INNER path is unchanged. Tests in subselect_gp cover COUNT and mixed COUNT+other aggregate expressions on both the optimizer=off and ORCA->planner fallback paths. Co-Authored-By: excaliiibur <excaliiibur@foxmail.com>
1 parent 5b2fd56 commit e640835

5 files changed

Lines changed: 395 additions & 1 deletion

File tree

‎src/backend/cdb/cdbsubselect.c‎

Lines changed: 104 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
#include "access/htup_details.h"
1818
#include "access/skey.h"
1919
#include "catalog/pg_operator.h"
20+
#include "catalog/pg_proc.h"
2021
#include "catalog/pg_type.h"
2122
#include "nodes/makefuncs.h"
2223
#include "optimizer/clauses.h"
@@ -43,6 +44,10 @@ static JoinExpr *make_join_expr(Node *larg, int r_rtindex, int join_type);
4344
static Node *make_lasj_quals(PlannerInfo *root, SubLink *sublink, int subquery_indx);
4445

4546
static Node *add_null_match_clause(Node *clause);
47+
static bool contains_count_agg_expr(Node *expr);
48+
static Expr *build_count_coalesce_expr(Var *aggVar, Expr *defaultExpr);
49+
static Expr *build_count_default_expr(Node *expr);
50+
static Node *replace_agg_with_empty_default_mutator(Node *node, void *context);
4651

4752
typedef struct NonNullableVarsContext
4853
{
@@ -574,6 +579,7 @@ convert_EXPR_to_join(PlannerInfo *root, OpExpr *opexp)
574579
* targetlist.
575580
*/
576581
TargetEntry *origSubqueryTLE = (TargetEntry *) list_nth(subselect->targetList, 0);
582+
bool has_count_expr = contains_count_agg_expr((Node *) origSubqueryTLE->expr);
577583

578584
List *subselectTargetList = (List *) copyObject(ctx1.targetList);
579585

@@ -624,14 +630,111 @@ convert_EXPR_to_join(PlannerInfo *root, OpExpr *opexp)
624630
exprCollation((Node *) subselectAggTLE->expr),
625631
0);
626632

627-
list_nth_replace(opexp->args, 1, aggVar);
633+
if (has_count_expr)
634+
{
635+
/*
636+
* Expressions containing COUNT over no matching rows must evaluate
637+
* with COUNT=0 instead of NULL after pull-up. For non-COUNT
638+
* aggregates in the same expression, use NULL empty-input defaults.
639+
* Preserve semantics with LEFT JOIN + COALESCE(agg_expr, default_expr).
640+
*/
641+
Expr *defaultExpr = build_count_default_expr((Node *) origSubqueryTLE->expr);
642+
643+
join_expr->jointype = JOIN_LEFT;
644+
list_nth_replace(opexp->args, 1, build_count_coalesce_expr(aggVar, defaultExpr));
645+
}
646+
else
647+
{
648+
list_nth_replace(opexp->args, 1, aggVar);
649+
}
628650

629651
return join_expr;
630652
}
631653

632654
return NULL;
633655
}
634656

657+
static bool
658+
contains_count_agg_expr(Node *expr)
659+
{
660+
if (expr == NULL)
661+
return false;
662+
663+
if (IsA(expr, Aggref))
664+
{
665+
Aggref *aggref = (Aggref *) expr;
666+
return aggref->aggfnoid == COUNT_ANY_OID ||
667+
aggref->aggfnoid == COUNT_STAR_OID;
668+
}
669+
670+
return expression_tree_walker(expr,
671+
contains_count_agg_expr,
672+
NULL);
673+
}
674+
675+
static Expr *
676+
build_count_coalesce_expr(Var *aggVar, Expr *defaultExpr)
677+
{
678+
CoalesceExpr *coalesce;
679+
680+
Assert(aggVar != NULL);
681+
Assert(defaultExpr != NULL);
682+
683+
coalesce = makeNode(CoalesceExpr);
684+
coalesce->coalescetype = exprType((Node *) aggVar);
685+
coalesce->coalescecollid = exprCollation((Node *) aggVar);
686+
coalesce->args = list_make2(aggVar, defaultExpr);
687+
coalesce->location = -1;
688+
689+
return (Expr *) coalesce;
690+
}
691+
692+
static Expr *
693+
build_count_default_expr(Node *expr)
694+
{
695+
Node *rewritten;
696+
697+
rewritten = replace_agg_with_empty_default_mutator(copyObject(expr), NULL);
698+
return (Expr *) rewritten;
699+
}
700+
701+
static Node *
702+
replace_agg_with_empty_default_mutator(Node *node, void *context)
703+
{
704+
Aggref *aggref;
705+
Oid default_type;
706+
Oid default_collation;
707+
int16 typlen;
708+
bool typbyval;
709+
710+
if (node == NULL)
711+
return NULL;
712+
713+
if (IsA(node, Aggref))
714+
{
715+
aggref = (Aggref *) node;
716+
if (aggref->aggfnoid == COUNT_ANY_OID ||
717+
aggref->aggfnoid == COUNT_STAR_OID)
718+
{
719+
default_type = INT8OID;
720+
default_collation = InvalidOid;
721+
}
722+
else
723+
{
724+
default_type = aggref->aggtype;
725+
default_collation = exprCollation((Node *) aggref);
726+
}
727+
728+
get_typlenbyval(default_type, &typlen, &typbyval);
729+
return (Node *) makeConst(default_type, -1, default_collation, typlen,
730+
(default_type == INT8OID) ? Int64GetDatum(0) : (Datum) 0,
731+
(default_type != INT8OID), typbyval);
732+
}
733+
734+
return expression_tree_mutator(node, replace_agg_with_empty_default_mutator,
735+
context);
736+
}
737+
635738
/* NOTIN subquery transformation -start */
636739

637740
/* check if NOT IN conversion to antijoin is possible */

‎src/backend/optimizer/prep/prepjointree.c‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -691,6 +691,26 @@ pull_up_sublinks_qual_recurse(PlannerInfo *root, Node *node,
691691
{
692692
/* Yes, insert the new join node into the join tree */
693693
j->larg = *jtlink1;
694+
695+
if (j->jointype == JOIN_LEFT)
696+
{
697+
/*
698+
* COUNT-preserving pull-up (see convert_EXPR_to_join):
699+
* the comparison in opexp must be evaluated ABOVE the
700+
* LEFT JOIN, never as its join qual. If it stayed as a
701+
* qual of the join's larg it would be pulled up into the
702+
* outer join's condition; then a row that matches the
703+
* correlation but fails the comparison would be treated
704+
* as unmatched, get null-extended, and pass again through
705+
* the COALESCE empty-input default -- resurrecting rows
706+
* that should have been filtered out. Wrap the join in a
707+
* FromExpr that carries opexp as a post-join qual.
708+
*/
709+
*jtlink1 = (Node *) makeFromExpr(list_make1(j), node);
710+
return NULL;
711+
}
712+
713+
/* Inner-join case: opexp stays as an ordinary qual. */
694714
*jtlink1 = (Node *) j;
695715
}
696716
return node;

‎src/test/regress/expected/subselect_gp.out‎

Lines changed: 114 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -499,6 +499,120 @@ select count(*) from csq_t1 t1 where a > ( select avg(a)::int from csq_t1 t2 whe
499499
49
500500
(1 row)
501501

502+
-- COUNT correlated scalar subquery should keep no-match rows (COUNT = 0)
503+
-- Scenario 1: optimizer=off (always PostgreSQL planner path)
504+
set optimizer=off;
505+
SET
506+
select count(*) from csq_t1 t1 where a > (select count(*) from csq_t1 t2 where t2.a = t1.d);
507+
count
508+
-------
509+
99
510+
(1 row)
511+
512+
select min(a), max(a) from csq_t1 t1 where (select count(*) from csq_t1 t2 where t2.a = t1.d) = 0;
513+
min | max
514+
-----+-----
515+
100 | 100
516+
(1 row)
517+
518+
reset optimizer;
519+
RESET
520+
-- Scenario 2: optimizer=on, but query shape forces ORCA fallback to PostgreSQL planner
521+
-- (ordered aggregate disabled in ORCA by default)
522+
set optimizer=on;
523+
SET
524+
set optimizer_enable_orderedagg=off;
525+
SET
526+
explain select string_agg(t1.a::text, ',' order by t1.a)
527+
from csq_t1 t1
528+
where t1.a > (select count(*) from csq_t1 t2 where t2.a = t1.d);
529+
QUERY PLAN
530+
-----------------------------------------------------------------------------------------------------
531+
Aggregate (cost=16.29..16.30 rows=1 width=32)
532+
-> Gather Motion 3:1 (slice2; segments: 3) (cost=7.75..16.04 rows=34 width=4)
533+
-> Hash Left Join (cost=7.75..14.71 rows=12 width=4)
534+
Hash Cond: (t1.d = t2.a)
535+
Filter: (t1.a > COALESCE((count(*)), '0'::bigint))
536+
-> Redistribute Motion 3:3 (slice1; segments: 3) (cost=0.00..6.00 rows=34 width=8)
537+
Hash Key: t1.d
538+
-> Seq Scan on csq_t1 t1 (cost=0.00..4.00 rows=34 width=8)
539+
-> Hash (cost=6.50..6.50 rows=34 width=12)
540+
-> HashAggregate (cost=4.50..5.50 rows=34 width=12)
541+
Group Key: t2.a
542+
-> Seq Scan on csq_t1 t2 (cost=0.00..4.00 rows=34 width=4)
543+
Optimizer: Postgres query optimizer
544+
(13 rows)
545+
546+
select string_agg(t1.a::text, ',' order by t1.a)
547+
from csq_t1 t1
548+
where t1.a > (select count(*) from csq_t1 t2 where t2.a = t1.d);
549+
string_agg
550+
-----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------
551+
2,3,4,5,6,7,8,9,10,11,12,13,14,15,16,17,18,19,20,21,22,23,24,25,26,27,28,29,30,31,32,33,34,35,36,37,38,39,40,41,42,43,44,45,46,47,48,49,50,51,52,53,54,55,56,57,58,59,60,61,62,63,64,65,66,67,68,69,70,71,72,73,74,75,76,77,78,79,80,81,82,83,84,85,86,87,88,89,90,91,92,93,94,95,96,97,98,99,100
552+
(1 row)
553+
554+
-- Non-plain COUNT expression (COUNT(*) + 1) should also preserve no-match semantics
555+
-- Scenario 1: optimizer=off (always PostgreSQL planner path)
556+
set optimizer=off;
557+
SET
558+
select min(a), max(a), count(*) from csq_t1 t1
559+
where (select count(*) + 1 from csq_t1 t2 where t2.a = t1.d) = 1;
560+
min | max | count
561+
-----+-----+-------
562+
100 | 100 | 1
563+
(1 row)
564+
565+
select min(a), max(a), count(*) from csq_t1 t1
566+
where (select (count(*) + 1)::bigint from csq_t1 t2 where t2.a = t1.d) = 1;
567+
min | max | count
568+
-----+-----+-------
569+
100 | 100 | 1
570+
(1 row)
571+
572+
select min(a), max(a), count(*) from csq_t1 t1
573+
where (select case when count(*) > 0 then count(*) + 1 else 1 end
574+
from csq_t1 t2 where t2.a = t1.d) = 1;
575+
min | max | count
576+
-----+-----+-------
577+
100 | 100 | 1
578+
(1 row)
579+
580+
select min(a), max(a), count(*) from csq_t1 t1
581+
where (select abs(count(*) - 1) from csq_t1 t2 where t2.a = t1.d) = 1;
582+
min | max | count
583+
-----+-----+-------
584+
100 | 100 | 1
585+
(1 row)
586+
587+
select min(a), max(a), count(*) from csq_t1 t1
588+
where (select count(*) + coalesce(sum(t2.a), 0) from csq_t1 t2 where t2.a = t1.d) = 0;
589+
min | max | count
590+
-----+-----+-------
591+
100 | 100 | 1
592+
(1 row)
593+
594+
select min(a), max(a), count(*) from csq_t1 t1
595+
where (select coalesce(count(*) + sum(t2.a), -1) from csq_t1 t2 where t2.a = t1.d) = -1;
596+
min | max | count
597+
-----+-----+-------
598+
100 | 100 | 1
599+
(1 row)
600+
601+
reset optimizer;
602+
RESET
603+
-- Scenario 2: optimizer=on, but query shape forces ORCA fallback to PostgreSQL planner
604+
set optimizer=on;
605+
SET
606+
set optimizer_enable_orderedagg=off;
607+
SET
608+
select string_agg(t1.a::text, ',' order by t1.a)
609+
from csq_t1 t1
610+
where (select count(*) + 1 from csq_t1 t2 where t2.a = t1.d) = 1;
611+
string_agg
612+
------------
613+
100
614+
(1 row)
615+
502616
--
503617
-- correlation in a func expr
504618
--

0 commit comments

Comments
 (0)