On Wed, 19 Feb 2025 at 01:42, Dean Rasheed <dean.a.rash...@gmail.com> wrote:
>
> One thing I don't like about this is that it's introducing more code
> duplication between pullup_replace_vars() and
> ReplaceVarsFromTargetList(). Those already had a lot of code in common
> before RETURNING OLD/NEW was added, and this is duplicating even more
> code. I think it'd be better to refactor so that they share common
> code, since it has become quite complex, and it would be better to
> have just one place to maintain. Attached is an updated patch doing
> that.
>
I've been doing some more testing of this, and attached is another
update, improving a few comments and adding regression tests based on
the cases discussed so far here.
One of the new regression tests fails, which actually appears to be a
pre-existing grouping sets bug, due to the fact that only non-Vars are
wrapped in PHVs. This bug can be triggered without virtual generated
columns:
CREATE TABLE t (a int, b int);
INSERT INTO t VALUES (1, 1);
SELECT * FROM (SELECT a, a AS b FROM t) AS vt
GROUP BY GROUPING SETS (a, b)
HAVING b = 1;
a | b
---+---
1 |
(1 row)
whereas the result should be
a | b
---+---
| 1
(1 row)
For reference, this code dates back to 90947674fc.
Regards,
Dean
From 057d8a8f63d7b6b5d8efb5d4f519b8557e264a36 Mon Sep 17 00:00:00 2001
From: Dean Rasheed <dean.a.rash...@gmail.com>
Date: Wed, 19 Feb 2025 10:30:33 +0000
Subject: [PATCH v4] Expand virtual generated columns in the planner.
Commit 83ea6c54025 added support for virtual generated columns, which
were expanded in the rewriter, in a similar way to view columns.
However, that does not propagate varnullingrels onto the replacement
expressions, which is OK for view columns because they are expanded
inside a subquery, but it is not OK for virtual generated columns
which are expanded directly into the main query and so must be
correctly marked. This lead to various problems with outer join
queries, including "wrong varnullingrels" errors, incorrect results,
and improper outer-join removal.
To fix this, expanded virtual generated columns must be properly
marked, based on the original Var's varnullingrels, and if necessary,
wrapped in PlaceHolderVars. Since PlaceHolderVar is a planner node,
this really has to be done in the planner. To avoid a lot of code
duplication, reuse the planner's subquery-pullup code for this.
Since this now means that the subquery-pullup code may be dealing with
Vars referring to the result relation rather than a subquery, it must
also be taught to handle Vars with non-default varreturningtype
(OLD/NEW references in the RETURNING list), which previously only the
rewriter had to worry about. To avoid even more code duplication
between this code and the rewriter, arrange for pullup_replace_vars()
to reuse ReplaceVarsFromTargetList()'s mutator function. This
eliminates a lot of existing duplicated code between the rewriter and
the planner, and avoids adding even more.
---
src/backend/optimizer/plan/planner.c | 6 +
src/backend/optimizer/prep/prepjointree.c | 262 +++++++++++++++-------
src/backend/rewrite/rewriteHandler.c | 23 +-
src/backend/rewrite/rewriteManip.c | 50 ++++-
src/include/optimizer/prep.h | 1 +
src/include/rewrite/rewriteManip.h | 7 +
src/test/regress/expected/join.out | 66 ++++++
src/test/regress/sql/join.sql | 43 ++++
8 files changed, 351 insertions(+), 107 deletions(-)
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index 7b1a8a0a9f1..6beb58e8f04 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -749,6 +749,12 @@ subquery_planner(PlannerGlobal *glob, Query *parse, PlannerInfo *parent_root,
if (parse->setOperations)
flatten_simple_union_all(root);
+ /*
+ * Scan the rangetable for relations with virtual generated columns, and
+ * expand them.
+ */
+ parse = root->parse = expand_virtual_generated_columns(root);
+
/*
* Survey the rangetable to see what kinds of entries are present. We can
* skip some later processing if relevant SQL features are not used; for
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 5d9225e9909..8fdca35d087 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -9,6 +9,7 @@
* preprocess_function_rtes
* pull_up_subqueries
* flatten_simple_union_all
+ * expand_virtual_generated_columns
* do expression preprocessing (including flattening JOIN alias vars)
* reduce_outer_joins
* remove_useless_result_rtes
@@ -25,6 +26,7 @@
*/
#include "postgres.h"
+#include "access/table.h"
#include "catalog/pg_type.h"
#include "funcapi.h"
#include "miscadmin.h"
@@ -39,7 +41,9 @@
#include "optimizer/tlist.h"
#include "parser/parse_relation.h"
#include "parser/parsetree.h"
+#include "rewrite/rewriteHandler.h"
#include "rewrite/rewriteManip.h"
+#include "utils/rel.h"
typedef struct nullingrel_info
@@ -57,6 +61,7 @@ typedef struct pullup_replace_vars_context
{
PlannerInfo *root;
List *targetlist; /* tlist of subquery being pulled up */
+ int result_relation; /* the index of the result relation */
RangeTblEntry *target_rte; /* RTE of subquery */
Relids relids; /* relids within subquery, as numbered after
* pullup (set only if target_rte->lateral) */
@@ -1273,6 +1278,7 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
*/
rvcontext.root = root;
rvcontext.targetlist = subquery->targetList;
+ rvcontext.result_relation = 0;
rvcontext.target_rte = rte;
if (rte->lateral)
{
@@ -1833,6 +1839,7 @@ pull_up_simple_values(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte)
}
rvcontext.root = root;
rvcontext.targetlist = tlist;
+ rvcontext.result_relation = 0;
rvcontext.target_rte = rte;
rvcontext.relids = NULL; /* can't be any lateral references here */
rvcontext.nullinfo = NULL;
@@ -1992,6 +1999,7 @@ pull_up_constant_function(PlannerInfo *root, Node *jtnode,
1, /* resno */
NULL, /* resname */
false)); /* resjunk */
+ rvcontext.result_relation = 0;
rvcontext.target_rte = rte;
/*
@@ -2490,6 +2498,10 @@ pullup_replace_vars_callback(Var *var,
bool need_phv;
Node *newnode;
+ /* System columns are not replaced. */
+ if (varattno < InvalidAttrNumber)
+ return (Node *) copyObject(var);
+
/*
* We need a PlaceHolderVar if the Var-to-be-replaced has nonempty
* varnullingrels (unless we find below that the replacement expression is
@@ -2516,85 +2528,43 @@ pullup_replace_vars_callback(Var *var,
varattno <= list_length(rcon->targetlist) &&
rcon->rv_cache[varattno] != NULL)
{
- /* Just copy the entry and fall through to adjust phlevelsup etc */
+ /* Copy the cached item and adjust its varlevelsup */
newnode = copyObject(rcon->rv_cache[varattno]);
+ if (var->varlevelsup > 0)
+ IncrementVarSublevelsUp(newnode, var->varlevelsup, 0);
}
- else if (varattno == InvalidAttrNumber)
+ else
{
- /* Must expand whole-tuple reference into RowExpr */
- RowExpr *rowexpr;
- List *colnames;
- List *fields;
- bool save_wrap_non_vars = rcon->wrap_non_vars;
- int save_sublevelsup = context->sublevels_up;
-
/*
- * If generating an expansion for a var of a named rowtype (ie, this
- * is a plain relation RTE), then we must include dummy items for
- * dropped columns. If the var is RECORD (ie, this is a JOIN), then
- * omit dropped columns. In the latter case, attach column names to
- * the RowExpr for use of the executor and ruleutils.c.
- *
- * In order to be able to cache the results, we always generate the
- * expansion with varlevelsup = 0, and then adjust below if needed.
+ * Generate the replacement expression. This takes care of expanding
+ * wholerow references, as well as adjusting varlevelsup and dealing
+ * with non-default varreturningtype.
*/
- expandRTE(rcon->target_rte,
- var->varno, 0 /* not varlevelsup */ ,
- var->varreturningtype, var->location,
- (var->vartype != RECORDOID),
- &colnames, &fields);
- /* Expand the generated per-field Vars, but don't insert PHVs there */
- rcon->wrap_non_vars = false;
- context->sublevels_up = 0; /* to match the expandRTE output */
- fields = (List *) replace_rte_variables_mutator((Node *) fields,
- context);
- rcon->wrap_non_vars = save_wrap_non_vars;
- context->sublevels_up = save_sublevelsup;
-
- rowexpr = makeNode(RowExpr);
- rowexpr->args = fields;
- rowexpr->row_typeid = var->vartype;
- rowexpr->row_format = COERCE_IMPLICIT_CAST;
- rowexpr->colnames = (var->vartype == RECORDOID) ? colnames : NIL;
- rowexpr->location = var->location;
- newnode = (Node *) rowexpr;
-
- /*
- * Insert PlaceHolderVar if needed. Notice that we are wrapping one
- * PlaceHolderVar around the whole RowExpr, rather than putting one
- * around each element of the row. This is because we need the
- * expression to yield NULL, not ROW(NULL,NULL,...) when it is forced
- * to null by an outer join.
- */
- if (need_phv)
- {
- newnode = (Node *)
- make_placeholder_expr(rcon->root,
- (Expr *) newnode,
- bms_make_singleton(rcon->varno));
- /* cache it with the PHV, and with phlevelsup etc not set yet */
- rcon->rv_cache[InvalidAttrNumber] = copyObject(newnode);
- }
- }
- else
- {
- /* Normal case referencing one targetlist element */
- TargetEntry *tle = get_tle_by_resno(rcon->targetlist, varattno);
-
- if (tle == NULL) /* shouldn't happen */
- elog(ERROR, "could not find attribute %d in subquery targetlist",
- varattno);
-
- /* Make a copy of the tlist item to return */
- newnode = (Node *) copyObject(tle->expr);
+ newnode = ReplaceVarFromTargetList(var,
+ rcon->target_rte,
+ rcon->targetlist,
+ rcon->result_relation,
+ REPLACEVARS_REPORT_ERROR,
+ 0);
/* Insert PlaceHolderVar if needed */
if (need_phv)
{
bool wrap;
- if (newnode && IsA(newnode, Var) &&
- ((Var *) newnode)->varlevelsup == 0)
+ if (varattno == InvalidAttrNumber)
+ {
+ /*
+ * A wholerow Var will have been expanded to a RowExpr, which
+ * we wrap with a single PlaceHolderVar, rather than putting
+ * one around each element of the row. This is because we need
+ * the expression to yield NULL, not ROW(NULL,NULL,...) when
+ * it is forced to null by an outer join.
+ */
+ wrap = true;
+ }
+ else if (newnode && IsA(newnode, Var) &&
+ ((Var *) newnode)->varlevelsup == var->varlevelsup)
{
/*
* Simple Vars always escape being wrapped, unless they are
@@ -2616,7 +2586,7 @@ pullup_replace_vars_callback(Var *var,
}
}
else if (newnode && IsA(newnode, PlaceHolderVar) &&
- ((PlaceHolderVar *) newnode)->phlevelsup == 0)
+ ((PlaceHolderVar *) newnode)->phlevelsup == var->varlevelsup)
{
/* The same rules apply for a PlaceHolderVar */
wrap = false;
@@ -2735,14 +2705,25 @@ pullup_replace_vars_callback(Var *var,
make_placeholder_expr(rcon->root,
(Expr *) newnode,
bms_make_singleton(rcon->varno));
+ ((PlaceHolderVar *) newnode)->phlevelsup = var->varlevelsup;
/*
* Cache it if possible (ie, if the attno is in range, which
- * it probably always should be).
+ * it probably always should be), ensuring that the cached
+ * item has phlevelsup = 0.
*/
- if (varattno > InvalidAttrNumber &&
+ if (varattno >= InvalidAttrNumber &&
varattno <= list_length(rcon->targetlist))
- rcon->rv_cache[varattno] = copyObject(newnode);
+ {
+ Node *cachenode = copyObject(newnode);
+
+ if (var->varlevelsup > 0)
+ IncrementVarSublevelsUp(cachenode,
+ -((int) var->varlevelsup),
+ 0);
+
+ rcon->rv_cache[varattno] = cachenode;
+ }
}
}
}
@@ -2754,7 +2735,7 @@ pullup_replace_vars_callback(Var *var,
{
Var *newvar = (Var *) newnode;
- Assert(newvar->varlevelsup == 0);
+ Assert(newvar->varlevelsup == var->varlevelsup);
newvar->varnullingrels = bms_add_members(newvar->varnullingrels,
var->varnullingrels);
}
@@ -2762,7 +2743,7 @@ pullup_replace_vars_callback(Var *var,
{
PlaceHolderVar *newphv = (PlaceHolderVar *) newnode;
- Assert(newphv->phlevelsup == 0);
+ Assert(newphv->phlevelsup == var->varlevelsup);
newphv->phnullingrels = bms_add_members(newphv->phnullingrels,
var->varnullingrels);
}
@@ -2825,10 +2806,6 @@ pullup_replace_vars_callback(Var *var,
}
}
- /* Must adjust varlevelsup if replaced Var is within a subquery */
- if (var->varlevelsup > 0)
- IncrementVarSublevelsUp(newnode, var->varlevelsup, 0);
-
return newnode;
}
@@ -2947,6 +2924,135 @@ flatten_simple_union_all(PlannerInfo *root)
}
+/*
+ * expand_virtual_generated_columns
+ * Expand all virtual generated column references in a query.
+ *
+ * This scans the rangetable for relations with virtual generated columns, and
+ * replaces all Var nodes in the query that refer to such columns with the
+ * appropriate expressions. Note that this happens after subquery pullup, and
+ * it does not recurse into any remaining subqueries; that is taken care of
+ * when those subqueries are planned.
+ *
+ * Returns a modified copy of the query tree, if any relations with virtual
+ * generated columns are present.
+ */
+Query *
+expand_virtual_generated_columns(PlannerInfo *root)
+{
+ Query *parse = root->parse;
+ int rt_index;
+ ListCell *lc;
+
+ rt_index = 0;
+ foreach(lc, parse->rtable)
+ {
+ RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc);
+ Relation rel;
+ TupleDesc tupdesc;
+
+ ++rt_index;
+
+ /*
+ * Only normal relations can have virtual generated columns.
+ */
+ if (rte->rtekind != RTE_RELATION)
+ continue;
+
+ rel = table_open(rte->relid, NoLock);
+
+ tupdesc = RelationGetDescr(rel);
+ if (tupdesc->constr && tupdesc->constr->has_generated_virtual)
+ {
+ List *tlist = NIL;
+ pullup_replace_vars_context rvcontext;
+
+ for (int i = 0; i < tupdesc->natts; i++)
+ {
+ Form_pg_attribute attr = TupleDescAttr(tupdesc, i);
+ int attnum = i + 1;
+ TargetEntry *te;
+
+ if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
+ {
+ Node *defexpr;
+ Oid attcollid;
+
+ defexpr = build_column_default(rel, attnum);
+ if (defexpr == NULL)
+ elog(ERROR, "no generation expression found for column number %d of table \"%s\"",
+ attnum, RelationGetRelationName(rel));
+
+ /*
+ * If the column definition has a collation and it is
+ * different from the collation of the generation
+ * expression, put a COLLATE clause around the expression.
+ */
+ attcollid = attr->attcollation;
+ if (attcollid && attcollid != exprCollation(defexpr))
+ {
+ CollateExpr *ce = makeNode(CollateExpr);
+
+ ce->arg = (Expr *) defexpr;
+ ce->collOid = attcollid;
+ ce->location = -1;
+
+ defexpr = (Node *) ce;
+ }
+
+ ChangeVarNodes(defexpr, 1, rt_index, 0);
+
+ te = makeTargetEntry((Expr *) defexpr, attnum, 0, false);
+ tlist = lappend(tlist, te);
+ }
+ else
+ {
+ Var *var;
+
+ var = makeVar(rt_index,
+ attnum,
+ attr->atttypid,
+ attr->atttypmod,
+ attr->attcollation,
+ 0);
+ te = makeTargetEntry((Expr *) var, attnum, 0, false);
+ tlist = lappend(tlist, te);
+ }
+ }
+
+ Assert(list_length(tlist) > 0);
+
+ rvcontext.root = root;
+ rvcontext.targetlist = tlist;
+ rvcontext.result_relation = parse->resultRelation;
+ rvcontext.target_rte = rte;
+ rvcontext.relids = NULL;
+ rvcontext.nullinfo = NULL;
+ rvcontext.outer_hasSubLinks = NULL;
+ rvcontext.varno = rt_index;
+ rvcontext.wrap_non_vars = false;
+ rvcontext.rv_cache = palloc0((list_length(tlist) + 1) *
+ sizeof(Node *));
+
+ /*
+ * If the query uses grouping sets, we need a PlaceHolderVar for
+ * anything that's not a simple Var. This ensures that
+ * expressions retain their separate identity so that they will
+ * match grouping set columns when appropriate.
+ */
+ if (parse->groupingSets)
+ rvcontext.wrap_non_vars = true;
+
+ parse = (Query *) pullup_replace_vars((Node *) parse, &rvcontext);
+ }
+
+ table_close(rel, NoLock);
+ }
+
+ return parse;
+}
+
+
/*
* reduce_outer_joins
* Attempt to reduce outer joins to plain inner joins.
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index e996bdc0d21..7a39abd4d86 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -2190,10 +2190,6 @@ fireRIRrules(Query *parsetree, List *activeRIRs)
* requires special recursion detection if the new quals have sublink
* subqueries, and if we did it in the loop above query_tree_walker would
* then recurse into those quals a second time.
- *
- * Finally, we expand any virtual generated columns. We do this after
- * each table's RLS policies are applied because the RLS policies might
- * also refer to the table's virtual generated columns.
*/
rt_index = 0;
foreach(lc, parsetree->rtable)
@@ -2207,11 +2203,10 @@ fireRIRrules(Query *parsetree, List *activeRIRs)
++rt_index;
- /*
- * Only normal relations can have RLS policies or virtual generated
- * columns.
- */
- if (rte->rtekind != RTE_RELATION)
+ /* Only normal relations can have RLS policies */
+ if (rte->rtekind != RTE_RELATION ||
+ (rte->relkind != RELKIND_RELATION &&
+ rte->relkind != RELKIND_PARTITIONED_TABLE))
continue;
rel = table_open(rte->relid, NoLock);
@@ -2300,16 +2295,6 @@ fireRIRrules(Query *parsetree, List *activeRIRs)
if (hasSubLinks)
parsetree->hasSubLinks = true;
- /*
- * Expand any references to virtual generated columns of this table.
- * Note that subqueries in virtual generated column expressions are
- * not currently supported, so this cannot add any more sublinks.
- */
- parsetree = (Query *)
- expand_generated_columns_internal((Node *) parsetree,
- rel, rt_index, rte,
- parsetree->resultRelation);
-
table_close(rel, NoLock);
}
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index 9433548d279..e81624f9b09 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -1814,6 +1814,23 @@ ReplaceVarsFromTargetList_callback(Var *var,
replace_rte_variables_context *context)
{
ReplaceVarsFromTargetList_context *rcon = (ReplaceVarsFromTargetList_context *) context->callback_arg;
+
+ return ReplaceVarFromTargetList(var,
+ rcon->target_rte,
+ rcon->targetlist,
+ rcon->result_relation,
+ rcon->nomatch_option,
+ rcon->nomatch_varno);
+}
+
+Node *
+ReplaceVarFromTargetList(Var *var,
+ RangeTblEntry *target_rte,
+ List *targetlist,
+ int result_relation,
+ ReplaceVarsNoMatchOption nomatch_option,
+ int nomatch_varno)
+{
TargetEntry *tle;
if (var->varattno == InvalidAttrNumber)
@@ -1822,6 +1839,7 @@ ReplaceVarsFromTargetList_callback(Var *var,
RowExpr *rowexpr;
List *colnames;
List *fields;
+ ListCell *lc;
/*
* If generating an expansion for a var of a named rowtype (ie, this
@@ -1833,15 +1851,27 @@ ReplaceVarsFromTargetList_callback(Var *var,
* The varreturningtype is copied onto each individual field Var, so
* that it is handled correctly when we recurse.
*/
- expandRTE(rcon->target_rte,
+ expandRTE(target_rte,
var->varno, var->varlevelsup, var->varreturningtype,
var->location, (var->vartype != RECORDOID),
&colnames, &fields);
/* Adjust the generated per-field Vars... */
- fields = (List *) replace_rte_variables_mutator((Node *) fields,
- context);
rowexpr = makeNode(RowExpr);
- rowexpr->args = fields;
+ rowexpr->args = NIL;
+ foreach(lc, fields)
+ {
+ Node *field = lfirst(lc);
+
+ if (field != NULL)
+ field = ReplaceVarFromTargetList((Var *) field,
+ target_rte,
+ targetlist,
+ result_relation,
+ nomatch_option,
+ nomatch_varno);
+
+ rowexpr->args = lappend(rowexpr->args, field);
+ }
rowexpr->row_typeid = var->vartype;
rowexpr->row_format = COERCE_IMPLICIT_CAST;
rowexpr->colnames = (var->vartype == RECORDOID) ? colnames : NIL;
@@ -1863,12 +1893,12 @@ ReplaceVarsFromTargetList_callback(Var *var,
}
/* Normal case referencing one targetlist element */
- tle = get_tle_by_resno(rcon->targetlist, var->varattno);
+ tle = get_tle_by_resno(targetlist, var->varattno);
if (tle == NULL || tle->resjunk)
{
/* Failed to find column in targetlist */
- switch (rcon->nomatch_option)
+ switch (nomatch_option)
{
case REPLACEVARS_REPORT_ERROR:
/* fall through, throw error below */
@@ -1876,7 +1906,7 @@ ReplaceVarsFromTargetList_callback(Var *var,
case REPLACEVARS_CHANGE_VARNO:
var = copyObject(var);
- var->varno = rcon->nomatch_varno;
+ var->varno = nomatch_varno;
/* we leave the syntactic referent alone */
return (Node *) var;
@@ -1932,15 +1962,15 @@ ReplaceVarsFromTargetList_callback(Var *var,
* Copy varreturningtype onto any Vars in the tlist item that
* refer to result_relation (which had better be non-zero).
*/
- if (rcon->result_relation == 0)
+ if (result_relation == 0)
elog(ERROR, "variable returning old/new found outside RETURNING list");
- SetVarReturningType((Node *) newnode, rcon->result_relation,
+ SetVarReturningType((Node *) newnode, result_relation,
var->varlevelsup, var->varreturningtype);
/* Wrap it in a ReturningExpr, if needed, per comments above */
if (!IsA(newnode, Var) ||
- ((Var *) newnode)->varno != rcon->result_relation ||
+ ((Var *) newnode)->varno != result_relation ||
((Var *) newnode)->varlevelsup != var->varlevelsup)
{
ReturningExpr *rexpr = makeNode(ReturningExpr);
diff --git a/src/include/optimizer/prep.h b/src/include/optimizer/prep.h
index 0ae57ec24a4..6bc75ac9f38 100644
--- a/src/include/optimizer/prep.h
+++ b/src/include/optimizer/prep.h
@@ -27,6 +27,7 @@ extern void pull_up_sublinks(PlannerInfo *root);
extern void preprocess_function_rtes(PlannerInfo *root);
extern void pull_up_subqueries(PlannerInfo *root);
extern void flatten_simple_union_all(PlannerInfo *root);
+extern Query *expand_virtual_generated_columns(PlannerInfo *root);
extern void reduce_outer_joins(PlannerInfo *root);
extern void remove_useless_result_rtes(PlannerInfo *root);
extern Relids get_relids_in_jointree(Node *jtnode, bool include_outer_joins,
diff --git a/src/include/rewrite/rewriteManip.h b/src/include/rewrite/rewriteManip.h
index 5ec475c63e9..8ca0face062 100644
--- a/src/include/rewrite/rewriteManip.h
+++ b/src/include/rewrite/rewriteManip.h
@@ -89,6 +89,13 @@ extern Node *map_variable_attnos(Node *node,
const struct AttrMap *attno_map,
Oid to_rowtype, bool *found_whole_row);
+extern Node *ReplaceVarFromTargetList(Var *var,
+ RangeTblEntry *target_rte,
+ List *targetlist,
+ int result_relation,
+ ReplaceVarsNoMatchOption nomatch_option,
+ int nomatch_varno);
+
extern Node *ReplaceVarsFromTargetList(Node *node,
int target_varno, int sublevels_up,
RangeTblEntry *target_rte,
diff --git a/src/test/regress/expected/join.out b/src/test/regress/expected/join.out
index a57bb18c24f..e787792ece1 100644
--- a/src/test/regress/expected/join.out
+++ b/src/test/regress/expected/join.out
@@ -3514,6 +3514,72 @@ order by c.name;
C | | |
(3 rows)
+rollback;
+--
+-- test NULL behavior of virtual generated columns
+--
+begin;
+create temp table t (
+ a int primary key,
+ b int generated always as (1 + 1),
+ c int generated always as (a),
+ d int generated always as (a * 10),
+ e int generated always as (coalesce(a, 100))
+);
+insert into t values (1), (2);
+-- test propagtion of varnullingrels
+select sum(t2.b) over (partition by t2.a),
+ sum(t2.c) over (partition by t2.a),
+ sum(t2.d) over (partition by t2.a),
+ sum(t2.e) over (partition by t2.a)
+from t as t1 left join t as t2 on (t1.a = t2.a)
+order by t1.a;
+ sum | sum | sum | sum
+-----+-----+-----+-----
+ 2 | 1 | 10 | 1
+ 2 | 2 | 20 | 2
+(2 rows)
+
+select t1.*, t2.*
+from t as t1 left join t as t2 on false
+order by t1.a;
+ a | b | c | d | e | a | b | c | d | e
+---+---+---+----+---+---+---+---+---+---
+ 1 | 2 | 1 | 10 | 1 | | | | |
+ 2 | 2 | 2 | 20 | 2 | | | | |
+(2 rows)
+
+explain (costs off)
+select t1.a
+from t as t1 left join t as t2 on (t1.a = t2.a)
+where coalesce(t2.d, 1) = 1;
+ QUERY PLAN
+------------------------------------------
+ Hash Left Join
+ Hash Cond: (t1.a = t2.a)
+ Filter: (COALESCE((t2.a * 10), 1) = 1)
+ -> Seq Scan on t t1
+ -> Hash
+ -> Seq Scan on t t2
+(6 rows)
+
+-- grouping sets requires PlaceHolderVar for non-Var clauses
+select * from t
+group by grouping sets (a, b, c, d, e)
+having b = 2;
+ a | b | c | d | e
+---+---+---+---+---
+ | 2 | | |
+(1 row)
+
+select * from t
+group by grouping sets (a, b, c, d, e)
+having c = 2;
+ a | b | c | d | e
+---+---+---+---+---
+ | | 2 | |
+(1 row)
+
rollback;
--
-- test incorrect handling of placeholders that only appear in targetlists,
diff --git a/src/test/regress/sql/join.sql b/src/test/regress/sql/join.sql
index c29d13b9fed..828cbdfd9db 100644
--- a/src/test/regress/sql/join.sql
+++ b/src/test/regress/sql/join.sql
@@ -1044,6 +1044,49 @@ order by c.name;
rollback;
+--
+-- test NULL behavior of virtual generated columns
+--
+begin;
+
+create temp table t (
+ a int primary key,
+ b int generated always as (1 + 1),
+ c int generated always as (a),
+ d int generated always as (a * 10),
+ e int generated always as (coalesce(a, 100))
+);
+
+insert into t values (1), (2);
+
+-- test propagtion of varnullingrels
+select sum(t2.b) over (partition by t2.a),
+ sum(t2.c) over (partition by t2.a),
+ sum(t2.d) over (partition by t2.a),
+ sum(t2.e) over (partition by t2.a)
+from t as t1 left join t as t2 on (t1.a = t2.a)
+order by t1.a;
+
+select t1.*, t2.*
+from t as t1 left join t as t2 on false
+order by t1.a;
+
+explain (costs off)
+select t1.a
+from t as t1 left join t as t2 on (t1.a = t2.a)
+where coalesce(t2.d, 1) = 1;
+
+-- grouping sets requires PlaceHolderVar for non-Var clauses
+select * from t
+group by grouping sets (a, b, c, d, e)
+having b = 2;
+
+select * from t
+group by grouping sets (a, b, c, d, e)
+having c = 2;
+
+rollback;
+
--
-- test incorrect handling of placeholders that only appear in targetlists,
-- per bug #6154
--
2.43.0
