[subquery] Preserve COUNT semantics in scalar subquery pull-up#376
[subquery] Preserve COUNT semantics in scalar subquery pull-up#376Excaliiiibur wants to merge 2 commits into
Conversation
|
As I see, the case like this (SELECT COUNT(*)+1 FROM ...) is still incorrect. You should add the check that the type of expression here can be OpExpr in the is_plain_count_agg_expr function. Is this case is reproducible for orca? we should mention it in the commit message if it isn't and only for fallback postgres optimizer (just in case). |
…back rewrite Fix scalar-subquery pull-up in cdbsubselect for target expressions that contain COUNT aggregates. In the fallback PostgreSQL optimizer rewrite path, preserve no-match semantics with LEFT JOIN + COALESCE(agg_expr, default_expr). The default expression is derived from the original aggregate expression tree using empty-input defaults: COUNT -> 0, non-COUNT aggregates -> NULL of aggregate result type. Add regression coverage for COUNT-only and mixed COUNT+other aggregate expressions (including CASE/FuncExpr/COALESCE wrappers), and keep explicit fallback-path validation (non-ORCA-specific).
Excaliiiibur
force-pushed
the
bugfix/count-aggr-first-fallback
branch
from
e7efea8 to
df87bf0
Compare
|
Thanks for the feedback. I’ve updated the fix to cover non-plain COUNT expressions as well, not just plain COUNT(*). This now includes cases like Implementation-wise, for COUNT-containing scalar aggregate expressions in the fallback PostgreSQL rewrite path, we preserve no-match semantics via I also added regression coverage for these cases, including fallback-path validation with Could you please take another look? |
Yes, sure) |
|
Hi! I am confused, what commit I should see. This commit Excaliiiibur@df87bf0, right? |
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. Two problems with that alone:
* 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. Fixed here in pull_up_sublinks_qual_recurse(): when the
pulled-up join is LEFT, evaluate the comparison ABOVE the join (wrap it in
a FromExpr) rather than as the join condition.
* It switched to a LEFT join for every COUNT-containing subquery, even where
an INNER join was already correct (e.g. "1 = (select count(*) ...)", whose
no-match value 0 can never satisfy the predicate), churning many unrelated
plans. Narrowed here: only use the LEFT join + COALESCE when a no-match row
could actually survive, i.e. when "outerExpr OP default" does not fold to a
constant FALSE/NULL. Otherwise keep the INNER join unchanged.
The INNER path is otherwise untouched.
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>
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. Two problems with that alone:
* 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. Fixed here in pull_up_sublinks_qual_recurse(): when the
pulled-up join is LEFT, evaluate the comparison ABOVE the join (wrap it in
a FromExpr) rather than as the join condition.
* It switched to a LEFT join for every COUNT-containing subquery, even where
an INNER join was already correct (e.g. "1 = (select count(*) ...)", whose
no-match value 0 can never satisfy the predicate), churning many unrelated
plans. Narrowed here: only use the LEFT join + COALESCE when a no-match row
could actually survive, i.e. when "outerExpr OP default" does not fold to a
constant FALSE/NULL. Otherwise keep the INNER join unchanged.
The INNER path is otherwise untouched.
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>
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. Two problems with that alone:
* 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. Fixed here in pull_up_sublinks_qual_recurse(): when the
pulled-up join is LEFT, evaluate the comparison ABOVE the join (wrap it in
a FromExpr) rather than as the join condition.
* It switched to a LEFT join for every COUNT-containing subquery, even where
an INNER join was already correct (e.g. "1 = (select count(*) ...)", whose
no-match value 0 can never satisfy the predicate), churning many unrelated
plans. Narrowed here: only use the LEFT join + COALESCE when a no-match row
could actually survive, i.e. when "outerExpr OP default" does not fold to a
constant FALSE/NULL. Otherwise keep the INNER join unchanged.
The INNER path is otherwise untouched.
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>
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. Two problems with that alone:
* 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. Fixed here in pull_up_sublinks_qual_recurse(): when the
pulled-up join is LEFT, evaluate the comparison ABOVE the join (wrap it in
a FromExpr) rather than as the join condition.
* It switched to a LEFT join for every COUNT-containing subquery, even where
an INNER join was already correct (e.g. "1 = (select count(*) ...)", whose
no-match value 0 can never satisfy the predicate), churning many unrelated
plans. Narrowed here: only use the LEFT join + COALESCE when a no-match row
could actually survive, i.e. when "outerExpr OP default" does not fold to a
constant FALSE/NULL. Otherwise keep the INNER join unchanged.
The INNER path is otherwise untouched.
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>
|
Because of lack of access to your branches and it is impossible to change the branch in PR, I opened the separate PR #397. |
2 participants
Fixes #378
Summary
This is not an ORCA issue; it occurs in the fallback PostgreSQL optimizer rewrite path.
Fix incorrect semantics in correlated scalar aggregate subquery pull-up when target expression contains
COUNT.This risk applies not only to manual
optimizer=off, but also to other cases where planning falls back from ORCA to PostgreSQL optimizer.What Changed
In
cdbsubselect.c(convert_EXPR_to_join), for COUNT-containing scalar aggregate expressions:LEFT JOINin rewriteCOALESCE(agg_expr, default_expr)default_exprfrom empty-input aggregate defaults:COUNT(...) -> 0NULL(aggregate result type)This preserves no-match semantics for both COUNT-only and mixed COUNT+other aggregate expressions.
Tests Updated
Added/updated
subselect_gpregression cases for:count(*) + 1(count(*) + 1)::bigintcase when count(*) > 0 then count(*) + 1 else 1 endabs(count(*) - 1)count(*) + coalesce(sum(...), 0)coalesce(count(*) + sum(...), -1)Files
src/backend/cdb/cdbsubselect.csrc/test/regress/sql/subselect_gp.sqlsrc/test/regress/expected/subselect_gp.outsrc/test/regress/expected/subselect_gp_optimizer.out