v6-0002-Use-ReplaceVarsFromTargetList-for-rewriting-virtu.patch

text/plain

Filename: v6-0002-Use-ReplaceVarsFromTargetList-for-rewriting-virtu.patch
Type: text/plain
Part: 1
Message: Re: Virtual generated columns

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: format-patch
Series: patch v6-0002
Subject: Use ReplaceVarsFromTargetList() for rewriting virtual columns
File+
src/backend/parser/analyze.c 0 8
src/backend/parser/parse_clause.c 0 4
src/backend/parser/parse_merge.c 0 1
src/backend/parser/parse_relation.c 0 3
src/backend/rewrite/rewriteHandler.c 54 131
src/include/nodes/parsenodes.h 0 2
src/include/parser/parse_node.h 0 1
From 7bfe4fc9ce81124d9e7fdc24a1f46099bcaec726 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Wed, 4 Sep 2024 09:38:42 +0200
Subject: [PATCH v6 2/2] Use ReplaceVarsFromTargetList() for rewriting virtual
 columns

---
 src/backend/parser/analyze.c         |   8 --
 src/backend/parser/parse_clause.c    |   4 -
 src/backend/parser/parse_merge.c     |   1 -
 src/backend/parser/parse_relation.c  |   3 -
 src/backend/rewrite/rewriteHandler.c | 185 ++++++++-------------------
 src/include/nodes/parsenodes.h       |   2 -
 src/include/parser/parse_node.h      |   1 -
 7 files changed, 54 insertions(+), 150 deletions(-)

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index e3936a0b8ea..e901203424d 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -568,7 +568,6 @@ transformDeleteStmt(ParseState *pstate, DeleteStmt *stmt)
 	qry->hasWindowFuncs = pstate->p_hasWindowFuncs;
 	qry->hasTargetSRFs = pstate->p_hasTargetSRFs;
 	qry->hasAggs = pstate->p_hasAggs;
-	qry->hasGeneratedVirtual = pstate->p_hasGeneratedVirtual;
 
 	assign_query_collations(pstate, qry);
 
@@ -995,7 +994,6 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt)
 
 	qry->hasTargetSRFs = pstate->p_hasTargetSRFs;
 	qry->hasSubLinks = pstate->p_hasSubLinks;
-	qry->hasGeneratedVirtual = pstate->p_hasGeneratedVirtual;
 
 	assign_query_collations(pstate, qry);
 
@@ -1461,7 +1459,6 @@ transformSelectStmt(ParseState *pstate, SelectStmt *stmt)
 	qry->hasWindowFuncs = pstate->p_hasWindowFuncs;
 	qry->hasTargetSRFs = pstate->p_hasTargetSRFs;
 	qry->hasAggs = pstate->p_hasAggs;
-	qry->hasGeneratedVirtual = pstate->p_hasGeneratedVirtual;
 
 	foreach(l, stmt->lockingClause)
 	{
@@ -1688,7 +1685,6 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt)
 	qry->jointree = makeFromExpr(pstate->p_joinlist, NULL);
 
 	qry->hasSubLinks = pstate->p_hasSubLinks;
-	qry->hasGeneratedVirtual = pstate->p_hasGeneratedVirtual;
 
 	assign_query_collations(pstate, qry);
 
@@ -1940,7 +1936,6 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
 	qry->hasWindowFuncs = pstate->p_hasWindowFuncs;
 	qry->hasTargetSRFs = pstate->p_hasTargetSRFs;
 	qry->hasAggs = pstate->p_hasAggs;
-	qry->hasGeneratedVirtual = pstate->p_hasGeneratedVirtual;
 
 	foreach(l, lockingClause)
 	{
@@ -2415,7 +2410,6 @@ transformReturnStmt(ParseState *pstate, ReturnStmt *stmt)
 	qry->hasWindowFuncs = pstate->p_hasWindowFuncs;
 	qry->hasTargetSRFs = pstate->p_hasTargetSRFs;
 	qry->hasAggs = pstate->p_hasAggs;
-	qry->hasGeneratedVirtual = pstate->p_hasGeneratedVirtual;
 
 	assign_query_collations(pstate, qry);
 
@@ -2483,7 +2477,6 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt)
 
 	qry->hasTargetSRFs = pstate->p_hasTargetSRFs;
 	qry->hasSubLinks = pstate->p_hasSubLinks;
