v1-0001-vgc-where-current-of-fix.patch

application/octet-stream

Filename: v1-0001-vgc-where-current-of-fix.patch
Type: application/octet-stream
Part: 0
Message: [BUG]: WHERE CURRENT OF cursor fail on tables that have 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: unified
Series: patch v1-0001
File+
src/backend/optimizer/prep/prepjointree.c 16 5
src/backend/rewrite/rewriteHandler.c 6 4
src/backend/rewrite/rewriteManip.c 76 10
src/include/rewrite/rewriteManip.h 16 1
src/test/regress/expected/generated_virtual.out 40 0
src/test/regress/sql/generated_virtual.sql 21 0
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 95bf5160..654e3768 100644
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -82,6 +82,7 @@ typedef struct pullup_replace_vars_context
 	int			varno;			/* varno of subquery */
 	ReplaceWrapOption wrap_option;	/* do we need certain outputs to be PHVs? */
 	Node	  **rv_cache;		/* cache for results with PHVs */
+	bool		error_on_current_of;	/* error on CurrentOfExpr? */
 } pullup_replace_vars_context;
 
 typedef struct reduce_outer_joins_pass1_state
@@ -566,6 +567,12 @@ expand_virtual_generated_columns(PlannerInfo *root, Query *parse,
 		if (parse->groupingSets)
 			rvcontext.wrap_option = REPLACE_WRAP_ALL;
 
+		/*
+		 * Don't error on CurrentOfExpr during VGC expansion; the table is
+		 * real, not a view, so WHERE CURRENT OF is legitimate.
+		 */
+		rvcontext.error_on_current_of = false;
+
 		/*
 		 * Apply pullup variable replacement throughout the query tree.
 		 */
@@ -1579,6 +1586,7 @@ pull_up_simple_subquery(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte,
 	/* initialize cache array with indexes 0 .. length(tlist) */
 	rvcontext.rv_cache = palloc0((list_length(subquery->targetList) + 1) *
 								 sizeof(Node *));
+	rvcontext.error_on_current_of = true;
 
 	/*
 	 * If the parent query uses grouping sets, we need a PlaceHolderVar for
@@ -2124,6 +2132,7 @@ pull_up_simple_values(PlannerInfo *root, Node *jtnode, RangeTblEntry *rte)
 	/* initialize cache array with indexes 0 .. length(tlist) */
 	rvcontext.rv_cache = palloc0((list_length(tlist) + 1) *
 								 sizeof(Node *));
+	rvcontext.error_on_current_of = true;
 
 	/*
 	 * Replace all of the top query's references to the RTE's outputs with
@@ -2292,6 +2301,7 @@ pull_up_constant_function(PlannerInfo *root, Node *jtnode,
 	/* initialize cache array with indexes 0 .. length(tlist) */
 	rvcontext.rv_cache = palloc0((list_length(rvcontext.targetlist) + 1) *
 								 sizeof(Node *));
+	rvcontext.error_on_current_of = true;
 
 	/*
 	 * If the parent query uses grouping sets, we need a PlaceHolderVar for
@@ -2754,11 +2764,12 @@ replace_vars_in_jointree(Node *jtnode,
 static Node *
 pullup_replace_vars(Node *expr, pullup_replace_vars_context *context)
 {
-	return replace_rte_variables(expr,
-								 context->varno, 0,
-								 pullup_replace_vars_callback,
-								 context,
-								 context->outer_hasSubLinks);
+	return replace_rte_variables_ext(expr,
+									 context->varno, 0,
+									 pullup_replace_vars_callback,
+									 context,
+									 context->outer_hasSubLinks,
+									 context->error_on_current_of);
 }
 
 static Node *
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index 021c73f1..97f94715 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -4574,10 +4574,12 @@ expand_generated_columns_internal(Node *node, Relation rel, int rt_index,
 
 		Assert(list_length(tlist) > 0);
 
-		node = ReplaceVarsFromTargetList(node, rt_index, 0, rte, tlist,
-										 result_relation,
-										 REPLACEVARS_CHANGE_VARNO, rt_index,
-										 NULL);
+		node = ReplaceVarsFromTargetListExtended(node, rt_index, 0,
+												rte, tlist,
+												result_relation,
+												REPLACEVARS_CHANGE_VARNO,
+												rt_index,
+												NULL, false);
 	}
 
 	return node;
diff --git a/src/backend/rewrite/rewriteManip.c b/src/backend/rewrite/rewriteManip.c
index 4bf4aa0d..5b6872ef 100644
--- a/src/backend/rewrite/rewriteManip.c
+++ b/src/backend/rewrite/rewriteManip.c
@@ -1443,11 +1443,12 @@ remove_nulling_relids_mutator(Node *node,
  * convenient to recurse directly to the mutator on sub-expressions of
  * what they will return.
  */
-Node *
-replace_rte_variables(Node *node, int target_varno, int sublevels_up,
-					  replace_rte_variables_callback callback,
-					  void *callback_arg,
-					  bool *outer_hasSubLinks)
+static Node *
+replace_rte_variables_internal(Node *node, int target_varno, int sublevels_up,
+							   replace_rte_variables_callback callback,
+							   void *callback_arg,
+							   bool *outer_hasSubLinks,
+							   bool error_on_current_of)
 {
 	Node	   *result;
 	replace_rte_variables_context context;
@@ -1456,6 +1457,7 @@ replace_rte_variables(Node *node, int target_varno, int sublevels_up,
 	context.callback_arg = callback_arg;
 	context.target_varno = target_varno;
 	context.sublevels_up = sublevels_up;
+	context.error_on_current_of = error_on_current_of;
 
 	/*
 	 * We try to initialize inserted_sublink to true if there is no need to
@@ -1490,6 +1492,37 @@ replace_rte_variables(Node *node, int target_varno, int sublevels_up,
 	return result;
 }
 
+Node *
+replace_rte_variables(Node *node, int target_varno, int sublevels_up,
+					  replace_rte_variables_callback callback,
+					  void *callback_arg,
+					  bool *outer_hasSubLinks)
+{
+	return replace_rte_variables_internal(node, target_varno, sublevels_up,
+										  callback, callback_arg,
+										  outer_hasSubLinks, true);
+}
+
+/*
+ * replace_rte_variables_ext -
+ *		As above, but with control over CurrentOfExpr error behavior.
+ *
+ * If error_on_current_of is false, CurrentOfExpr nodes referencing the
+ * target relation will be left unchanged rather than raising an error.
+ */
+Node *
+replace_rte_variables_ext(Node *node, int target_varno, int sublevels_up,
+						  replace_rte_variables_callback callback,
+						  void *callback_arg,
+						  bool *outer_hasSubLinks,
+						  bool error_on_current_of)
+{
+	return replace_rte_variables_internal(node, target_varno, sublevels_up,
+										  callback, callback_arg,
+										  outer_hasSubLinks,
+										  error_on_current_of);
+}
+
 Node *
 replace_rte_variables_mutator(Node *node,
 							  replace_rte_variables_context *context)
@@ -1519,7 +1552,8 @@ replace_rte_variables_mutator(Node *node,
 		CurrentOfExpr *cexpr = (CurrentOfExpr *) node;
 
 		if (cexpr->cvarno == context->target_varno &&
-			context->sublevels_up == 0)
+			context->sublevels_up == 0 &&
+			context->error_on_current_of)
 		{
 			/*
 			 * We get here if a WHERE CURRENT OF expression turns out to apply
@@ -1978,6 +2012,37 @@ ReplaceVarsFromTargetList(Node *node,
 						  ReplaceVarsNoMatchOption nomatch_option,
 						  int nomatch_varno,
 						  bool *outer_hasSubLinks)
+{
+	return ReplaceVarsFromTargetListExtended(node, target_varno, sublevels_up,
+											target_rte, targetlist,
+											result_relation, nomatch_option,
+											nomatch_varno, outer_hasSubLinks,
+											true);
+}
+
+/*
+ * ReplaceVarsFromTargetListExtended -
+ *		As above, but with additional control over CurrentOfExpr handling.
+ *
+ * If error_on_current_of is true (the default for ReplaceVarsFromTargetList),
+ * an error is raised if a CurrentOfExpr is found referencing the target
+ * relation.  This is appropriate for view rewriting, where WHERE CURRENT OF
+ * cannot be translated to the underlying table.
+ *
+ * If error_on_current_of is false, CurrentOfExpr nodes are left unchanged.
+ * This is used by virtual generated column expansion, where we are only
+ * replacing column references, not changing the target relation.
+ */
+Node *
+ReplaceVarsFromTargetListExtended(Node *node,
+								  int target_varno, int sublevels_up,
+								  RangeTblEntry *target_rte,
+								  List *targetlist,
+								  int result_relation,
+								  ReplaceVarsNoMatchOption nomatch_option,
+								  int nomatch_varno,
+								  bool *outer_hasSubLinks,
+								  bool error_on_current_of)
 {
 	ReplaceVarsFromTargetList_context context;
 
@@ -1987,8 +2052,9 @@ ReplaceVarsFromTargetList(Node *node,
 	context.nomatch_option = nomatch_option;
 	context.nomatch_varno = nomatch_varno;
 
-	return replace_rte_variables(node, target_varno, sublevels_up,
-								 ReplaceVarsFromTargetList_callback,
-								 &context,
-								 outer_hasSubLinks);
+	return replace_rte_variables_internal(node, target_varno, sublevels_up,
+										  ReplaceVarsFromTargetList_callback,
+										  &context,
+										  outer_hasSubLinks,
+										  error_on_current_of);
 }
diff --git a/src/include/rewrite/rewriteManip.h b/src/include/rewrite/rewriteManip.h
index a6d4e888..eb66b714 100644
--- a/src/include/rewrite/rewriteManip.h
+++ b/src/include/rewrite/rewriteManip.h
@@ -32,6 +32,7 @@ struct replace_rte_variables_context
 	int			target_varno;	/* RTE index to search for */
 	int			sublevels_up;	/* (current) nesting depth */
 	bool		inserted_sublink;	/* have we inserted a SubLink? */
+	bool		error_on_current_of;	/* error on CurrentOfExpr? */
 };
 
 typedef enum ReplaceVarsNoMatchOption
@@ -96,6 +97,12 @@ extern Node *replace_rte_variables(Node *node,
 								   replace_rte_variables_callback callback,
 								   void *callback_arg,
 								   bool *outer_hasSubLinks);
+extern Node *replace_rte_variables_ext(Node *node,
+									   int target_varno, int sublevels_up,
+									   replace_rte_variables_callback callback,
+									   void *callback_arg,
+									   bool *outer_hasSubLinks,
+									   bool error_on_current_of);
 extern Node *replace_rte_variables_mutator(Node *node,
 										   replace_rte_variables_context *context);
 
@@ -118,5 +125,13 @@ extern Node *ReplaceVarsFromTargetList(Node *node,
 									   ReplaceVarsNoMatchOption nomatch_option,
 									   int nomatch_varno,
 									   bool *outer_hasSubLinks);
-
+extern Node *ReplaceVarsFromTargetListExtended(Node *node,
+											   int target_varno, int sublevels_up,
+											   RangeTblEntry *target_rte,
+											   List *targetlist,
+											   int result_relation,
+											   ReplaceVarsNoMatchOption nomatch_option,
+											   int nomatch_varno,
+											   bool *outer_hasSubLinks,
+											   bool error_on_current_of);
 #endif							/* REWRITEMANIP_H */
diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out
index fc41c480..c238ef8a 100644
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -1723,3 +1723,43 @@ select * from gtest33 where b is null;
 
 reset constraint_exclusion;
 drop table gtest33;
+create table gtest_cursor (id int primary key, a int, b int generated always as (a * 2) virtual);
+insert into gtest_cursor values (1, 10), (2, 20), (3, 30);
+-- UPDATE via cursor
+begin;
+declare cur1 cursor for select * from gtest_cursor order by id for update;
+fetch 1 from cur1;
+ id | a  | b  
+----+----+----
+  1 | 10 | 20
+(1 row)
+
+update gtest_cursor set a = 99 where current of cur1;
+select * from gtest_cursor order by id;
+ id | a  |  b  
+----+----+-----
+  1 | 99 | 198
+  2 | 20 |  40
+  3 | 30 |  60
+(3 rows)
+
+commit;
+-- DELETE via cursor
+begin;
+declare cur2 cursor for select * from gtest_cursor order by id for update;
+fetch 1 from cur2;
+ id | a  |  b  
+----+----+-----
+  1 | 99 | 198
+(1 row)
+
+delete from gtest_cursor where current of cur2;
+select * from gtest_cursor order by id;
+ id | a  | b  
+----+----+----
+  2 | 20 | 40
+  3 | 30 | 60
+(2 rows)
+
+commit;
+drop table gtest_cursor;
diff --git a/src/test/regress/sql/generated_virtual.sql b/src/test/regress/sql/generated_virtual.sql
index 9b32413e..5bab7d35 100644
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -906,3 +906,24 @@ select * from gtest33 where b is null;
 
 reset constraint_exclusion;
 drop table gtest33;
+
+create table gtest_cursor (id int primary key, a int, b int generated always as (a * 2) virtual);
+insert into gtest_cursor values (1, 10), (2, 20), (3, 30);
+
+-- UPDATE via cursor
+begin;
+declare cur1 cursor for select * from gtest_cursor order by id for update;
+fetch 1 from cur1;
+update gtest_cursor set a = 99 where current of cur1;
+select * from gtest_cursor order by id;
+commit;
+
+-- DELETE via cursor
+begin;
+declare cur2 cursor for select * from gtest_cursor order by id for update;
+fetch 1 from cur2;
+delete from gtest_cursor where current of cur2;
+select * from gtest_cursor order by id;
+commit;
+
+drop table gtest_cursor;