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.

3 files changed, 52 insertions, 47 deletions

diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c
index 49cc73f24e4..83989292d6a 100644
--- a/src/backend/utils/adt/ruleutils.c
+++ b/src/backend/utils/adt/ruleutils.c
@@ -3,7 +3,7 @@
  *				back to source text
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/adt/ruleutils.c,v 1.150 2003/08/08 21:42:09 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/adt/ruleutils.c,v 1.151 2003/08/11 23:04:49 tgl Exp $
  *
  *	  This software is copyrighted by Jan Wieck - Hamburg.
  *
@@ -207,7 +207,6 @@ static char *generate_relation_name(Oid relid);
 static char *generate_function_name(Oid funcid, int nargs, Oid *argtypes);
 static char *generate_operator_name(Oid operid, Oid arg1, Oid arg2);
 static void print_operator_name(StringInfo buf, List *opname);
-static char *get_relid_attribute_name(Oid relid, AttrNumber attnum);
 
 #define only_marker(rte)  ((rte)->inh ? "" : "ONLY ")
 
@@ -1140,7 +1139,7 @@ decompile_column_index_array(Datum column_index_array, Oid relId,
 	{
 		char	   *colName;
 
-		colName = get_attname(relId, DatumGetInt16(keys[j]));
+		colName = get_relid_attribute_name(relId, DatumGetInt16(keys[j]));
 
 		if (j == 0)
 			appendStringInfo(buf, "%s",
@@ -1901,7 +1900,6 @@ get_basic_select_query(Query *query, deparse_context *context,
 	foreach(l, query->targetList)
 	{
 		TargetEntry *tle = (TargetEntry *) lfirst(l);
-		bool		tell_as = false;
 		char	   *colname;
 
 		if (tle->resdom->resjunk)
@@ -1924,24 +1922,30 @@ get_basic_select_query(Query *query, deparse_context *context,
 		else
 			colname = tle->resdom->resname;
 
-		/* Check if we must say AS ... */
-		if (!IsA(tle->expr, Var))
-			tell_as = (strcmp(colname, "?column?") != 0);
-		else
+		if (colname)			/* resname could be NULL */
 		{
-			Var		   *var = (Var *) (tle->expr);
-			char	   *schemaname;
-			char	   *refname;
-			char	   *attname;
+			/* Check if we must say AS ... */
+			bool		tell_as;
 
-			get_names_for_var(var, context, &schemaname, &refname, &attname);
-			tell_as = (attname == NULL ||
-					   strcmp(attname, colname) != 0);
-		}
+			if (!IsA(tle->expr, Var))
+				tell_as = (strcmp(colname, "?column?") != 0);
+			else
+			{
+				Var		   *var = (Var *) (tle->expr);
+				char	   *schemaname;
+				char	   *refname;
+				char	   *attname;
 
-		/* and do if so */
-		if (tell_as)
-			appendStringInfo(buf, " AS %s", quote_identifier(colname));
+				get_names_for_var(var, context,
+								  &schemaname, &refname, &attname);
+				tell_as = (attname == NULL ||
+						   strcmp(attname, colname) != 0);
+			}
+
+			/* and do if so */
+			if (tell_as)
+				appendStringInfo(buf, " AS %s", quote_identifier(colname));
+		}
 	}
 
 	/* Add the FROM clause if needed */
@@ -2151,7 +2155,9 @@ get_insert_query_def(Query *query, deparse_context *context)
 
 		appendStringInfo(buf, sep);
 		sep = ", ";
-		appendStringInfo(buf, "%s", quote_identifier(tle->resdom->resname));
+		appendStringInfo(buf, "%s",
+						 quote_identifier(get_relid_attribute_name(rte->relid,
+														tle->resdom->resno)));
 	}
 	appendStringInfo(buf, ") ");
 
@@ -2225,7 +2231,8 @@ get_update_query_def(Query *query, deparse_context *context)
 		 */
 		if (!tleIsArrayAssign(tle))
 			appendStringInfo(buf, "%s = ",
-							 quote_identifier(tle->resdom->resname));
+						quote_identifier(get_relid_attribute_name(rte->relid,
+														tle->resdom->resno)));
 		get_rule_expr((Node *) tle->expr, context, false);
 	}
 
@@ -4351,22 +4358,3 @@ print_operator_name(StringInfo buf, List *opname)
 		appendStringInfo(buf, "%s)", strVal(lfirst(opname)));
 	}
 }