-	qry->hasGeneratedVirtual = pstate->p_hasGeneratedVirtual;
 
 	assign_query_collations(pstate, qry);
 
@@ -2850,7 +2843,6 @@ transformPLAssignStmt(ParseState *pstate, PLAssignStmt *stmt)
 	qry->hasWindowFuncs = pstate->p_hasWindowFuncs;
 	qry->hasTargetSRFs = pstate->p_hasTargetSRFs;
 	qry->hasAggs = pstate->p_hasAggs;
-	qry->hasGeneratedVirtual = pstate->p_hasGeneratedVirtual;
 
 	foreach(l, sstmt->lockingClause)
 	{
diff --git a/src/backend/parser/parse_clause.c b/src/backend/parser/parse_clause.c
index 48524ac3fc2..8118036495b 100644
--- a/src/backend/parser/parse_clause.c
+++ b/src/backend/parser/parse_clause.c
@@ -207,10 +207,6 @@ setTargetTable(ParseState *pstate, RangeVar *relation,
 	pstate->p_target_relation = parserOpenTable(pstate, relation,
 												RowExclusiveLock);
 
-	if (pstate->p_target_relation->rd_att->constr &&
-		pstate->p_target_relation->rd_att->constr->has_generated_virtual)
-		pstate->p_hasGeneratedVirtual = true;
-
 	/*
 	 * Now build an RTE and a ParseNamespaceItem.
 	 */
diff --git a/src/backend/parser/parse_merge.c b/src/backend/parser/parse_merge.c
index 350e9e18885..87df79027d7 100644
--- a/src/backend/parser/parse_merge.c
+++ b/src/backend/parser/parse_merge.c
@@ -405,7 +405,6 @@ transformMergeStmt(ParseState *pstate, MergeStmt *stmt)
 
 	qry->hasTargetSRFs = false;
 	qry->hasSubLinks = pstate->p_hasSubLinks;
-	qry->hasGeneratedVirtual = pstate->p_hasGeneratedVirtual;
 
 	assign_query_collations(pstate, qry);
 
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index b864749f09c..7a3199ae1d3 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -1515,9 +1515,6 @@ addRangeTableEntry(ParseState *pstate,
 	rte->eref = makeAlias(refname, NIL);
 	buildRelationAliases(rel->rd_att, alias, rte->eref);
 
-	if (rel->rd_att->constr && rel->rd_att->constr->has_generated_virtual)
-		pstate->p_hasGeneratedVirtual = true;
-
 	/*
 	 * Set flags and initialize access permissions.
 	 *
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index f2713eaef23..0241d471571 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -44,7 +44,6 @@
 #include "utils/builtins.h"
 #include "utils/lsyscache.h"
 #include "utils/rel.h"
-#include "utils/syscache.h"
 
 
 /* We use a list of these to detect recursion in RewriteQuery */
@@ -91,8 +90,7 @@ static List *matchLocks(CmdType event, Relation relation,
 						int varno, Query *parsetree, bool *hasUpdate);
 static Query *fireRIRrules(Query *parsetree, List *activeRIRs);
 static Bitmapset *adjust_view_column_set(Bitmapset *cols, List *targetlist);
-struct expand_generated_context;
-static Query *expand_generated_columns_in_query(Query *query, struct expand_generated_context *context);
+static Node *expand_generated_columns_internal(Node *node, Relation rel, int rt_index, RangeTblEntry *rte);
 
 
 /*
@@ -2139,6 +2137,9 @@ fireRIRrules(Query *parsetree, List *activeRIRs)
 			}
 		}
 
+		/* Expand virtual generated columns of this table */
+		parsetree = (Query *) expand_generated_columns_internal((Node *) parsetree, rel, rt_index, rte);
+
 		table_close(rel, NoLock);
 	}
 
@@ -4370,84 +4371,66 @@ RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length)
 
 
 /*
- * Virtual generated columns support
+ * Expand virtual generated columns
+ *
+ * If the table contains virtual generated columns, build a target list
+ * containing the expanded expressions and use ReplaceVarsFromTargetList() to
+ * do the replacements.
  */
-
-struct expand_generated_context
-{
-	/* list of range tables, innermost last */
-	List	   *rtables;
-
-	/* incremented for every level where it's true */
-	int			ancestor_has_virtual;
-};
-
 static Node *
-expand_generated_columns_mutator(Node *node, struct expand_generated_context *context)
+expand_generated_columns_internal(Node *node, Relation rel, int rt_index, RangeTblEntry *rte)
 {
-	if (node == NULL)
-		return NULL;
+	TupleDesc	tupdesc;
 
-	if (IsA(node, Var))
+	tupdesc = RelationGetDescr(rel);
+	if (tupdesc->constr && tupdesc->constr->has_generated_virtual)
 	{
-		Var		   *v = (Var *) node;
-		Oid			relid;
-		AttrNumber	attnum;
-		List	   *rtable = list_nth_node(List,
-										   context->rtables,
-										   list_length(context->rtables) - v->varlevelsup - 1);
-
-		relid = rt_fetch(v->varno, rtable)->relid;
-		attnum = v->varattno;
+		List	   *tlist = NIL;
 
-		if (!relid || !attnum)
-			return node;
-
-		if (get_attgenerated(relid, attnum) == ATTRIBUTE_GENERATED_VIRTUAL)
+		for (int i = 0; i < tupdesc->natts; i++)
 		{
-			Relation	rt_entry_relation = table_open(relid, NoLock);
-			Oid			attcollid;
-
-			node = build_column_default(rt_entry_relation, attnum);
-			if (node == NULL)
-				elog(ERROR, "no generation expression found for column number %d of table \"%s\"",
-					 attnum, RelationGetRelationName(rt_entry_relation));
+			Form_pg_attribute attr = TupleDescAttr(tupdesc, i);
 
-			/*
-			 * 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 = GetSysCacheOid(ATTNUM, Anum_pg_attribute_attcollation, relid, attnum, 0, 0);
-			if (attcollid && attcollid != exprCollation(node))
+			if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
 			{
-				CollateExpr *ce = makeNode(CollateExpr);
+				Node	   *defexpr;
+				int			attnum = i + 1;
+				Oid			attcollid;
+				TargetEntry *te;
 
-				ce->arg = (Expr *) node;
-				ce->collOid = attcollid;
-				ce->location = -1;
+				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));
 
-				node = (Node *) ce;
-			}
+				/*
+				 * 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);
 
-			IncrementVarSublevelsUp(node, v->varlevelsup, 0);
-			ChangeVarNodes(node, 1, v->varno, v->varlevelsup);
+					ce->arg = (Expr *) defexpr;
+					ce->collOid = attcollid;
+					ce->location = -1;
 
-			table_close(rt_entry_relation, NoLock);
-		}
+					defexpr = (Node *) ce;
+				}
 
-		return node;
-	}
-	else if (IsA(node, Query))
-	{
-		Query	   *query = (Query *) node;
+				ChangeVarNodes(defexpr, 1, rt_index, 0);
 
-		query = expand_generated_columns_in_query(query, context);
+				te = makeTargetEntry((Expr *) defexpr, attnum, 0, false);
+				tlist = lappend(tlist, te);
+			}
+		}
 
-		return (Node *) query;
+		node = ReplaceVarsFromTargetList(node, rt_index, 0, rte, tlist, REPLACEVARS_CHANGE_VARNO, rt_index, NULL);
 	}
-	else
-		return expression_tree_mutator(node, expand_generated_columns_mutator, context);
+
+	return node;
 }
 
 Node *
@@ -4458,76 +4441,19 @@ expand_generated_columns_in_expr(Node *node, Relation rel)
 	if (tupdesc->constr && tupdesc->constr->has_generated_virtual)
 	{
 		RangeTblEntry *rte;
-		List	   *rtable;
-		struct expand_generated_context context;
 
-		/*
-		 * Make a dummy range table for a single relation.  For the benefit of
-		 * triggers, add the same entry twice, so it covers PRS2_OLD_VARNO and
-		 * PRS2_NEW_VARNO.
-		 */
 		rte = makeNode(RangeTblEntry);
 		rte->relid = RelationGetRelid(rel);
-		rtable = list_make2(rte, rte);
-		context.rtables = list_make1(rtable);
-
-		return expression_tree_mutator(node, expand_generated_columns_mutator, &context);
-	}
-	else
-		return node;
-}
-
-/*
- * Expand virtual generated columns in a Query.  We do some optimizations here
- * to avoid digging through the whole Query unless necessary.
- */
-static Query *
-expand_generated_columns_in_query(Query *query, struct expand_generated_context *context)
-{
-	context->rtables = lappend(context->rtables, query->rtable);
-	if (query->hasGeneratedVirtual)
-		context->ancestor_has_virtual++;
-
-	/*
-	 * If any table in the query has a virtual column or there is a sublink,
-	 * then we need to do the whole walk.
-	 */
-	if (query->hasGeneratedVirtual || query->hasSubLinks || context->ancestor_has_virtual)
-	{
-		query = query_tree_mutator(query,
-								   expand_generated_columns_mutator,
-								   context,
-								   QTW_DONT_COPY_QUERY);
-	}
-
-	/*
-	 * Else we only need to process subqueries.
-	 */
-	else
-	{
-		ListCell   *lc;
-
-		foreach(lc, query->rtable)
-		{
-			RangeTblEntry *rte = (RangeTblEntry *) lfirst(lc);
-
-			if (rte->rtekind == RTE_SUBQUERY)
-				rte->subquery = expand_generated_columns_in_query(rte->subquery, context);
-		}
 
-		foreach(lc, query->cteList)
-		{
-			CommonTableExpr *cte = (CommonTableExpr *) lfirst(lc);
-
-			cte->ctequery = (Node *) expand_generated_columns_in_query(castNode(Query, cte->ctequery), context);
-		}
+		/*
+		 * XXX For the benefit of triggers, make two passes, so it covers
+		 * PRS2_OLD_VARNO and PRS2_NEW_VARNO.
+		 */
+		node = expand_generated_columns_internal(node, rel, 1, rte);
+		node = expand_generated_columns_internal(node, rel, 2, rte);
 	}
 
-	if (query->hasGeneratedVirtual)
-		context->ancestor_has_virtual--;
-	context->rtables = list_truncate(context->rtables, list_length(context->rtables) - 1);
-
-	return query;
+	return node;
 }
 
 
@@ -4575,12 +4501,9 @@ QueryRewrite(Query *parsetree)
 	foreach(l, querylist)
 	{
 		Query	   *query = (Query *) lfirst(l);
-		struct expand_generated_context context = {0};
 
 		query = fireRIRrules(query, NIL);
 
-		query = expand_generated_columns_in_query(query, &context);
-
 		query->queryId = input_query_id;
 
 		results = lappend(results, query);
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 355f52199fa..0e91c084a01 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -160,8 +160,6 @@ typedef struct Query
 	bool		hasForUpdate pg_node_attr(query_jumble_ignore);
 	/* rewriter has applied some RLS policy */
 	bool		hasRowSecurity pg_node_attr(query_jumble_ignore);
-	/* some table has a virtual generated column */
-	bool		hasGeneratedVirtual pg_node_attr(query_jumble_ignore);
 	/* is a RETURN statement */
 	bool		isReturn pg_node_attr(query_jumble_ignore);
 
diff --git a/src/include/parser/parse_node.h b/src/include/parser/parse_node.h
index 1b02128548b..5b781d87a9d 100644
--- a/src/include/parser/parse_node.h
+++ b/src/include/parser/parse_node.h
@@ -225,7 +225,6 @@ struct ParseState
 	bool		p_hasTargetSRFs;
 	bool		p_hasSubLinks;
 	bool		p_hasModifyingCTE;
-	bool		p_hasGeneratedVirtual;
 
 	Node	   *p_last_srf;		/* most recent set-returning func/op found */
 
-- 
2.46.0