Fix DEFAULT handling for multi-row INSERT rules.

When updating a relation with a rule whose action performed an INSERT
from a multi-row VALUES list, the rewriter might skip processing the
VALUES list, and therefore fail to replace any DEFAULTs in it. This
would lead to an "unrecognized node type" error.

The reason was that RewriteQuery() assumed that a query doing an
INSERT from a multi-row VALUES list would necessarily only have one
item in its fromlist, pointing to the VALUES RTE to read from. That
assumption is correct for the original query, but not for product
queries produced for rule actions. In such cases, there may be
multiple items in the fromlist, possibly including multiple VALUES
RTEs.

What is required instead is for RewriteQuery() to skip any RTEs from
the product query's originating query, which might include one or more
already-processed VALUES RTEs. What's left should then include at most
one VALUES RTE (from the rule action) to be processed.

Patch by me. Thanks to Tom Lane for reviewing.

Back-patch to all supported branches.

Discussion: https://postgr.es/m/CAEZATCV39OOW7LAR_Xq4i%2BLc1Byux%3DeK3Q%3DHD_pF1o9LBt%3DphA%40mail.gmail.com

1 files changed, 43 insertions, 10 deletions

diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
index b35a52d2ef4..45bcc84cf02 100644
--- a/src/backend/rewrite/rewriteHandler.c
+++ b/src/backend/rewrite/rewriteHandler.c
@@ -418,6 +418,10 @@ rewriteRuleAction(Query *parsetree,
 	 * NOTE: because planner will destructively alter rtable, we must ensure
 	 * that rule action's rtable is separate and shares no substructure with
 	 * the main rtable.  Hence do a deep copy here.
+	 *
+	 * Note also that RewriteQuery() relies on the fact that RT entries from
+	 * the original query appear at the start of the expanded rtable, so
+	 * beware of changing this.
 	 */
 	sub_action->rtable = list_concat(copyObject(parsetree->rtable),
 									 sub_action->rtable);
@@ -3622,9 +3626,13 @@ rewriteTargetView(Query *parsetree, Relation view)
  *
  * rewrite_events is a list of open query-rewrite actions, so we can detect
  * infinite recursion.
+ *
+ * orig_rt_length is the length of the originating query's rtable, for product
+ * queries created by fireRules(), and 0 otherwise.  This is used to skip any
+ * already-processed VALUES RTEs from the original query.
  */
 static List *
-RewriteQuery(Query *parsetree, List *rewrite_events)
+RewriteQuery(Query *parsetree, List *rewrite_events, int orig_rt_length)
 {
 	CmdType		event = parsetree->commandType;
 	bool		instead = false;
@@ -3648,7 +3656,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
 		if (ctequery->commandType == CMD_SELECT)
 			continue;
 
-		newstuff = RewriteQuery(ctequery, rewrite_events);
+		newstuff = RewriteQuery(ctequery, rewrite_events, 0);
 
 		/*
 		 * Currently we can only handle unconditional, single-statement DO
@@ -3722,6 +3730,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
 		RangeTblEntry *rt_entry;
 		Relation	rt_entry_relation;
 		List	   *locks;
+		int			product_orig_rt_length;
 		List	   *product_queries;
 		bool		hasUpdate = false;
 		int			values_rte_index = 0;
@@ -3743,23 +3752,30 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
 		 */
 		if (event == CMD_INSERT)
 		{
+			ListCell   *lc2;
 			RangeTblEntry *values_rte = NULL;
 
 			/*
-			 * If it's an INSERT ... VALUES (...), (...), ... there will be a
-			 * single RTE for the VALUES targetlists.
+			 * Test if it's a multi-row INSERT ... VALUES (...), (...), ... by
+			 * looking for a VALUES RTE in the fromlist.  For product queries,
+			 * we must ignore any already-processed VALUES RTEs from the
+			 * original query.  These appear at the start of the rangetable.
 			 */
-			if (list_length(parsetree->jointree->fromlist) == 1)
+			foreach(lc2, parsetree->jointree->fromlist)
 			{
-				RangeTblRef *rtr = (RangeTblRef *) linitial(parsetree->jointree->fromlist);
+				RangeTblRef *rtr = (RangeTblRef *) lfirst(lc2);
 
-				if (IsA(rtr, RangeTblRef))
+				if (IsA(rtr, RangeTblRef) && rtr->rtindex > orig_rt_length)
 				{
 					RangeTblEntry *rte = rt_fetch(rtr->rtindex,
 												  parsetree->rtable);
 
 					if (rte->rtekind == RTE_VALUES)
 					{
+						/* should not find more than one VALUES RTE */
+						if (values_rte != NULL)
+							elog(ERROR, "more than one VALUES RTE found");
+
 						values_rte = rte;
 						values_rte_index = rtr->rtindex;
 					}
@@ -3837,7 +3853,11 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
 						break;
 					case CMD_UPDATE:
 					case CMD_INSERT:
-						/* XXX is it possible to have a VALUES clause? */
+
+						/*
+						 * MERGE actions do not permit multi-row INSERTs, so
+						 * there is no VALUES RTE to deal with here.
+						 */
 						action->targetList =
 							rewriteTargetListIU(action->targetList,
 												action->commandType,
@@ -3864,6 +3884,7 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
 		locks = matchLocks(event, rt_entry_relation->rd_rules,
 						   result_relation, parsetree, &hasUpdate);
 
+		product_orig_rt_length = list_length(parsetree->rtable);
 		product_queries = fireRules(parsetree,
 									result_relation,
 									event,
@@ -4020,7 +4041,19 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
 				Query	   *pt = (Query *) lfirst(n);
 				List	   *newstuff;
 
-				newstuff = RewriteQuery(pt, rewrite_events);
+				/*
+				 * For an updatable view, pt might be the rewritten version of
+				 * the original query, in which case we pass on orig_rt_length
+				 * to finish processing any VALUES RTE it contained.
+				 *
+				 * Otherwise, we have a product query created by fireRules().
+				 * Any VALUES RTEs from the original query have been fully
+				 * processed, and must be skipped when we recurse.
+				 */
+				newstuff = RewriteQuery(pt, rewrite_events,
+										pt == parsetree ?
+										orig_rt_length :
+										product_orig_rt_length);
 				rewritten = list_concat(rewritten, newstuff);
 			}
 
@@ -4172,7 +4205,7 @@ QueryRewrite(Query *parsetree)
 	 *
 	 * Apply all non-SELECT rules possibly getting 0 or many queries
 	 */
-	querylist = RewriteQuery(parsetree, NIL);
+	querylist = RewriteQuery(parsetree, NIL, 0);
 
 	/*
 	 * Step 2