-
-/*
- * get_relid_attribute_name
- *		Get an attribute name by its relations Oid and its attnum
- *
- * Same as underlying syscache routine get_attname(), except that error
- * is handled by elog() instead of returning NULL.
- */
-static char *
-get_relid_attribute_name(Oid relid, AttrNumber attnum)
-{
-	char	   *attname;
-
-	attname = get_attname(relid, attnum);
-	if (attname == NULL)
-		elog(ERROR, "cache lookup failed for attribute %d of relation %u",
-			 attnum, relid);
-	return attname;
-}
diff --git a/src/backend/utils/cache/lsyscache.c b/src/backend/utils/cache/lsyscache.c
index 0dfa0eb7c79..0faa097f349 100644
--- a/src/backend/utils/cache/lsyscache.c
+++ b/src/backend/utils/cache/lsyscache.c
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/lsyscache.c,v 1.105 2003/08/04 02:40:06 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/cache/lsyscache.c,v 1.106 2003/08/11 23:04:49 tgl Exp $
  *
  * NOTES
  *	  Eventually, the index information should go through here, too.
@@ -180,11 +180,10 @@ get_op_hash_function(Oid opno)
 
 /*
  * get_attname
- *
  *		Given the relation id and the attribute number,
  *		return the "attname" field from the attribute relation.
  *
- * Note: returns a palloc'd copy of the string, or NULL if no such operator.
+ * Note: returns a palloc'd copy of the string, or NULL if no such attribute.
  */
 char *
 get_attname(Oid relid, AttrNumber attnum)
@@ -209,6 +208,24 @@ get_attname(Oid relid, AttrNumber attnum)
 }
 
 /*
+ * get_relid_attribute_name
+ *
+ * Same as above routine get_attname(), except that error
+ * is handled by elog() instead of returning NULL.
+ */
+char *
+get_relid_attribute_name(Oid relid, AttrNumber attnum)
+{
+	char	   *attname;
+
+	attname = get_attname(relid, attnum);
+	if (attname == NULL)
+		elog(ERROR, "cache lookup failed for attribute %d of relation %u",
+			 attnum, relid);
+	return attname;
+}
+
+/*
  * get_attnum
  *
  *		Given the relation id and the attribute name,
@@ -1443,7 +1460,7 @@ get_typtype(Oid typid)
  * get_typname
  *		Returns the name of a given type.
  *
- * Returns a palloc'd copy of the string, or NULL if no such relation.
+ * Returns a palloc'd copy of the string, or NULL if no such type.
  *
  * NOTE: since type name is not unique, be wary of code that uses this
  * for anything except preparing error messages.
diff --git a/src/backend/utils/misc/guc.c b/src/backend/utils/misc/guc.c
index ddaee15de57..2ff904510ca 100644
--- a/src/backend/utils/misc/guc.c
+++ b/src/backend/utils/misc/guc.c
@@ -10,7 +10,7 @@
  * Written by Peter Eisentraut <peter_e@gmx.net>.
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/utils/misc/guc.c,v 1.148 2003/08/04 23:59:39 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/utils/misc/guc.c,v 1.149 2003/08/11 23:04:49 tgl Exp $
  *
  *--------------------------------------------------------------------
  */
@@ -3293,7 +3293,7 @@ GetPGVariableResultDesc(const char *name)
 
 		/* need a tuple descriptor representing a single TEXT column */
 		tupdesc = CreateTemplateTupleDesc(1, false);
-		TupleDescInitEntry(tupdesc, (AttrNumber) 1, (char *) varname,
+		TupleDescInitEntry(tupdesc, (AttrNumber) 1, varname,
 						   TEXTOID, -1, 0, false);
 	}
 	return tupdesc;
@@ -3333,7 +3333,7 @@ ShowGUCConfigOption(const char *name, DestReceiver *dest)
 
 	/* need a tuple descriptor representing a single TEXT column */
 	tupdesc = CreateTemplateTupleDesc(1, false);
-	TupleDescInitEntry(tupdesc, (AttrNumber) 1, (char *) varname,
+	TupleDescInitEntry(tupdesc, (AttrNumber) 1, varname,
 					   TEXTOID, -1, 0, false);
 
 	/* prepare for projection of tuples */