ALTER TABLE DROP COLUMN works. Patch by Christopher Kings-Lynne,

code review by Tom Lane.  Remaining issues: functions that take or
return tuple types are likely to break if one drops (or adds!)
a column in the table defining the type.  Need to think about what
to do here.

Along the way: some code review for recent COPY changes; mark system
columns attnotnull = true where appropriate, per discussion a month ago.

5 files changed, 165 insertions, 32 deletions

diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 6337b61f2ac..e3d8ce070bc 100644
--- a/src/backend/parser/analyze.c
+++ b/src/backend/parser/analyze.c
@@ -6,7 +6,7 @@
  * Portions Copyright (c) 1996-2002, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- *	$Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.239 2002/07/16 22:12:19 tgl Exp $
+ *	$Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.240 2002/08/02 18:15:06 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1143,6 +1143,8 @@ transformIndexConstraints(ParseState *pstate, CreateStmtContext *cxt)
 						Form_pg_attribute inhattr = rel->rd_att->attrs[count];
 						char	   *inhname = NameStr(inhattr->attname);
 
+						if (inhattr->attisdropped)
+							continue;
 						if (strcmp(key->name, inhname) == 0)
 						{
 							found = true;
@@ -1178,10 +1180,7 @@ transformIndexConstraints(ParseState *pstate, CreateStmtContext *cxt)
 				/* ALTER TABLE case: does column already exist? */
 				HeapTuple	atttuple;
 
-				atttuple = SearchSysCache(ATTNAME,
-										  ObjectIdGetDatum(cxt->relOid),
-										  PointerGetDatum(key->name),
-										  0, 0);
+				atttuple = SearchSysCacheAttName(cxt->relOid, key->name);
 				if (HeapTupleIsValid(atttuple))
 				{
 					found = true;
@@ -2369,7 +2368,7 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt)
 		origTarget = (ResTarget *) lfirst(origTargetList);
 		updateTargetListEntry(pstate, tle, origTarget->name,
 							  attnameAttNum(pstate->p_target_relation,
-											origTarget->name),
+											origTarget->name, true),
 							  origTarget->indirection);
 		origTargetList = lnext(origTargetList);
 	}
