Skip to content

Commit 1e4351a

Browse files
author
Richard Guo
committed
Expand virtual generated columns in the planner
Commit 83ea6c5 added support for virtual generated columns that are computed on read. All Var nodes in the query that reference virtual generated columns must be replaced with the corresponding generation expressions. Currently, this replacement occurs in the rewriter. However, this approach has several issues. If a Var referencing a virtual generated column has any varnullingrels, those varnullingrels need to be propagated into the generation expression. Failing to do so can lead to "wrong varnullingrels" errors and improper outer-join removal. Additionally, if such a Var comes from the nullable side of an outer join, we may need to wrap the generation expression in a PlaceHolderVar to ensure that it is evaluated at the right place and hence is forced to null when the outer join should do so. In certain cases, such as when the query uses grouping sets, we also need a PlaceHolderVar for anything that is not a simple Var to isolate subexpressions. Failure to do so can result in incorrect results. To fix these issues, this patch expands the virtual generated columns in the planner rather than in the rewriter, and leverages the pullup_replace_vars architecture to avoid code duplication. The generation expressions will be correctly marked with nullingrel bits and wrapped in PlaceHolderVars when needed by the pullup_replace_vars callback function. This requires handling the OLD/NEW RETURNING list Vars in pullup_replace_vars_callback, as it may now deal with Vars referencing the result relation instead of a subquery. The "wrong varnullingrels" error was reported by Alexander Lakhin. The incorrect result issue and the improper outer-join removal issue were reported by Richard Guo. Author: Richard Guo <guofenglinux@gmail.com> Author: Dean Rasheed <dean.a.rasheed@gmail.com> Reviewed-by: Jian He <jian.universality@gmail.com> Discussion: https://postgr.es/m/75eb1a6f-d59f-42e6-8a78-124ee808cda7@gmail.com
1 parent 560a842 commit 1e4351a

File tree

10 files changed

+445
-46
lines changed

10 files changed

+445
-46
lines changed

