Rewriter and planner should use only resno, not resname, to identify

target columns in INSERT and UPDATE targetlists.  Don't rely on resname
to be accurate in ruleutils, either.  This fixes bug reported by
Donald Fraser, in which renaming a column referenced in a rule did not
work very well.

Author: tglsfdc

src/backend/access/common/tupdesc.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/access/common/tupdesc.c,v 1.98 2003/08/04 02:39:56 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/access/common/tupdesc.c,v 1.99 2003/08/11 23:04:49 tgl Exp $
  *
  * NOTES
  *	  some of the executor utility code such as "ExecTypeFromTL" should be
@@ -357,7 +357,7 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
 void
 TupleDescInitEntry(TupleDesc desc,
 				   AttrNumber attributeNumber,
-				   char *attributeName,
+				   const char *attributeName,
 				   Oid oidtypeid,
 				   int32 typmod,
 				   int attdim,
@@ -373,13 +373,6 @@ TupleDescInitEntry(TupleDesc desc,
 	AssertArg(PointerIsValid(desc));
 	AssertArg(attributeNumber >= 1);
 	AssertArg(attributeNumber <= desc->natts);
-
-	/*
-	 * attributeName's are sometimes NULL, from resdom's.  I don't know
-	 * why that is, though -- Jolly
-	 */
-/*	  AssertArg(NameIsValid(attributeName));*/
-
 	AssertArg(!PointerIsValid(desc->attrs[attributeNumber - 1]));
 
 	/*
@@ -394,6 +387,11 @@ TupleDescInitEntry(TupleDesc desc,
 	 */
 	att->attrelid = 0;			/* dummy value */
 
+	/*
+	 * Note: attributeName can be NULL, because the planner doesn't always
+	 * fill in valid resname values in targetlists, particularly for resjunk
+	 * attributes.
+	 */
 	if (attributeName != NULL)
 		namestrcpy(&(att->attname), attributeName);
 	else

src/backend/catalog/dependency.c

@@ -8,7 +8,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/catalog/dependency.c,v 1.30 2003/08/04 02:39:58 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/catalog/dependency.c,v 1.31 2003/08/11 23:04:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1439,8 +1439,8 @@ getObjectDescription(const ObjectAddress *object)
 			getRelationDescription(&buffer, object->objectId);
 			if (object->objectSubId != 0)
 				appendStringInfo(&buffer, " column %s",
-								 get_attname(object->objectId,
-											 object->objectSubId));
+								 get_relid_attribute_name(object->objectId,
+														  object->objectSubId));
 			break;
 
 		case OCLASS_PROC:

src/backend/nodes/print.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/nodes/print.c,v 1.63 2003/08/04 02:39:59 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/nodes/print.c,v 1.64 2003/08/11 23:04:49 tgl Exp $
  *
  * HISTORY
  *	  AUTHOR			DATE			MAJOR EVENT
@@ -448,7 +448,8 @@ print_tl(List *tlist, List *rtable)
 	{
 		TargetEntry *tle = lfirst(tl);
 
-		printf("\t%d %s\t", tle->resdom->resno, tle->resdom->resname);
+		printf("\t%d %s\t", tle->resdom->resno,
+			   tle->resdom->resname ? tle->resdom->resname : "<null>");
 		if (tle->resdom->ressortgroupref != 0)
 			printf("(%u):\t", tle->resdom->ressortgroupref);
 		else

src/backend/optimizer/prep/preptlist.c

@@ -15,7 +15,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/prep/preptlist.c,v 1.64 2003/08/04 02:40:01 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/prep/preptlist.c,v 1.65 2003/08/11 23:04:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -150,8 +150,6 @@ expand_targetlist(List *tlist, int command_type,
 
 			if (!resdom->resjunk && resdom->resno == attrno)
 			{
-				Assert(strcmp(resdom->resname,
-							  NameStr(att_tup->attname)) == 0);
 				new_tle = old_tle;
 				tlist = lnext(tlist);
 			}

src/backend/optimizer/prep/prepunion.c

@@ -14,7 +14,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/optimizer/prep/prepunion.c,v 1.103 2003/08/04 02:40:01 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/optimizer/prep/prepunion.c,v 1.104 2003/08/11 23:04:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -64,7 +64,7 @@ static bool tlist_same_datatypes(List *tlist, List *colTypes, bool junkOK);
 static Node *adjust_inherited_attrs_mutator(Node *node,
 							   adjust_inherited_attrs_context *context);
 static Relids adjust_relid_set(Relids relids, Index oldrelid, Index newrelid);
-static List *adjust_inherited_tlist(List *tlist, Oid new_relid);
+static List *adjust_inherited_tlist(List *tlist, Oid old_relid, Oid new_relid);
 
 
 /*
@@ -787,6 +787,7 @@ adjust_inherited_attrs(Node *node,
 			if (newnode->commandType == CMD_UPDATE)
 				newnode->targetList =
 					adjust_inherited_tlist(newnode->targetList,
+										   old_relid,
 										   new_relid);
 		}
 		return (Node *) newnode;
@@ -812,9 +813,10 @@ adjust_inherited_attrs_mutator(Node *node,
 			var->varnoold = context->new_rt_index;
 			if (var->varattno > 0)
 			{
-				char	   *attname = get_attname(context->old_relid,
-												  var->varattno);
+				char	   *attname;
 
+				attname = get_relid_attribute_name(context->old_relid,
+												   var->varattno);
 				var->varattno = get_attnum(context->new_relid, attname);
 				if (var->varattno == InvalidAttrNumber)
 					elog(ERROR, "attribute \"%s\" of relation \"%s\" does not exist",
@@ -976,7 +978,7 @@ adjust_relid_set(Relids relids, Index oldrelid, Index newrelid)
  * Note that this is not needed for INSERT because INSERT isn't inheritable.
  */
 static List *
-adjust_inherited_tlist(List *tlist, Oid new_relid)
+adjust_inherited_tlist(List *tlist, Oid old_relid, Oid new_relid)
 {
 	bool		changed_it = false;
 	List	   *tl;
@@ -989,21 +991,26 @@ adjust_inherited_tlist(List *tlist, Oid new_relid)
 	{
 		TargetEntry *tle = (TargetEntry *) lfirst(tl);
 		Resdom	   *resdom = tle->resdom;
+		char	   *attname;
 
 		if (resdom->resjunk)
 			continue;			/* ignore junk items */
 
-		attrno = get_attnum(new_relid, resdom->resname);
+		attname = get_relid_attribute_name(old_relid, resdom->resno);
+		attrno = get_attnum(new_relid, attname);
 		if (attrno == InvalidAttrNumber)
 			elog(ERROR, "attribute \"%s\" of relation \"%s\" does not exist",
-				 resdom->resname, get_rel_name(new_relid));
+				 attname, get_rel_name(new_relid));
 		if (resdom->resno != attrno)
 		{
 			resdom = (Resdom *) copyObject((Node *) resdom);
 			resdom->resno = attrno;
+			resdom->resname = attname;
 			tle->resdom = resdom;
 			changed_it = true;
 		}
+		else
+			pfree(attname);
 	}
 
 	/*

src/backend/parser/analyze.c

@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *	$Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.286 2003/08/08 21:41:55 momjian Exp $
+ *	$Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.287 2003/08/11 23:04:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -2124,10 +2124,12 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
 	{
 		Oid			colType = lfirsto(dtlist);
 		Resdom	   *leftResdom = ((TargetEntry *) lfirst(lefttl))->resdom;
-		char	   *colName = pstrdup(leftResdom->resname);
+		char	   *colName;
 		Resdom	   *resdom;
 		Expr	   *expr;
 
+		Assert(!leftResdom->resjunk);
+		colName = pstrdup(leftResdom->resname);
 		resdom = makeResdom((AttrNumber) pstate->p_next_resno++,
 							colType,
 							-1,
@@ -2501,11 +2503,12 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt)
 		{
 			/*
 			 * Resjunk nodes need no additional processing, but be sure
-			 * they have names and resnos that do not match any target
-			 * columns; else rewriter or planner might get confused.
+			 * they have resnos that do not match any target columns;
+			 * else rewriter or planner might get confused.  They don't
+			 * need a resname either.
 			 */
-			resnode->resname = "?resjunk?";
 			resnode->resno = (AttrNumber) pstate->p_next_resno++;
+			resnode->resname = NULL;
 			continue;
 		}
 		if (origTargetList == NIL)

src/backend/parser/parse_relation.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_relation.c,v 1.88 2003/08/11 20:46:46 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_relation.c,v 1.89 2003/08/11 23:04:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1516,8 +1516,6 @@ expandRelAttrs(ParseState *pstate, RangeTblEntry *rte)
 char *
 get_rte_attribute_name(RangeTblEntry *rte, AttrNumber attnum)
 {
-	char	   *attname;
-
 	if (attnum == InvalidAttrNumber)
 		return "*";
 
@@ -1535,13 +1533,7 @@ get_rte_attribute_name(RangeTblEntry *rte, AttrNumber attnum)
 	 * built (which can easily happen for rules).
 	 */
 	if (rte->rtekind == RTE_RELATION)
-	{
-		attname = get_attname(rte->relid, attnum);
-		if (attname == NULL)
-			elog(ERROR, "cache lookup failed for attribute %d of relation %u",
-				 attnum, rte->relid);
-		return attname;
-	}
+		return get_relid_attribute_name(rte->relid, attnum);
 
 	/*
 	 * Otherwise use the column name from eref.  There should always be

src/backend/parser/parse_target.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_target.c,v 1.111 2003/08/11 20:46:46 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_target.c,v 1.112 2003/08/11 23:04:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -71,7 +71,7 @@ transformTargetEntry(ParseState *pstate,
 	type_id = exprType(expr);
 	type_mod = exprTypmod(expr);
 
-	if (colname == NULL)
+	if (colname == NULL && !resjunk)
 	{
 		/*
 		 * Generate a suitable column name for a column without any
@@ -428,14 +428,19 @@ updateTargetListEntry(ParseState *pstate,
 
 	/*
 	 * The result of the target expression should now match the
-	 * destination column's type.  Also, reset the resname and resno to
-	 * identify the destination column --- rewriter and planner depend on
-	 * that!
+	 * destination column's type.
 	 */
 	resnode->restype = attrtype;
 	resnode->restypmod = attrtypmod;
-	resnode->resname = colname;
+	/*
+	 * Set the resno to identify the target column --- the rewriter and
+	 * planner depend on this.  We also set the resname to identify the
+	 * target column, but this is only for debugging purposes; it should
+	 * not be relied on.  (In particular, it might be out of date in a
+	 * stored rule.)
+	 */
 	resnode->resno = (AttrNumber) attrno;
+	resnode->resname = colname;
 }
 
 

src/backend/rewrite/rewriteHandler.c

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.128 2003/08/08 21:41:56 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.129 2003/08/11 23:04:49 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -48,7 +48,8 @@ static Query *rewriteRuleAction(Query *parsetree,
 static List *adjustJoinTreeList(Query *parsetree, bool removert, int rt_index);
 static void rewriteTargetList(Query *parsetree, Relation target_relation);
 static TargetEntry *process_matched_tle(TargetEntry *src_tle,
-					TargetEntry *prior_tle);
+										TargetEntry *prior_tle,
+										const char *attrName);
 static void markQueryForUpdate(Query *qry, bool skipOldNew);
 static List *matchLocks(CmdType event, RuleLock *rulelocks,
 		   int varno, Query *parsetree);
@@ -312,8 +313,7 @@ rewriteTargetList(Query *parsetree, Relation target_relation)
 			continue;
 
 		/*
-		 * Look for targetlist entries matching this attr.	We match by
-		 * resno, but the resname should match too.
+		 * Look for targetlist entries matching this attr.
 		 *
 		 * Junk attributes are not candidates to be matched.
 		 */
@@ -324,9 +324,8 @@ rewriteTargetList(Query *parsetree, Relation target_relation)
 
 			if (!resdom->resjunk && resdom->resno == attrno)
 			{
-				Assert(strcmp(resdom->resname,
-							  NameStr(att_tup->attname)) == 0);
-				new_tle = process_matched_tle(old_tle, new_tle);
+				new_tle = process_matched_tle(old_tle, new_tle,
+											  NameStr(att_tup->attname));
 				/* keep scanning to detect multiple assignments to attr */
 			}
 		}
@@ -424,11 +423,12 @@ rewriteTargetList(Query *parsetree, Relation target_relation)
  * Convert a matched TLE from the original tlist into a correct new TLE.
  *
  * This routine detects and handles multiple assignments to the same target
- * attribute.
+ * attribute.  (The attribute name is needed only for error messages.)
  */
 static TargetEntry *
 process_matched_tle(TargetEntry *src_tle,
-					TargetEntry *prior_tle)
+					TargetEntry *prior_tle,
+					const char *attrName)
 {
 	Resdom	   *resdom = src_tle->resdom;
 	Node	   *priorbottom;
@@ -456,7 +456,7 @@ process_matched_tle(TargetEntry *src_tle,
 		ereport(ERROR,
 				(errcode(ERRCODE_SYNTAX_ERROR),
 				 errmsg("multiple assignments to same attribute \"%s\"",
-						resdom->resname)));
+						attrName)));
 
 	/*
 	 * Prior TLE could be a nest of ArrayRefs if we do this more than
@@ -470,7 +470,7 @@ process_matched_tle(TargetEntry *src_tle,
 		ereport(ERROR,
 				(errcode(ERRCODE_SYNTAX_ERROR),
 				 errmsg("multiple assignments to same attribute \"%s\"",
-						resdom->resname)));
+						attrName)));
 
 	/*
 	 * Looks OK to nest 'em.