@@ -2820,11 +2819,14 @@ transformFkeyGetColType(CreateStmtContext *cxt, char *colname)
 				 inh->relname);
 		for (count = 0; count < rel->rd_att->natts; count++)
 		{
-			char	   *name = NameStr(rel->rd_att->attrs[count]->attname);
+			Form_pg_attribute inhattr = rel->rd_att->attrs[count];
+			char	   *inhname = NameStr(inhattr->attname);
 
-			if (strcmp(name, colname) == 0)
+			if (inhattr->attisdropped)
+				continue;
+			if (strcmp(inhname, colname) == 0)
 			{
-				result = rel->rd_att->attrs[count]->atttypid;
+				result = inhattr->atttypid;
 				heap_close(rel, NoLock);
 				return result;
 			}
@@ -2836,10 +2838,7 @@ transformFkeyGetColType(CreateStmtContext *cxt, char *colname)
 	{
 		HeapTuple	atttuple;
 
-		atttuple = SearchSysCache(ATTNAME,
-								  ObjectIdGetDatum(cxt->relOid),
-								  PointerGetDatum(colname),
-								  0, 0);
+		atttuple = SearchSysCacheAttName(cxt->relOid, colname);
 		if (HeapTupleIsValid(atttuple))
 		{
 			result = ((Form_pg_attribute) GETSTRUCT(atttuple))->atttypid;
diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c
index f92bbe8ac5c..677acf9d1a3 100644
--- a/src/backend/parser/parse_func.c
+++ b/src/backend/parser/parse_func.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_func.c,v 1.132 2002/06/20 20:29:32 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_func.c,v 1.133 2002/08/02 18:15:07 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1150,6 +1150,9 @@ setup_field_select(Node *input, char *attname, Oid relid)
 	AttrNumber	attno;
 
 	attno = get_attnum(relid, attname);
+	if (attno == InvalidAttrNumber)
+		elog(ERROR, "Relation \"%s\" has no column \"%s\"",
+			 get_rel_name(relid), attname);
 
 	fselect->arg = input;
 	fselect->fieldnum = attno;
diff --git a/src/backend/parser/parse_relation.c b/src/backend/parser/parse_relation.c
index 78a16ea08f8..99b639d73e3 100644
--- a/src/backend/parser/parse_relation.c
+++ b/src/backend/parser/parse_relation.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_relation.c,v 1.70 2002/06/20 20:29:33 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_relation.c,v 1.71 2002/08/02 18:15:07 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -37,7 +37,9 @@ static Node *scanNameSpaceForRefname(ParseState *pstate, Node *nsnode,
 static Node *scanRTEForColumn(ParseState *pstate, RangeTblEntry *rte,
 				 char *colname);
 static bool isForUpdate(ParseState *pstate, char *refname);
-static int	specialAttNum(char *a);
+static bool get_rte_attribute_is_dropped(RangeTblEntry *rte,
+										 AttrNumber attnum);
+static int	specialAttNum(const char *attname);
 static void warnAutoRange(ParseState *pstate, RangeVar *relation);
 
 
@@ -267,12 +269,28 @@ scanRTEForColumn(ParseState *pstate, RangeTblEntry *rte, char *colname)
 	/*
 	 * Scan the user column names (or aliases) for a match. Complain if
 	 * multiple matches.
+	 *
+	 * Note: because eref->colnames may include names of dropped columns,
+	 * we need to check for non-droppedness before accepting a match.
+	 * This takes an extra cache lookup, but we can skip the lookup most
+	 * of the time by exploiting the knowledge that dropped columns are
+	 * assigned dummy names starting with '.', which is an unusual choice
+	 * for actual column names.
+	 *
+	 * Should the user try to fool us by altering pg_attribute.attname
+	 * for a dropped column, we'll still catch it by virtue of the checks
+	 * in get_rte_attribute_type(), which is called by make_var().  That
+	 * routine has to do a cache lookup anyway, so the check there is
+	 * cheap.
 	 */
 	foreach(c, rte->eref->colnames)
 	{
 		attnum++;
 		if (strcmp(strVal(lfirst(c)), colname) == 0)
 		{
+			if (colname[0] == '.' && /* see note above */
+				get_rte_attribute_is_dropped(rte, attnum))
+				continue;
 			if (result)
 				elog(ERROR, "Column reference \"%s\" is ambiguous", colname);
 			result = (Node *) make_var(pstate, rte, attnum);
@@ -962,6 +980,9 @@ expandRTE(ParseState *pstate, RangeTblEntry *rte,
 				{
 					Form_pg_attribute attr = rel->rd_att->attrs[varattno];
 
+					if (attr->attisdropped)
+						continue;
+
 					if (colnames)
 					{
 						char	   *label;
@@ -1051,6 +1072,9 @@ expandRTE(ParseState *pstate, RangeTblEntry *rte,
 					{
 						Form_pg_attribute attr = rel->rd_att->attrs[varattno];
 
+						if (attr->attisdropped)
+							continue;
+
 						if (colnames)
 						{
 							char	   *label;
@@ -1246,9 +1270,16 @@ get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
 									0, 0);
 				/* this shouldn't happen... */
 				if (!HeapTupleIsValid(tp))
-					elog(ERROR, "Relation %s does not have attribute %d",
+					elog(ERROR, "Relation \"%s\" does not have attribute %d",
 						 get_rel_name(rte->relid), attnum);
 				att_tup = (Form_pg_attribute) GETSTRUCT(tp);
+				/*
+				 * If dropped column, pretend it ain't there.  See notes
+				 * in scanRTEForColumn.
+				 */
+				if (att_tup->attisdropped)
+					elog(ERROR, "Relation \"%s\" has no column \"%s\"",
+						 get_rel_name(rte->relid), NameStr(att_tup->attname));
 				*vartype = att_tup->atttypid;
 				*vartypmod = att_tup->atttypmod;
 				ReleaseSysCache(tp);
@@ -1298,6 +1329,14 @@ get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
 						elog(ERROR, "Relation %s does not have attribute %d",
 							 get_rel_name(funcrelid), attnum);
 					att_tup = (Form_pg_attribute) GETSTRUCT(tp);
+					/*
+					 * If dropped column, pretend it ain't there.  See notes
+					 * in scanRTEForColumn.
+					 */
+					if (att_tup->attisdropped)
+						elog(ERROR, "Relation \"%s\" has no column \"%s\"",
+							 get_rel_name(funcrelid),
+							 NameStr(att_tup->attname));
 					*vartype = att_tup->atttypid;
 					*vartypmod = att_tup->atttypmod;
 					ReleaseSysCache(tp);
@@ -1330,6 +1369,86 @@ get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
 }
 
 /*
+ * get_rte_attribute_is_dropped
+ *		Check whether attempted attribute ref is to a dropped column
+ */
+static bool
+get_rte_attribute_is_dropped(RangeTblEntry *rte, AttrNumber attnum)
+{
+	bool result;
+
+	switch (rte->rtekind)
+	{
+		case RTE_RELATION:
+			{
+				/* Plain relation RTE --- get the attribute's type info */
+				HeapTuple	tp;
+				Form_pg_attribute att_tup;
+
+				tp = SearchSysCache(ATTNUM,
+									ObjectIdGetDatum(rte->relid),
+									Int16GetDatum(attnum),
+									0, 0);
+				/* this shouldn't happen... */
+				if (!HeapTupleIsValid(tp))
+					elog(ERROR, "Relation \"%s\" does not have attribute %d",
+						 get_rel_name(rte->relid), attnum);
+				att_tup = (Form_pg_attribute) GETSTRUCT(tp);
+				result = att_tup->attisdropped;
+				ReleaseSysCache(tp);
+			}
+			break;
+		case RTE_SUBQUERY:
+		case RTE_JOIN:
+			/* Subselect and join RTEs never have dropped columns */
+			result = false;
+			break;
+		case RTE_FUNCTION:
+			{
+				/* Function RTE */
+				Oid			funcrettype = exprType(rte->funcexpr);
+				Oid			funcrelid = typeidTypeRelid(funcrettype);
+
+				if (OidIsValid(funcrelid))
+				{
+					/*
+					 * Composite data type, i.e. a table's row type
+					 * Same as ordinary relation RTE
+					 */
+					HeapTuple			tp;
+					Form_pg_attribute	att_tup;
+
+					tp = SearchSysCache(ATTNUM,
+										ObjectIdGetDatum(funcrelid),
+										Int16GetDatum(attnum),
+										0, 0);
+					/* this shouldn't happen... */
+					if (!HeapTupleIsValid(tp))
+						elog(ERROR, "Relation %s does not have attribute %d",
+							 get_rel_name(funcrelid), attnum);
+					att_tup = (Form_pg_attribute) GETSTRUCT(tp);
+					result = att_tup->attisdropped;
+					ReleaseSysCache(tp);
+				}
+				else
+				{
+					/*
+					 * Must be a base data type, i.e. scalar
+					 */
+					result = false;
+				}
+			}
+			break;
+		default:
+			elog(ERROR, "get_rte_attribute_is_dropped: unsupported RTE kind %d",
+				 (int) rte->rtekind);
+			result = false;		/* keep compiler quiet */
+	}
+
+	return result;
+}
+
+/*
  *	given relation and att name, return id of variable
  *
  *	This should only be used if the relation is already
@@ -1337,23 +1456,30 @@ get_rte_attribute_type(RangeTblEntry *rte, AttrNumber attnum,
  *	for access to non-opened relations.
  */
 int
-attnameAttNum(Relation rd, char *a)
+attnameAttNum(Relation rd, const char *attname, bool sysColOK)
 {
 	int			i;
 
 	for (i = 0; i < rd->rd_rel->relnatts; i++)
-		if (namestrcmp(&(rd->rd_att->attrs[i]->attname), a) == 0)
+	{
+		Form_pg_attribute	att = rd->rd_att->attrs[i];
+
+		if (namestrcmp(&(att->attname), attname) == 0 && !att->attisdropped)
 			return i + 1;
+	}
 
-	if ((i = specialAttNum(a)) != InvalidAttrNumber)
+	if (sysColOK)
 	{
-		if (i != ObjectIdAttributeNumber || rd->rd_rel->relhasoids)
-			return i;
+		if ((i = specialAttNum(attname)) != InvalidAttrNumber)
+		{
+			if (i != ObjectIdAttributeNumber || rd->rd_rel->relhasoids)
+				return i;
+		}
 	}
 
 	/* on failure */
-	elog(ERROR, "Relation '%s' does not have attribute '%s'",
-		 RelationGetRelationName(rd), a);
+	elog(ERROR, "Relation \"%s\" has no column \"%s\"",
+		 RelationGetRelationName(rd), attname);
 	return InvalidAttrNumber;	/* lint */
 }
 
@@ -1367,11 +1493,12 @@ attnameAttNum(Relation rd, char *a)
  * at least in the case of "oid", which is now optional.
  */
 static int
-specialAttNum(char *a)
+specialAttNum(const char *attname)
 {
 	Form_pg_attribute sysatt;
 
-	sysatt = SystemAttributeByName(a, true /* "oid" will be accepted */ );
+	sysatt = SystemAttributeByName(attname,
+								   true /* "oid" will be accepted */ );
 	if (sysatt != NULL)
 		return sysatt->attnum;
 	return InvalidAttrNumber;
diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c
index 608ca7613dc..1e51f23d704 100644
--- a/src/backend/parser/parse_target.c
+++ b/src/backend/parser/parse_target.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_target.c,v 1.85 2002/06/20 20:29:33 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_target.c,v 1.86 2002/08/02 18:15:07 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -385,8 +385,12 @@ checkInsertTargets(ParseState *pstate, List *cols, List **attrnos)
 
 		for (i = 0; i < numcol; i++)
 		{
-			ResTarget	   *col = makeNode(ResTarget);
+			ResTarget	   *col;
 
+			if (attr[i]->attisdropped)
+				continue;
+
+			col = makeNode(ResTarget);
 			col->name = pstrdup(NameStr(attr[i]->attname));
 			col->indirection = NIL;
 			col->val = NULL;
@@ -407,7 +411,7 @@ checkInsertTargets(ParseState *pstate, List *cols, List **attrnos)
 			int			attrno;
 
 			/* Lookup column name, elog on failure */
-			attrno = attnameAttNum(pstate->p_target_relation, name);
+			attrno = attnameAttNum(pstate->p_target_relation, name, false);
 			/* Check for duplicates */
 			if (intMember(attrno, *attrnos))
 				elog(ERROR, "Attribute '%s' specified more than once", name);
diff --git a/src/backend/parser/parse_type.c b/src/backend/parser/parse_type.c
index 159428894e4..59c534a7ef3 100644
--- a/src/backend/parser/parse_type.c
+++ b/src/backend/parser/parse_type.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_type.c,v 1.46 2002/07/29 23:46:35 tgl Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/parser/parse_type.c,v 1.47 2002/08/02 18:15:07 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -85,8 +85,8 @@ LookupTypeName(const TypeName *typename)
 		relid = RangeVarGetRelid(rel, false);
 		attnum = get_attnum(relid, field);
 		if (attnum == InvalidAttrNumber)
-			elog(ERROR, "'%s' is not an attribute of class '%s'",
-				 field, rel->relname);
+			elog(ERROR, "Relation \"%s\" has no column \"%s\"",
+				 rel->relname, field);
 		restype = get_atttype(relid, attnum);
 
 		/* this construct should never have an array indicator */