From 03107b4eda7f2946bb78308cb088a513b718c793 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 16 Apr 2024 11:03:43 -0400 Subject: [PATCH] Ensure generated join clauses for child rels have correct relids. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When building a join clause derived from an EquivalenceClass, if the clause is to be used with an appendrel child relation then make sure its clause_relids include the relids of that child relation. Normally this would be true already because the EquivalenceMember would be a Var of that relation. However, if the appendrel represents a flattened UNION ALL construct then some child EquivalenceMembers could be constants with no relids. The resulting under-marked clause is problematic because it could mislead join_clause_is_movable_into about where the clause should be evaluated. We do not have an example showing incorrect plan generation, but there are existing cases in the regression tests that will fail the Asserts this patch adds to get_baserel_parampathinfo. A similarly wrong conclusion about a clause being considered by get_joinrel_parampathinfo would lead to wrong placement of the clause. (This also squares with the way that clause_relids is calculated for non-equijoin clauses in adjust_appendrel_attrs.) The other reason for wanting these new Asserts is that the previous blithe assumption that the results of generate_join_implied_equalities "necessarily satisfy join_clause_is_movable_into" turns out to be wrong pre-v16. If it's still wrong it'd be good to find out. Per bug #18429 from Benoît Ryder. The bug as filed was fixed by commit 2489d76c4, but these changes correlate with the fix we will need to apply in pre-v16 branches. Discussion: https://postgr.es/m/18429-8982d4a348cc86c6@postgresql.org --- src/backend/optimizer/path/equivclass.c | 15 ++++++++++++++ src/backend/optimizer/util/relnode.c | 26 ++++++++++++++++++------- 2 files changed, 34 insertions(+), 7 deletions(-) diff --git a/src/backend/optimizer/path/equivclass.c b/src/backend/optimizer/path/equivclass.c index a619ff9177..1d6bedb399 100644 --- a/src/backend/optimizer/path/equivclass.c +++ b/src/backend/optimizer/path/equivclass.c @@ -1896,6 +1896,21 @@ create_join_clause(PlannerInfo *root, rightem->em_relids), ec->ec_min_security); + /* + * If either EM is a child, force the clause's clause_relids to include + * the relid(s) of the child rel. In normal cases it would already, but + * not if we are considering appendrel child relations with pseudoconstant + * translated variables (i.e., UNION ALL sub-selects with constant output + * items). We must do this so that join_clause_is_movable_into() will + * think that the clause should be evaluated at the correct place. + */ + if (leftem->em_is_child) + rinfo->clause_relids = bms_add_members(rinfo->clause_relids, + leftem->em_relids); + if (rightem->em_is_child) + rinfo->clause_relids = bms_add_members(rinfo->clause_relids, + rightem->em_relids); + /* If it's a child clause, copy the parent's rinfo_serial */ if (parent_rinfo) rinfo->rinfo_serial = parent_rinfo->rinfo_serial; diff --git a/src/backend/optimizer/util/relnode.c b/src/backend/optimizer/util/relnode.c index 0c125e42e8..e05b21c884 100644 --- a/src/backend/optimizer/util/relnode.c +++ b/src/backend/optimizer/util/relnode.c @@ -1560,6 +1560,7 @@ get_baserel_parampathinfo(PlannerInfo *root, RelOptInfo *baserel, ParamPathInfo *ppi; Relids joinrelids; List *pclauses; + List *eqclauses; Bitmapset *pserials; double rows; ListCell *lc; @@ -1595,14 +1596,25 @@ get_baserel_parampathinfo(PlannerInfo *root, RelOptInfo *baserel, /* * Add in joinclauses generated by EquivalenceClasses, too. (These - * necessarily satisfy join_clause_is_movable_into.) + * necessarily satisfy join_clause_is_movable_into; but in assert-enabled + * builds, let's verify that.) */ - pclauses = list_concat(pclauses, - generate_join_implied_equalities(root, - joinrelids, - required_outer, - baserel, - NULL)); + eqclauses = generate_join_implied_equalities(root, + joinrelids, + required_outer, + baserel, + NULL); +#ifdef USE_ASSERT_CHECKING + foreach(lc, eqclauses) + { + RestrictInfo *rinfo = (RestrictInfo *) lfirst(lc); + + Assert(join_clause_is_movable_into(rinfo, + baserel->relids, + joinrelids)); + } +#endif + pclauses = list_concat(pclauses, eqclauses); /* Compute set of serial numbers of the enforced clauses */ pserials = NULL;