Remove rewriteTargetListIU's expansion of view targetlists in UPDATE.

tglsfdc · tglsfdc · commit 04942bffd0aa · 2021-04-26T13:58:00.000-04:00
Commit 2ec993a, which added triggers on views, modified the rewriter to add dummy entries like "SET x = x" for all columns that weren't actually being updated by the user in any UPDATE directed at a view. That was needed at the time to produce a complete "NEW" row to pass to the trigger. Later it was found to cause problems for ordinary updatable views, so commit cab5dc5 restricted it to happen only for trigger-updatable views. But in the wake of commit 86dc900, we really don't need it at all. nodeModifyTable.c populates the trigger "OLD" row from the whole-row variable that is generated for the view, and then it computes the "NEW" row using that old row and the UPDATE targetlist. So there is no need for the UPDATE tlist to have dummy entries, any more than it needs them for regular tables or other types of views. (The comments for rewriteTargetListIU suggest that we must do this for correct expansion of NEW references in rules, but I now think that that was just lazy comment editing in 2ec993a. If we didn't need it for rules on views before there were triggers, we don't need it after that.) This essentially propagates 86dc900's decision that we don't need dummy column updates into the view case. Aside from making the different cases more uniform and hence possibly forestalling future bugs, it ought to save a little bit of rewriter/planner effort. Discussion: https://postgr.es/m/2181213.1619397634@sss.pgh.pa.us
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
@@ -679,30 +679,19 @@ adjustJoinTreeList(Query *parsetree, bool removert, int rt_index)
  * and UPDATE, replace explicit DEFAULT specifications with column default
  * expressions.
  *
- * 2. For an UPDATE on a trigger-updatable view, add tlist entries for any
- * unassigned-to attributes, assigning them their old values.  These will
- * later get expanded to the output values of the view.  This is only needed
- * for trigger-updatable views, for which the view remains the result relation
- * of the query; without it, we would not have a complete "new tuple" to pass
- * to triggers.  For auto-updatable views we must not do this, since it might
- * add assignments to non-updatable view columns.  For rule-updatable views it
- * is unnecessary extra work, since the query will be rewritten with a
- * different result relation which will be processed when we recurse via
- * RewriteQuery.
- *
- * 3. Merge multiple entries for the same target attribute, or declare error
+ * 2. Merge multiple entries for the same target attribute, or declare error
  * if we can't.  Multiple entries are only allowed for INSERT/UPDATE of
  * portions of an array or record field, for example
  *			UPDATE table SET foo[2] = 42, foo[4] = 43;
  * We can merge such operations into a single assignment op.  Essentially,
  * the expression we want to produce in this case is like
  *		foo = array_set_element(array_set_element(foo, 2, 42), 4, 43)
  *
- * 4. Sort the tlist into standard order: non-junk fields in order by resno,
+ * 3. Sort the tlist into standard order: non-junk fields in order by resno,
  * then junk fields (these in no particular order).
  *
- * We must do items 1,2,3 before firing rewrite rules, else rewritten
- * references to NEW.foo will produce wrong or incomplete results.  Item 4
+ * We must do items 1 and 2 before firing rewrite rules, else rewritten
+ * references to NEW.foo will produce wrong or incomplete results.  Item 3
  * is not needed for rewriting, but it is helpful for the planner, and we
  * can do it essentially for free while handling the other items.
  *
@@ -984,29 +973,6 @@ rewriteTargetListIU(List *targetList,
 										  false);
 		}
 
-		/*
-		 * For an UPDATE on a trigger-updatable view, provide a dummy entry
-		 * whenever there is no explicit assignment.
-		 */
-		if (new_tle == NULL && commandType == CMD_UPDATE &&
-			target_relation->rd_rel->relkind == RELKIND_VIEW &&
-			view_has_instead_trigger(target_relation, CMD_UPDATE))
-		{
-			Node	   *new_expr;
-
-			new_expr = (Node *) makeVar(result_rti,
-										attrno,
-										att_tup->atttypid,
-										att_tup->atttypmod,
-										att_tup->attcollation,
-										0);
-
-			new_tle = makeTargetEntry((Expr *) new_expr,
-									  attrno,
-									  pstrdup(NameStr(att_tup->attname)),
-									  false);
-		}
-
 		if (new_tle)
 			new_tlist = lappend(new_tlist, new_tle);
 	}
