aboutsummaryrefslogtreecommitdiff
path: root/src/backend/optimizer/util/appendinfo.c
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2023-01-30 13:16:20 -0500
committerTom Lane <tgl@sss.pgh.pa.us>2023-01-30 13:16:20 -0500
commit2489d76c4906f4461a364ca8ad7e0751ead8aa0d (patch)
tree145ebc28d5ea8f5a5ba340b9e353a11de786adae /src/backend/optimizer/util/appendinfo.c
parentec7e053a98f39a9e3c7e6d35f0d2e83933882399 (diff)
downloadpostgresql-2489d76c4906f4461a364ca8ad7e0751ead8aa0d.tar.gz
postgresql-2489d76c4906f4461a364ca8ad7e0751ead8aa0d.zip
Make Vars be outer-join-aware.
Traditionally we used the same Var struct to represent the value of a table column everywhere in parse and plan trees. This choice predates our support for SQL outer joins, and it's really a pretty bad idea with outer joins, because the Var's value can depend on where it is in the tree: it might go to NULL above an outer join. So expression nodes that are equal() per equalfuncs.c might not represent the same value, which is a huge correctness hazard for the planner. To improve this, decorate Var nodes with a bitmapset showing which outer joins (identified by RTE indexes) may have nulled them at the point in the parse tree where the Var appears. This allows us to trust that equal() Vars represent the same value. A certain amount of klugery is still needed to cope with cases where we re-order two outer joins, but it's possible to make it work without sacrificing that core principle. PlaceHolderVars receive similar decoration for the same reason. In the planner, we include these outer join bitmapsets into the relids that an expression is considered to depend on, and in consequence also add outer-join relids to the relids of join RelOptInfos. This allows us to correctly perceive whether an expression can be calculated above or below a particular outer join. This change affects FDWs that want to plan foreign joins. They *must* follow suit when labeling foreign joins in order to match with the core planner, but for many purposes (if postgres_fdw is any guide) they'd prefer to consider only base relations within the join. To support both requirements, redefine ForeignScan.fs_relids as base+OJ relids, and add a new field fs_base_relids that's set up by the core planner. Large though it is, this commit just does the minimum necessary to install the new mechanisms and get check-world passing again. Follow-up patches will perform some cleanup. (The README additions and comments mention some stuff that will appear in the follow-up.) Patch by me; thanks to Richard Guo for review. Discussion: https://postgr.es/m/830269.1656693747@sss.pgh.pa.us
Diffstat (limited to 'src/backend/optimizer/util/appendinfo.c')
-rw-r--r--src/backend/optimizer/util/appendinfo.c59
1 files changed, 53 insertions, 6 deletions
diff --git a/src/backend/optimizer/util/appendinfo.c b/src/backend/optimizer/util/appendinfo.c
index cd45ab48993..d449b5c2746 100644
--- a/src/backend/optimizer/util/appendinfo.c
+++ b/src/backend/optimizer/util/appendinfo.c
@@ -228,6 +228,28 @@ adjust_appendrel_attrs_mutator(Node *node,
if (var->varlevelsup != 0)
return (Node *) var; /* no changes needed */
+ /*
+ * You might think we need to adjust var->varnullingrels, but that
+ * shouldn't need any changes. It will contain outer-join relids,
+ * while the transformation we are making affects only baserels.
+ * Below, we just propagate var->varnullingrels into the translated
+ * Var.
+ *
+ * If var->varnullingrels isn't empty, and the translation wouldn't be
+ * a Var, we have to fail. One could imagine wrapping the translated
+ * expression in a PlaceHolderVar, but that won't work because this is
+ * typically used after freezing placeholders. Fortunately, the case
+ * appears unreachable at the moment. We can see nonempty
+ * var->varnullingrels here, but only in cases involving partitionwise
+ * joining, and in such cases the translations will always be Vars.
+ * (Non-Var translations occur only for appendrels made by flattening
+ * UNION ALL subqueries.) Should we need to make this work in future,
+ * a possible fix is to mandate that prepjointree.c create PHVs for
+ * all non-Var outputs of such subqueries, and then we could look up
+ * the pre-existing PHV here. Or perhaps just wrap the translations
+ * that way to begin with?
+ */
+
for (cnt = 0; cnt < nappinfos; cnt++)
{
if (var->varno == appinfos[cnt]->parent_relid)
@@ -255,6 +277,10 @@ adjust_appendrel_attrs_mutator(Node *node,
if (newnode == NULL)
elog(ERROR, "attribute %d of relation \"%s\" does not exist",
var->varattno, get_rel_name(appinfo->parent_reloid));
+ if (IsA(newnode, Var))
+ ((Var *) newnode)->varnullingrels = var->varnullingrels;
+ else if (var->varnullingrels != NULL)
+ elog(ERROR, "failed to apply nullingrels to a non-Var");
return newnode;
}
else if (var->varattno == 0)
@@ -308,6 +334,9 @@ adjust_appendrel_attrs_mutator(Node *node,
rowexpr->colnames = copyObject(rte->eref->colnames);
rowexpr->location = -1;
+ if (var->varnullingrels != NULL)
+ elog(ERROR, "failed to apply nullingrels to a non-Var");
+
return (Node *) rowexpr;
}
}
@@ -348,6 +377,8 @@ adjust_appendrel_attrs_mutator(Node *node,
var = copyObject(ridinfo->rowidvar);
/* ... but use the correct relid */
var->varno = leaf_relid;
+ /* identity vars shouldn't have nulling rels */
+ Assert(var->varnullingrels == NULL);
/* varnosyn in the RowIdentityVarInfo is probably wrong */
var->varnosyn = 0;
var->varattnosyn = 0;
@@ -392,8 +423,11 @@ adjust_appendrel_attrs_mutator(Node *node,
(void *) context);
/* now fix PlaceHolderVar's relid sets */
if (phv->phlevelsup == 0)
- phv->phrels = adjust_child_relids(phv->phrels, context->nappinfos,
- context->appinfos);
+ {
+ phv->phrels = adjust_child_relids(phv->phrels,
+ nappinfos, appinfos);
+ /* as above, we needn't touch phnullingrels */
+ }
return (Node *) phv;
}
/* Shouldn't need to handle planner auxiliary nodes here */
@@ -412,7 +446,7 @@ adjust_appendrel_attrs_mutator(Node *node,
RestrictInfo *oldinfo = (RestrictInfo *) node;
RestrictInfo *newinfo = makeNode(RestrictInfo);
- /* Copy all flat-copiable fields */
+ /* Copy all flat-copiable fields, notably including rinfo_serial */
memcpy(newinfo, oldinfo, sizeof(RestrictInfo));
/* Recursively fix the clause itself */
@@ -688,7 +722,11 @@ get_translated_update_targetlist(PlannerInfo *root, Index relid,
/*
* find_appinfos_by_relids
- * Find AppendRelInfo structures for all relations specified by relids.
+ * Find AppendRelInfo structures for base relations listed in relids.
+ *
+ * The relids argument is typically a join relation's relids, which can
+ * include outer-join RT indexes in addition to baserels. We silently
+ * ignore the outer joins.
*
* The AppendRelInfos are returned in an array, which can be pfree'd by the
* caller. *nappinfos is set to the number of entries in the array.
@@ -700,8 +738,9 @@ find_appinfos_by_relids(PlannerInfo *root, Relids relids, int *nappinfos)
int cnt = 0;
int i;
- *nappinfos = bms_num_members(relids);
- appinfos = (AppendRelInfo **) palloc(sizeof(AppendRelInfo *) * *nappinfos);
+ /* Allocate an array that's certainly big enough */
+ appinfos = (AppendRelInfo **)
+ palloc(sizeof(AppendRelInfo *) * bms_num_members(relids));
i = -1;
while ((i = bms_next_member(relids, i)) >= 0)
@@ -709,10 +748,17 @@ find_appinfos_by_relids(PlannerInfo *root, Relids relids, int *nappinfos)
AppendRelInfo *appinfo = root->append_rel_array[i];
if (!appinfo)
+ {
+ /* Probably i is an OJ index, but let's check */
+ if (find_base_rel_ignore_join(root, i) == NULL)
+ continue;
+ /* It's a base rel, but we lack an append_rel_array entry */
elog(ERROR, "child rel %d not found in append_rel_array", i);
+ }
appinfos[cnt++] = appinfo;
}
+ *nappinfos = cnt;
return appinfos;
}
@@ -754,6 +800,7 @@ add_row_identity_var(PlannerInfo *root, Var *orig_var,
Assert(IsA(orig_var, Var));
Assert(orig_var->varno == rtindex);
Assert(orig_var->varlevelsup == 0);
+ Assert(orig_var->varnullingrels == NULL);
/*
* If we're doing non-inherited UPDATE/DELETE/MERGE, there's little need