‎src/backend/optimizer/plan/planner.c

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -734,6 +734,14 @@ subquery_planner(PlannerGlobal *glob, Query *parse, PlannerInfo *parent_root,
734734
*/
735735
preprocess_function_rtes(root);
736736

737+
/*
738+
* Scan the rangetable for relations with virtual generated columns, and
739+
* replace all Var nodes in the query that reference these columns with
740+
* the generation expressions. Recursion issues here are handled in the
741+
* same way as for SubLinks.
742+
*/
743+
parse = root->parse = expand_virtual_generated_columns(root);
744+
737745
/*
738746
* Check to see if any subqueries in the jointree can be merged into this
739747
* query.

‎src/backend/optimizer/prep/prepjointree.c

Lines changed: 196 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
* replace_empty_jointree
88
* pull_up_sublinks
99
* preprocess_function_rtes
10+
* expand_virtual_generated_columns
1011
* pull_up_subqueries
1112
* flatten_simple_union_all
1213
* do expression preprocessing (including flattening JOIN alias vars)
@@ -25,6 +26,7 @@
2526
*/
2627
#include "postgres.h"
2728

29+
#include "access/table.h"
2830
#include "catalog/pg_type.h"
2931
#include "funcapi.h"
3032
#include "miscadmin.h"
@@ -39,7 +41,9 @@
3941
#include "optimizer/tlist.h"
4042
#include "parser/parse_relation.h"
4143
#include "parser/parsetree.h"
44+
#include "rewrite/rewriteHandler.h"
4245
#include "rewrite/rewriteManip.h"
46+
#include "utils/rel.h"
4347

4448

4549
typedef struct nullingrel_info
@@ -58,6 +62,8 @@ typedef struct pullup_replace_vars_context
5862
PlannerInfo *root;
5963
List *targetlist; /* tlist of subquery being pulled up */
6064
RangeTblEntry *target_rte; /* RTE of subquery */
65+
int result_relation; /* the index of the result relation in the
66+
* rewritten query */
6167
Relids relids; /* relids within subquery, as numbered after
6268
* pullup (set only if target_rte->lateral) */
6369
nullingrel_info *nullinfo; /* per-RTE nullingrel info (set only if
@@ -916,6 +922,133 @@ preprocess_function_rtes(PlannerInfo *root)
916922
}
917923
}
918924

925+
/*
926+
* expand_virtual_generated_columns
927+
* Expand all virtual generated column references in a query.
928+
*
929+
* This scans the rangetable for relations with virtual generated columns, and
930+
* replaces all Var nodes in the query that reference these columns with the
931+
* generation expressions. Note that we do not descend into subqueries; that
932+
* is taken care of when the subqueries are planned.
933+
*
934+
* This has to be done after we have pulled up any SubLinks within the query's
935+
* quals; otherwise any virtual generated column references within the SubLinks
936+
* that should be transformed into joins wouldn't get expanded.
937+
*
938+
* Returns a modified copy of the query tree, if any relations with virtual
939+
* generated columns are present.
940+
*/
941+
Query *
942+
expand_virtual_generated_columns(PlannerInfo *root)
943+
{
944+
Query *parse = root->parse;
945+
int rt_index;
946+
ListCell *lc;
947+
948+
rt_index = 0;
949+
foreach(lc, parse->rtable)
950+
{
951+
RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc);
952+
Relation rel;
953+
TupleDesc tupdesc;
954+
955+
++rt_index;
956+
957+
/*
958+
* Only normal relations can have virtual generated columns.
959+
*/
960+
if (rte->rtekind != RTE_RELATION)
961+
continue;
962+
963+
rel = table_open(rte->relid, NoLock);
964+
965+
tupdesc = RelationGetDescr(rel);
966+
if (tupdesc->constr && tupdesc->constr->has_generated_virtual)
967+
{
968+
List *tlist = NIL;
969+
pullup_replace_vars_context rvcontext;
970+
971+
for (int i = 0; i < tupdesc->natts; i++)
972+
{
973+
Form_pg_attribute attr = TupleDescAttr(tupdesc, i);
974+
TargetEntry *tle;
975+
976+
if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
977+
{
978+
Node *defexpr;
979+
980+
defexpr = build_generation_expression(rel, i + 1);
981+
ChangeVarNodes(defexpr, 1, rt_index, 0);
982+
983+
tle = makeTargetEntry((Expr *) defexpr, i + 1, 0, false);
984+
tlist = lappend(tlist, tle);
985+
}
986+
else
987+
{
988+
Var *var;
989+
990+
var = makeVar(rt_index,
991+
i + 1,
992+
attr->atttypid,
993+
attr->atttypmod,
994+
attr->attcollation,
995+
0);
996+
997+
tle = makeTargetEntry((Expr *) var, i + 1, 0, false);
998+
tlist = lappend(tlist, tle);
999+
}
1000+
}
1001+
1002+
Assert(list_length(tlist) > 0);
1003+
Assert(!rte->lateral);
1004+
1005+
/*
1006+
* The relation's targetlist items are now in the appropriate form
1007+
* to insert into the query, except that we may need to wrap them
1008+
* in PlaceHolderVars. Set up required context data for
1009+
* pullup_replace_vars.
1010+
*/
1011+
rvcontext.root = root;
1012+
rvcontext.targetlist = tlist;
1013+
rvcontext.target_rte = rte;
1014+
rvcontext.result_relation = parse->resultRelation;
1015+
/* won't need these values */
1016+
rvcontext.relids = NULL;
1017+
rvcontext.nullinfo = NULL;
1018+
/* pass NULL for outer_hasSubLinks */
1019+
rvcontext.outer_hasSubLinks = NULL;
1020+
rvcontext.varno = rt_index;
1021+
/* this flag will be set below, if needed */
1022+
rvcontext.wrap_non_vars = false;
1023+
/* initialize cache array with indexes 0 .. length(tlist) */
1024+
rvcontext.rv_cache = palloc0((list_length(tlist) + 1) *
1025+
sizeof(Node *));
1026+
1027+
/*
1028+
* If the query uses grouping sets, we need a PlaceHolderVar for
1029+
* anything that's not a simple Var. Again, this ensures that
1030+
* expressions retain their separate identity so that they will
1031+
* match grouping set columns when appropriate. (It'd be
1032+
* sufficient to wrap values used in grouping set columns, and do
1033+
* so only in non-aggregated portions of the tlist and havingQual,
1034+
* but that would require a lot of infrastructure that
1035+
* pullup_replace_vars hasn't currently got.)
1036+
*/
1037+
if (parse->groupingSets)
1038+
rvcontext.wrap_non_vars = true;
1039+
1040+
/*
1041+
* Apply pullup variable replacement throughout the query tree.
1042+
*/
1043+
parse = (Query *) pullup_replace_vars((Node *) parse, &rvcontext);
1044+
}
1045+
1046+
table_close(rel, NoLock);
1047+
}
1048+
1049+
return parse;
1050+
}
1051+
9191052
/*
9201053
* pull_up_subqueries
9211054
* Look for subqueries in the rangetable that can be pulled up into
@@ -1197,6 +1330,13 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
11971330
*/
11981331
preprocess_function_rtes(subroot);
11991332

