From fd64356ca7fdab1368e14168717fd17dbb0c180b Mon Sep 17 00:00:00 2001 From: Tomas Vondra Date: Fri, 13 Nov 2026 12:34:20 +0100 Subject: [PATCH v4-15] Fix FULL JOIN constant propagation with non-strict operands Deriving input restrictions from COALESCE(left, right) = constant can discard unmatched rows when either operand produces a non-NULL value for a null-extended input. Require both operands to be provably null-preserving before deriving either equality, so that the value remains NULL for null-extended input. An expression is null-preserving if it contains only strict functions, and at least one level-zero variable (Var or PlaceHolderVar). The check is implemented as an expression walker, closely mirrorring contain_nonstrict_functions_walker(), with different handling for Var and PlaceHolderVar at level zero, and CoalesceExpr at any depth. The walker does not descend into level-zero PlaceHolderVars, as those are evaluated below the join and so go to NULL for null-extended rows. CoalesceExpr is treated as null-preserving if all its arguments are null-preserving. So for example (when K is a constant) COALESCE(left.a, COALESCE(right.a, right.b)) = K is null-preserving, while COALESCE(left.a, COALESCE(right.a, K)) = K is not, because the nested COALESCE discards NULL values. PlaceHolderVars at level zero are automatically null-preserving. Those represent expressions calculated below the join, so the result will be nulled no matter the expression. Add regression coverage for unsafe operands and safe nested FULL JOIN USING cases. Backpatch to all supported branches. The issue was likely introduced in 8.1, by commit cc5e80b8d1c4. Author: Tomas Vondra Reviewed-by: Ayush Tiwari Reviewed-by: Kirill Reshke Discussion: https://postgr.es/m/0074fbf7-a92c-417a-937a-6b1fc27aed82@vondra.me Backpatch-through: 15 --- src/backend/optimizer/path/equivclass.c | 9 ++ src/backend/optimizer/util/clauses.c | 195 ++++++++++++++++++++++++ src/include/optimizer/clauses.h | 1 + src/test/regress/expected/join.out | 138 +++++++++++++++++ src/test/regress/sql/join.sql | 58 +++++++ 5 files changed, 401 insertions(+) diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c index b15f6603dd6..d2caeb5e7a3 100644 --- a/src/backend/optimizer/path/equivclass.c +++ b/src/backend/optimizer/path/equivclass.c @@ -2258,6 +2258,15 @@ reconsider_full_join_clause(PlannerInfo *root, RestrictInfo *rinfo) bms_is_subset(right_relids, sjinfo->syn_lefthand))) return false; + /* + * For an unmatched row, the other input's expression must become NULL. + * Otherwise COALESCE could supply the constant even when this input's + * expression is different, making the derived restriction invalid. + */ + if (!is_full_join_expr_null_preserving((Node *) leftvar) || + !is_full_join_expr_null_preserving((Node *) rightvar)) + return false; + foreach(lc1, root->eq_classes) { EquivalenceClass *cur_ec = (EquivalenceClass *) lfirst(lc1); diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index c38c76beb4d..8360906584c 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -154,6 +154,8 @@ static Node *substitute_actual_srf_parameters_mutator(Node *node, substitute_actual_srf_parameters_context *context); static bool pull_paramids_walker(Node *node, Bitmapset **context); +static bool full_join_expr_nonstrict_walker(Node *node, bool *found_nullable); + /***************************************************************************** * Aggregate-function clause manipulation @@ -941,6 +943,8 @@ max_parallel_hazard_walker(Node *node, max_parallel_hazard_context *context) * one or more Var or Param nodes (as appropriate for the caller's need), and * now wishes to prove that the expression result will be NULL if any of these * inputs is NULL. If we return false, then the proof succeeded. + * + * full_join_expr_nonstrict_walker() mirrors this logic, so keep the two in sync. */ bool contain_nonstrict_functions(Node *clause) @@ -1082,6 +1086,197 @@ contain_nonstrict_functions_walker(Node *node, void *context) context); } +/* + * is_full_join_expr_null_preserving + * Check whether an expression becomes NULL on null-extended input. + * + * The caller has verified that all referenced rels belong to one input. + * + * We conservatively require that the expression contains no non-strict + * constructs, and at least one level-zero Var or PlaceHolderVar. The latter + * test rules out non-NULL constants and pseudoconstants appearing as COALESCE + * arguments. + * + * A level-zero PlaceHolderVar is evaluated below the join, so its result is + * nulled regardless of the contained expression, and we don't look inside + * it. COALESCE is not strict in any one argument, but preserves NULL if all + * its arguments do. + * + * This mirrors contain_nonstrict_functions_walker(), except for the handling + * of Vars, PlaceHolderVars and COALESCE, so keep the two in sync. + */ +bool +is_full_join_expr_null_preserving(Node *node) +{ + bool found_nullable = false; + + if (full_join_expr_nonstrict_walker(node, &found_nullable)) + return false; + + return found_nullable; +} + +/* + * Walker for is_full_join_expr_null_preserving. Returns true if it finds a + * construct that might produce non-NULL output for a null-extended input, + * and sets *found_nullable if it finds an input nulled by the join. + */ +static bool +full_join_expr_nonstrict_walker(Node *node, bool *found_nullable) +{ + if (node == NULL) + return false; + if (IsA(node, Var)) + { + if (((Var *) node)->varlevelsup == 0) + *found_nullable = true; + return false; + } + else if (IsA(node, PlaceHolderVar)) + { + if (((PlaceHolderVar *) node)->phlevelsup == 0) + { + /* don't descend into the contained expression */ + *found_nullable = true; + return false; + } + /* else fall through to check the contained expression */ + } + else if (IsA(node, Aggref)) + { + /* an aggregate could return non-null with null input */ + return true; + } + else if (IsA(node, GroupingFunc)) + { + /* + * A GroupingFunc doesn't evaluate its arguments, and therefore must + * be treated as nonstrict. + */ + return true; + } + else if (IsA(node, WindowFunc)) + { + /* a window function could return non-null with null input */ + return true; + } + else if (IsA(node, SubscriptingRef)) + { + SubscriptingRef *sbsref = (SubscriptingRef *) node; + const SubscriptRoutines *sbsroutines; + + /* Subscripting assignment is always presumed nonstrict */ + if (sbsref->refassgnexpr != NULL) + return true; + /* Otherwise we must look up the subscripting support methods */ + sbsroutines = getSubscriptingRoutines(sbsref->refcontainertype, NULL); + if (!(sbsroutines && sbsroutines->fetch_strict)) + return true; + /* else fall through to check args */ + } + else if (IsA(node, DistinctExpr)) + { + /* IS DISTINCT FROM is inherently non-strict */ + return true; + } + else if (IsA(node, NullIfExpr)) + { + /* NULLIF is inherently non-strict */ + return true; + } + else if (IsA(node, ScalarArrayOpExpr)) + { + ScalarArrayOpExpr *expr = (ScalarArrayOpExpr *) node; + + if (!is_strict_saop(expr, false)) + return true; + /* else fall through to check args */ + } + else if (IsA(node, BoolExpr)) + { + BoolExpr *expr = (BoolExpr *) node; + + switch (expr->boolop) + { + case AND_EXPR: + case OR_EXPR: + /* AND, OR are inherently non-strict */ + return true; + default: + break; + } + } + else if (IsA(node, SubLink)) + { + /* In some cases a sublink might be strict, but in general not */ + return true; + } + else if (IsA(node, SubPlan)) + return true; + else if (IsA(node, AlternativeSubPlan)) + return true; + else if (IsA(node, FieldStore)) + return true; + else if (IsA(node, CoerceViaIO)) + { + /* + * CoerceViaIO is strict regardless of whether the I/O functions are, + * so we should skip check_functions_in_node() and just fall through + * to check the arguments. + */ + } + else if (IsA(node, ArrayCoerceExpr)) + { + /* + * ArrayCoerceExpr is strict at the array level, regardless of what + * the per-element expression is; so we should ignore elemexpr and + * recurse only into the arg. + */ + return full_join_expr_nonstrict_walker((Node *) ((ArrayCoerceExpr *) node)->arg, + found_nullable); + } + else if (IsA(node, CaseExpr)) + return true; + else if (IsA(node, ArrayExpr)) + return true; + else if (IsA(node, RowExpr)) + return true; + else if (IsA(node, RowCompareExpr)) + return true; + else if (IsA(node, CoalesceExpr)) + { + CoalesceExpr *cexpr = (CoalesceExpr *) node; + ListCell *lc; + + /* COALESCE is non-strict, but preserves NULL if all arguments do */ + foreach(lc, cexpr->args) + { + if (!is_full_join_expr_null_preserving(lfirst(lc))) + return true; + } + *found_nullable = true; + return false; + } + else if (IsA(node, MinMaxExpr)) + return true; + else if (IsA(node, XmlExpr)) + return true; + else if (IsA(node, NullTest)) + return true; + else if (IsA(node, BooleanTest)) + return true; + else + { + /* Check other function-containing nodes */ + if (check_functions_in_node(node, contain_nonstrict_functions_checker, + NULL)) + return true; + } + + return expression_tree_walker(node, full_join_expr_nonstrict_walker, + found_nullable); +} + /***************************************************************************** * Check clauses for Params *****************************************************************************/ diff --git a/src/include/optimizer/clauses.h b/src/include/optimizer/clauses.h index 6c5203dc448..39c18f1ae98 100644 --- a/src/include/optimizer/clauses.h +++ b/src/include/optimizer/clauses.h @@ -35,6 +35,7 @@ extern bool contain_subplans(Node *clause); extern char max_parallel_hazard(Query *parse); extern bool is_parallel_safe(PlannerInfo *root, Node *node); extern bool contain_nonstrict_functions(Node *clause); +extern bool is_full_join_expr_null_preserving(Node *node); extern bool contain_exec_param(Node *clause, List *param_ids); extern bool contain_leaked_vars(Node *clause); diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out index 7c68e4bd2b6..924774cbe1c 100644 --- a/src/test/regress/expected/join.out +++ b/src/test/regress/expected/join.out @@ -6653,6 +6653,144 @@ ERROR: invalid reference to FROM-clause entry for table "xx1" LINE 1: ...xx1 using lateral (select * from int4_tbl where f1 = x1) ss; ^ HINT: There is an entry for table "xx1", but it cannot be referenced from this part of the query. +-- non-strict operands must not cause unmatched rows to be filtered out +explain (costs off) +select * from (values (null::int), (1)) a(x) + full join (values (0), (1)) b(x) on a.x = coalesce(b.x, 0) + where coalesce(a.x, coalesce(b.x, 0)) = 0 + order by a.x, b.x; + QUERY PLAN +--------------------------------------------------------------------------------------- + Sort + Sort Key: "*VALUES*".column1, "*VALUES*_1".column1 + -> Hash Full Join + Hash Cond: ("*VALUES*".column1 = COALESCE("*VALUES*_1".column1, 0)) + Filter: (COALESCE("*VALUES*".column1, COALESCE("*VALUES*_1".column1, 0)) = 0) + -> Values Scan on "*VALUES*" + -> Hash + -> Values Scan on "*VALUES*_1" +(8 rows) + +select * from (values (null::int), (1)) a(x) + full join (values (0), (1)) b(x) on a.x = coalesce(b.x, 0) + where coalesce(a.x, coalesce(b.x, 0)) = 0 + order by a.x, b.x; + x | x +---+--- + | 0 + | +(2 rows) + +explain (costs off) +select * from (values (0), (1)) a(x) + full join (values (null::int), (1)) b(x) + on (case when a.x is null then 0 else a.x end) = b.x + where coalesce(case when a.x is null then 0 else a.x end, b.x) = 0 + order by a.x, b.x; + QUERY PLAN +--------------------------------------------------------------------------------------------------------------------------------- + Sort + Sort Key: "*VALUES*".column1, "*VALUES*_1".column1 + -> Hash Full Join + Hash Cond: (CASE WHEN ("*VALUES*".column1 IS NULL) THEN 0 ELSE "*VALUES*".column1 END = "*VALUES*_1".column1) + Filter: (COALESCE(CASE WHEN ("*VALUES*".column1 IS NULL) THEN 0 ELSE "*VALUES*".column1 END, "*VALUES*_1".column1) = 0) + -> Values Scan on "*VALUES*" + -> Hash + -> Values Scan on "*VALUES*_1" +(8 rows) + +select * from (values (0), (1)) a(x) + full join (values (null::int), (1)) b(x) + on (case when a.x is null then 0 else a.x end) = b.x + where coalesce(case when a.x is null then 0 else a.x end, b.x) = 0 + order by a.x, b.x; + x | x +---+--- + 0 | + | +(2 rows) + +-- nested merged columns and precomputed expressions remain null-preserving +explain (costs off) +select * from (select coalesce(f1, 0) as f1 from int4_tbl) a + full join int4_tbl b using (f1) + full join int4_tbl c using (f1) + where f1 = 0; + QUERY PLAN +---------------------------------------------------------------------------- + Hash Full Join + Hash Cond: (COALESCE((COALESCE(int4_tbl.f1, 0)), b.f1) = c.f1) + Filter: (COALESCE(COALESCE((COALESCE(int4_tbl.f1, 0)), b.f1), c.f1) = 0) + -> Hash Full Join + Hash Cond: ((COALESCE(int4_tbl.f1, 0)) = b.f1) + -> Seq Scan on int4_tbl + -> Hash + -> Seq Scan on int4_tbl b + -> Hash + -> Seq Scan on int4_tbl c + Filter: (f1 = 0) +(11 rows) + +select * from (select coalesce(f1, 0) as f1 from int4_tbl) a + full join int4_tbl b using (f1) + full join int4_tbl c using (f1) + where f1 = 0; + f1 +---- + 0 +(1 row) + +-- strict expressions of precomputed values remain null-preserving +explain (costs off) +select * from (select coalesce(x, 0) as x from (values (null::int), (1)) v(x)) a + full join (values (1), (2)) b(x) on a.x + 0 = b.x + where coalesce(a.x + 0, b.x) = 0 + order by a.x, b.x; + QUERY PLAN +----------------------------------------------------------------------------------------------- + Sort + Sort Key: (COALESCE("*VALUES*".column1, 0)), "*VALUES*_1".column1 + -> Hash Full Join + Hash Cond: (((COALESCE("*VALUES*".column1, 0)) + 0) = "*VALUES*_1".column1) + Filter: (COALESCE(((COALESCE("*VALUES*".column1, 0)) + 0), "*VALUES*_1".column1) = 0) + -> Values Scan on "*VALUES*" + -> Hash + -> Values Scan on "*VALUES*_1" + Filter: (column1 = 0) +(9 rows) + +select * from (select coalesce(x, 0) as x from (values (null::int), (1)) v(x)) a + full join (values (1), (2)) b(x) on a.x + 0 = b.x + where coalesce(a.x + 0, b.x) = 0 + order by a.x, b.x; + x | x +---+--- + 0 | +(1 row) + +explain (costs off) +select * from (select coalesce(x, '{0}') as x from (values (null::int[]), ('{1}')) v(x)) a + full join (values (1), (2)) b(x) on a.x[1] = b.x + where coalesce(a.x[1], b.x) = 0; + QUERY PLAN +----------------------------------------------------------------------------------------------------- + Hash Full Join + Hash Cond: ((COALESCE("*VALUES*".column1, '{0}'::integer[]))[1] = "*VALUES*_1".column1) + Filter: (COALESCE((COALESCE("*VALUES*".column1, '{0}'::integer[]))[1], "*VALUES*_1".column1) = 0) + -> Values Scan on "*VALUES*" + -> Hash + -> Values Scan on "*VALUES*_1" + Filter: (column1 = 0) +(7 rows) + +select * from (select coalesce(x, '{0}') as x from (values (null::int[]), ('{1}')) v(x)) a + full join (values (1), (2)) b(x) on a.x[1] = b.x + where coalesce(a.x[1], b.x) = 0; + x | x +-----+--- + {0} | +(1 row) + -- -- test LATERAL reference propagation down a multi-level inheritance hierarchy -- produced for a multi-level partitioned table hierarchy. diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql index affe1c4cbaf..490d4eeed34 100644 --- a/src/test/regress/sql/join.sql +++ b/src/test/regress/sql/join.sql @@ -2313,6 +2313,64 @@ delete from xx1 using (select * from int4_tbl where f1 = x1) ss; delete from xx1 using (select * from int4_tbl where f1 = xx1.x1) ss; delete from xx1 using lateral (select * from int4_tbl where f1 = x1) ss; +-- non-strict operands must not cause unmatched rows to be filtered out +explain (costs off) +select * from (values (null::int), (1)) a(x) + full join (values (0), (1)) b(x) on a.x = coalesce(b.x, 0) + where coalesce(a.x, coalesce(b.x, 0)) = 0 + order by a.x, b.x; + +select * from (values (null::int), (1)) a(x) + full join (values (0), (1)) b(x) on a.x = coalesce(b.x, 0) + where coalesce(a.x, coalesce(b.x, 0)) = 0 + order by a.x, b.x; + +explain (costs off) +select * from (values (0), (1)) a(x) + full join (values (null::int), (1)) b(x) + on (case when a.x is null then 0 else a.x end) = b.x + where coalesce(case when a.x is null then 0 else a.x end, b.x) = 0 + order by a.x, b.x; + +select * from (values (0), (1)) a(x) + full join (values (null::int), (1)) b(x) + on (case when a.x is null then 0 else a.x end) = b.x + where coalesce(case when a.x is null then 0 else a.x end, b.x) = 0 + order by a.x, b.x; + +-- nested merged columns and precomputed expressions remain null-preserving +explain (costs off) +select * from (select coalesce(f1, 0) as f1 from int4_tbl) a + full join int4_tbl b using (f1) + full join int4_tbl c using (f1) + where f1 = 0; + +select * from (select coalesce(f1, 0) as f1 from int4_tbl) a + full join int4_tbl b using (f1) + full join int4_tbl c using (f1) + where f1 = 0; + +-- strict expressions of precomputed values remain null-preserving +explain (costs off) +select * from (select coalesce(x, 0) as x from (values (null::int), (1)) v(x)) a + full join (values (1), (2)) b(x) on a.x + 0 = b.x + where coalesce(a.x + 0, b.x) = 0 + order by a.x, b.x; + +select * from (select coalesce(x, 0) as x from (values (null::int), (1)) v(x)) a + full join (values (1), (2)) b(x) on a.x + 0 = b.x + where coalesce(a.x + 0, b.x) = 0 + order by a.x, b.x; + +explain (costs off) +select * from (select coalesce(x, '{0}') as x from (values (null::int[]), ('{1}')) v(x)) a + full join (values (1), (2)) b(x) on a.x[1] = b.x + where coalesce(a.x[1], b.x) = 0; + +select * from (select coalesce(x, '{0}') as x from (values (null::int[]), ('{1}')) v(x)) a + full join (values (1), (2)) b(x) on a.x[1] = b.x + where coalesce(a.x[1], b.x) = 0; + -- -- test LATERAL reference propagation down a multi-level inheritance hierarchy -- produced for a multi-level partitioned table hierarchy. -- 2.56.0