v2-0002-add-nullingrels-fields.patch
text/x-diff
Filename: v2-0002-add-nullingrels-fields.patch
Type: text/x-diff
Part: 2
Message:
Re: Making Vars outer-join aware
Patch
Same data as JSON:
GET /api/v1/attachments/:id/patch
the parsed metadata as JSON — format, series position, per-file stats; never the diff bytes.
API reference →
Format: unified
Series: patch v2-0002
| File | + | − |
|---|---|---|
| src/backend/nodes/makefuncs.c | 6 | 4 |
| src/backend/nodes/nodeFuncs.c | 2 | 1 |
| src/backend/rewrite/rewriteManip.c | 173 | 5 |
| src/backend/utils/misc/queryjumble.c | 5 | 0 |
| src/include/nodes/pathnodes.h | 14 | 4 |
| src/include/nodes/primnodes.h | 10 | 0 |
| src/include/rewrite/rewriteManip.h | 4 | 0 |
commit bd58b68da9d15c0e2cce86a6a837302a8a764598
Author: Tom Lane <tgl@sss.pgh.pa.us>
Date: Sun Jul 10 13:26:16 2022 -0400
Add Var.varnullingrels and PlaceHolderVar.phnullingrels fields.
These fields are always empty as of this commit, so they don't
affect any behavior, even though equal() will compare them.
Update backend/nodes/ and backend/rewrite/ infrastructure as needed.
Also add some rewrite functions we'll need later.
diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index 28288dcfc1..19606c495f 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -81,11 +81,13 @@ makeVar(int varno,
var->varlevelsup = varlevelsup;
/*
- * Only a few callers need to make Var nodes with varnosyn/varattnosyn
- * different from varno/varattno. We don't provide separate arguments for
- * them, but just initialize them to the given varno/varattno. This
- * reduces code clutter and chance of error for most callers.
+ * Only a few callers need to make Var nodes with non-null varnullingrels,
+ * or with varnosyn/varattnosyn different from varno/varattno. We don't
+ * provide separate arguments for them, but just initialize them to NULL
+ * and the given varno/varattno. This reduces code clutter and chance of
+ * error for most callers.
*/
+ var->varnullingrels = NULL;
var->varnosyn = (Index) varno;
var->varattnosyn = varattno;
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index 4cb1744da6..ccf63515fa 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -2847,6 +2847,7 @@ expression_tree_mutator(Node *node,
Var *newnode;
FLATCOPY(newnode, var, Var);
+ /* Assume we need not copy the varnullingrels bitmapset */
return (Node *) newnode;
}
break;
@@ -3442,7 +3443,7 @@ expression_tree_mutator(Node *node,
FLATCOPY(newnode, phv, PlaceHolderVar);
MUTATE(newnode->phexpr, phv->phexpr, Expr *);
- /* Assume we need not copy the relids bitmapset */
+ /* Assume we need not copy the relids bitmapsets */
return (Node *) newnode;
}
break;
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index 101c39553a..a0a0026469 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -40,6 +40,13 @@ typedef struct
int win_location;
} locate_windowfunc_context;
+typedef struct
+{
+ Bitmapset *removable_relids;
+ Bitmapset *except_relids;
+ int sublevels_up;
+} remove_nulling_relids_context;
+
static bool contain_aggs_of_level_walker(Node *node,
contain_aggs_of_level_context *context);
static bool locate_agg_of_level_walker(Node *node,
@@ -50,6 +57,9 @@ static bool locate_windowfunc_walker(Node *node,
static bool checkExprHasSubLink_walker(Node *node, void *context);
static Relids offset_relid_set(Relids relids, int offset);
static Relids adjust_relid_set(Relids relids, int oldrelid, int newrelid);
+static bool get_nulling_relids_walker(Node *node, Bitmapset **context);
+static Node *remove_nulling_relids_mutator(Node *node,
+ remove_nulling_relids_context *context);
/*
@@ -348,6 +358,8 @@ OffsetVarNodes_walker(Node *node, OffsetVarNodes_context *context)
if (var->varlevelsup == context->sublevels_up)
{
var->varno += context->offset;
+ var->varnullingrels = offset_relid_set(var->varnullingrels,
+ context->offset);
if (var->varnosyn > 0)
var->varnosyn += context->offset;
}
@@ -386,6 +398,8 @@ OffsetVarNodes_walker(Node *node, OffsetVarNodes_context *context)
{
phv->phrels = offset_relid_set(phv->phrels,
context->offset);
+ phv->phnullingrels = offset_relid_set(phv->phnullingrels,
+ context->offset);
}
/* fall through to examine children */
}
@@ -510,11 +524,13 @@ ChangeVarNodes_walker(Node *node, ChangeVarNodes_context *context)
{
Var *var = (Var *) node;
- if (var->varlevelsup == context->sublevels_up &&
- var->varno == context->rt_index)
+ if (var->varlevelsup == context->sublevels_up)
{
- var->varno = context->new_index;
- /* If the syntactic referent is same RTE, fix it too */
+ if (var->varno == context->rt_index)
+ var->varno = context->new_index;
+ var->varnullingrels = adjust_relid_set(var->varnullingrels,
+ context->rt_index,
+ context->new_index);
if (var->varnosyn == context->rt_index)
var->varnosyn = context->new_index;
}
@@ -557,6 +573,9 @@ ChangeVarNodes_walker(Node *node, ChangeVarNodes_context *context)
phv->phrels = adjust_relid_set(phv->phrels,
context->rt_index,
context->new_index);
+ phv->phnullingrels = adjust_relid_set(phv->phnullingrels,
+ context->rt_index,
+ context->new_index);
}
/* fall through to examine children */
}
@@ -833,7 +852,8 @@ rangeTableEntry_used_walker(Node *node,
Var *var = (Var *) node;
if (var->varlevelsup == context->sublevels_up &&
- var->varno == context->rt_index)
+ (var->varno == context->rt_index ||
+ bms_is_member(context->rt_index, var->varnullingrels)))
return true;
return false;
}
@@ -1061,6 +1081,154 @@ AddInvertedQual(Query *parsetree, Node *qual)
}
+/*
+ * get_nulling_relids collects all the level-zero RT indexes mentioned in
+ * Var.varnullingrels and PlaceHolderVar.phnullingrels fields within the
+ * given expression.
+ */
+Bitmapset *
+get_nulling_relids(Node *node)
+{
+ Bitmapset *result = NULL;
+
+ (void) get_nulling_relids_walker(node, &result);
+ return result;
+}
+
+static bool
+get_nulling_relids_walker(Node *node, Bitmapset **context)
+{
+ if (node == NULL)
+ return false;
+ if (IsA(node, Var))
+ {
+ Var *var = (Var *) node;
+
+ if (var->varlevelsup == 0)
+ *context = bms_add_members(*context, var->varnullingrels);
+ }
+ else if (IsA(node, PlaceHolderVar))
+ {
+ PlaceHolderVar *phv = (PlaceHolderVar *) node;
+
+ if (phv->phlevelsup == 0)
+ *context = bms_add_members(*context, phv->phnullingrels);
+ }
+
+ /*
+ * Currently, this is only used after the planner has converted SubLinks
+ * to SubPlans, so we don't need to support recursion into sub-Queries; so
+ * no sublevels_up counting is needed.
+ */
+ Assert(!IsA(node, SubLink));
+ Assert(!IsA(node, Query));
+ return expression_tree_walker(node, get_nulling_relids_walker, context);
+}
+
+/*
+ * remove_nulling_relids removes mentions of the specified RT index(es)
+ * in Var.varnullingrels and PlaceHolderVar.phnullingrels fields within
+ * the given expression, except in nodes belonging to rels listed in
+ * except_relids.
+ *
+ * XXX consider making this a destructive walker.
+ */
+Node *
+remove_nulling_relids(Node *node, Bitmapset *removable_relids,
+ Bitmapset *except_relids)
+{
+ remove_nulling_relids_context context;
+
+ context.removable_relids = removable_relids;
+ context.except_relids = except_relids;
+ context.sublevels_up = 0;
+ return query_or_expression_tree_mutator(node,
+ remove_nulling_relids_mutator,
+ &context,
+ 0);
+}
+
+static Node *
+remove_nulling_relids_mutator(Node *node,
+ remove_nulling_relids_context *context)
+{
+ if (node == NULL)
+ return NULL;
+ if (IsA(node, Var))
+ {
+ Var *var = (Var *) node;
+
+ if (var->varlevelsup == context->sublevels_up &&
+ !bms_is_member(var->varno, context->except_relids) &&
+ bms_overlap(var->varnullingrels, context->removable_relids))
+ {
+ Relids newnullingrels = bms_difference(var->varnullingrels,
+ context->removable_relids);
+
+ /* Micro-optimization: ensure nullingrels is NULL if empty */
+ if (bms_is_empty(newnullingrels))
+ newnullingrels = NULL;
+ /* Copy the Var ... */
+ var = copyObject(var);
+ /* ... and replace the copy's varnullingrels field */
+ var->varnullingrels = newnullingrels;
+ return (Node *) var;
+ }
+ /* Otherwise fall through to copy the Var normally */
+ }
+ else if (IsA(node, PlaceHolderVar))
+ {
+ PlaceHolderVar *phv = (PlaceHolderVar *) node;
+
+ if (phv->phlevelsup == context->sublevels_up &&
+ !bms_overlap(phv->phrels, context->except_relids))
+ {
+ Relids newnullingrels = bms_difference(phv->phnullingrels,
+ context->removable_relids);
+
+ /*
+ * Micro-optimization: ensure nullingrels is NULL if empty.
+ *
+ * Note: it might seem desirable to remove the PHV altogether if
+ * phnullingrels goes to empty. Currently we dare not do that
+ * because we use PHVs in some cases to enforce separate identity
+ * of subexpressions; see wrap_non_vars usages in prepjointree.c.
+ */
+ if (bms_is_empty(newnullingrels))
+ newnullingrels = NULL;
+ /* Copy the PlaceHolderVar and mutate what's below ... */
+ phv = (PlaceHolderVar *)
+ expression_tree_mutator(node,
+ remove_nulling_relids_mutator,
+ (void *) context);
+ /* ... and replace the copy's phnullingrels field */
+ phv->phnullingrels = newnullingrels;
+ /* We must also update phrels, if it contains a removable RTI */
+ phv->phrels = bms_difference(phv->phrels,
+ context->removable_relids);
+ Assert(!bms_is_empty(phv->phrels));
+ return (Node *) phv;
+ }
+ /* Otherwise fall through to copy the PlaceHolderVar normally */
+ }
+ else if (IsA(node, Query))
+ {
+ /* Recurse into RTE or sublink subquery */
+ Query *newnode;
+
+ context->sublevels_up++;
+ newnode = query_tree_mutator((Query *) node,
+ remove_nulling_relids_mutator,
+ (void *) context,
+ 0);
+ context->sublevels_up--;
+ return (Node *) newnode;
+ }
+ return expression_tree_mutator(node, remove_nulling_relids_mutator,
+ (void *) context);
+}
+
+
/*
* replace_rte_variables() finds all Vars in an expression tree
* that reference a particular RTE, and replaces them with substitute
diff --git a/src/backend/utils/misc/queryjumble.c b/src/backend/utils/misc/queryjumble.c
index eeaa0b31fe..e517e0363c 100644
--- a/src/backend/utils/misc/queryjumble.c
+++ b/src/backend/utils/misc/queryjumble.c
@@ -381,6 +381,11 @@ JumbleExpr(JumbleState *jstate, Node *node)
APP_JUMB(var->varno);
APP_JUMB(var->varattno);
APP_JUMB(var->varlevelsup);
+
+ /*
+ * We can omit varnullingrels, because it's fully determined
+ * by varno/varlevelsup plus the Var's query location.
+ */
}
break;
case T_Const:
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index ee8bed6452..369ddfbddc 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -2574,10 +2574,15 @@ typedef struct MergeScanSelCache
* of a plan tree. This is used during planning to represent the contained
* expression. At the end of the planning process it is replaced by either
* the contained expression or a Var referring to a lower-level evaluation of
- * the contained expression. Typically the evaluation occurs below an outer
+ * the contained expression. Generally the evaluation occurs below an outer
* join, and Var references above the outer join might thereby yield NULL
* instead of the expression value.
*
+ * phrels and phlevelsup correspond to the varno/varlevelsup fields of a
+ * plain Var, except that phrels has to be a relid set since the evaluation
+ * level of a PlaceHolderVar might be a join rather than a base relation.
+ * Likewise, phnullingrels corresponds to varnullingrels.
+ *
* Although the planner treats this as an expression node type, it is not
* recognized by the parser or executor, so we declare it here rather than
* in primnodes.h.
@@ -2590,8 +2595,10 @@ typedef struct MergeScanSelCache
* PHV. Another way in which it can happen is that initplan sublinks
* could get replaced by differently-numbered Params when sublink folding
* is done. (The end result of such a situation would be some
- * unreferenced initplans, which is annoying but not really a problem.) On
- * the same reasoning, there is no need to examine phrels.
+ * unreferenced initplans, which is annoying but not really a problem.) On
+ * the same reasoning, there is no need to examine phrels. But we do need
+ * to compare phnullingrels, as that represents effects that are external
+ * to the original value of the PHV.
*/
typedef struct PlaceHolderVar
@@ -2601,9 +2608,12 @@ typedef struct PlaceHolderVar
/* the represented expression */
Expr *phexpr pg_node_attr(equal_ignore);
- /* base relids syntactically within expr src */
+ /* base+OJ relids syntactically within expr src */
Relids phrels pg_node_attr(equal_ignore);
+ /* RT indexes of outer joins that can null PHV's value */
+ Relids phnullingrels;
+
/* ID for PHV (unique within planner run) */
Index phid;
diff --git a/src/include/nodes/primnodes.h b/src/include/nodes/primnodes.h
index 1fc2fbffa3..f2a0739e6e 100644
--- a/src/include/nodes/primnodes.h
+++ b/src/include/nodes/primnodes.h
@@ -189,6 +189,14 @@ typedef struct Expr
* row identity information during UPDATE/DELETE. This value should never
* be seen outside the planner.
*
+ * varnullingrels is the set of RT indexes of outer joins that can force
+ * the Var's value to null (at the point where it appears in the query).
+ * See optimizer/README for discussion of that.
+ *
+ * varlevelsup is greater than zero in Vars that represent outer references.
+ * Note that it affects the meaning of all of varno, varnullingrels, and
+ * varnosyn, all of which refer to the range table of that query level.
+ *
* In the parser, varnosyn and varattnosyn are either identical to
* varno/varattno, or they specify the column's position in an aliased JOIN
* RTE that hides the semantic referent RTE's refname. This is a syntactic
@@ -231,6 +239,8 @@ typedef struct Var
int32 vartypmod;
/* OID of collation, or InvalidOid if none */
Oid varcollid;
+ /* RT indexes of outer joins that can replace the Var's value with null */
+ Bitmapset *varnullingrels;
/*
* for subquery variables referencing outer relations; 0 in a normal var,
diff --git a/src/include/rewrite/rewriteManip.h b/src/include/rewrite/rewriteManip.h
index 98b9b3a288..a3f902c1bb 100644
--- a/src/include/rewrite/rewriteManip.h
+++ b/src/include/rewrite/rewriteManip.h
@@ -63,6 +63,10 @@ extern bool contain_windowfuncs(Node *node);
extern int locate_windowfunc(Node *node);
extern bool checkExprHasSubLink(Node *node);
+extern Bitmapset *get_nulling_relids(Node *node);
+extern Node *remove_nulling_relids(Node *node, Bitmapset *removable_relids,
+ Bitmapset *except_relids);
+
extern Node *replace_rte_variables(Node *node,
int target_varno, int sublevels_up,
replace_rte_variables_callback callback,