1333+
/*
1334+
* Scan the rangetable for relations with virtual generated columns, and
1335+
* replace all Var nodes in the query that reference these columns with
1336+
* the generation expressions.
1337+
*/
1338+
subquery = subroot->parse = expand_virtual_generated_columns(subroot);
1339+
12001340
/*
12011341
* Recursively pull up the subquery's subqueries, so that
12021342
* pull_up_subqueries' processing is complete for its jointree and
@@ -1274,6 +1414,7 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
12741414
rvcontext.root = root;
12751415
rvcontext.targetlist = subquery->targetList;
12761416
rvcontext.target_rte = rte;
1417+
rvcontext.result_relation = 0;
12771418
if (rte->lateral)
12781419
{
12791420
rvcontext.relids = get_relids_in_jointree((Node *) subquery->jointree,
@@ -1834,6 +1975,7 @@ pull_up_simple_values(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte)
18341975
rvcontext.root = root;
18351976
rvcontext.targetlist = tlist;
18361977
rvcontext.target_rte = rte;
1978+
rvcontext.result_relation = 0;
18371979
rvcontext.relids = NULL; /* can't be any lateral references here */
18381980
rvcontext.nullinfo = NULL;
18391981
rvcontext.outer_hasSubLinks = &parse->hasSubLinks;
@@ -1993,6 +2135,7 @@ pull_up_constant_function(PlannerInfo *root, Node *jtnode,
19932135
NULL, /* resname */
19942136
false)); /* resjunk */
19952137
rvcontext.target_rte = rte;
2138+
rvcontext.result_relation = 0;
19962139

19972140
/*
19982141
* Since this function was reduced to a Const, it doesn't contain any
@@ -2490,6 +2633,10 @@ pullup_replace_vars_callback(Var *var,
24902633
bool need_phv;
24912634
Node *newnode;
24922635

2636+
/* System columns are not replaced. */
2637+
if (varattno < InvalidAttrNumber)
2638+
return (Node *) copyObject(var);
2639+
24932640
/*
24942641
* We need a PlaceHolderVar if the Var-to-be-replaced has nonempty
24952642
* varnullingrels (unless we find below that the replacement expression is
@@ -2559,6 +2706,22 @@ pullup_replace_vars_callback(Var *var,
25592706
rowexpr->location = var->location;
25602707
newnode = (Node *) rowexpr;
25612708

2709+
/* Handle any OLD/NEW RETURNING list Vars */
2710+
if (var->varreturningtype != VAR_RETURNING_DEFAULT)
2711+
{
2712+
/*
2713+
* Wrap the RowExpr in a ReturningExpr node, so that the executor
2714+
* returns NULL if the OLD/NEW row does not exist.
2715+
*/
2716+
ReturningExpr *rexpr = makeNode(ReturningExpr);
2717+
2718+
rexpr->retlevelsup = 0;
2719+
rexpr->retold = (var->varreturningtype == VAR_RETURNING_OLD);
2720+
rexpr->retexpr = (Expr *) newnode;
2721+
2722+
newnode = (Node *) rexpr;
2723+
}
2724+
25622725
/*
25632726
* Insert PlaceHolderVar if needed. Notice that we are wrapping one
25642727
* PlaceHolderVar around the whole RowExpr, rather than putting one
@@ -2588,6 +2751,39 @@ pullup_replace_vars_callback(Var *var,
25882751
/* Make a copy of the tlist item to return */
25892752
newnode = (Node *) copyObject(tle->expr);
25902753

2754+
/* Handle any OLD/NEW RETURNING list Vars */
2755+
if (var->varreturningtype != VAR_RETURNING_DEFAULT)
2756+
{
2757+
/*
2758+
* Copy varreturningtype onto any Vars in the tlist item that
2759+
* refer to result_relation (which had better be non-zero).
2760+
*/
2761+
if (rcon->result_relation == 0)
2762+
elog(ERROR, "variable returning old/new found outside RETURNING list");
2763+
2764+
SetVarReturningType((Node *) newnode, rcon->result_relation,
2765+
0, var->varreturningtype);
2766+
2767+
/*
2768+
* If the replacement expression in the targetlist is not simply a
2769+
* Var referencing result_relation, wrap it in a ReturningExpr
2770+
* node, so that the executor returns NULL if the OLD/NEW row does
2771+
* not exist.
2772+
*/
2773+
if (!IsA(newnode, Var) ||
2774+
((Var *) newnode)->varno != rcon->result_relation ||
2775+
((Var *) newnode)->varlevelsup != 0)
2776+
{
2777+
ReturningExpr *rexpr = makeNode(ReturningExpr);
2778+
2779+
rexpr->retlevelsup = 0;
2780+
rexpr->retold = (var->varreturningtype == VAR_RETURNING_OLD);
2781+
rexpr->retexpr = (Expr *) newnode;
2782+
2783+
newnode = (Node *) rexpr;
2784+
}
2785+
}
2786+
25912787
/* Insert PlaceHolderVar if needed */
25922788
if (need_phv)
25932789
{

0 commit comments

Comments
 (0)