Upgrade ALTER TABLE DROP COLUMN so that it can drop an OID column, and

remove separate implementation of ALTER TABLE SET WITHOUT OIDS in favor
of doing a regular DROP.  Also, cause CREATE TABLE to account completely
correctly for the inheritance status of the OID column.  This fixes
problems with dropping OID columns that have dependencies, as noted by
Christopher Kings-Lynne, as well as making sure that you can't drop an
OID column that was inherited from a parent.

doc/src/sgml/ref/alter_table.sgml

 <!--
-$PostgreSQL: pgsql/doc/src/sgml/ref/alter_table.sgml,v 1.66 2004/03/09 16:57:47 neilc Exp $
+$PostgreSQL: pgsql/doc/src/sgml/ref/alter_table.sgml,v 1.67 2004/03/23 19:35:15 tgl Exp $
 PostgreSQL documentation
 -->
 
@@ -150,12 +150,11 @@ ALTER TABLE <replaceable class="PARAMETER">name</replaceable>
     <term><literal>SET WITHOUT OIDS</literal></term>
     <listitem>
      <para>
-      This form removes the <literal>oid</literal> column from the
-      table.  Removing OIDs from a table does not occur immediately.
-      The space that the OID uses will be reclaimed when the row is
-      updated.  Without updating the row, both the space and the value
-      of the OID are kept indefinitely.  This is semantically similar
-      to the <literal>DROP COLUMN</literal> process.
+      This form removes the <literal>oid</literal> system column from the
+      table.  This is exactly equivalent to
+      <literal>DROP COLUMN oid RESTRICT</literal>,
+      except that it will not complain if there is already no
+      <literal>oid</literal> column.
      </para>
 
      <para>

src/backend/bootstrap/bootparse.y

@@ -9,7 +9,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/bootstrap/bootparse.y,v 1.64 2004/01/07 18:56:25 neilc Exp $
+ *	  $PostgreSQL: pgsql/src/backend/bootstrap/bootparse.y,v 1.65 2004/03/23 19:35:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -197,6 +197,8 @@ Boot_CreateStmt:
 													  tupdesc,
 													  RELKIND_RELATION,
 													  $3,
+													  true,
+													  0,
 													  ONCOMMIT_NOOP,
 													  true);
 						elog(DEBUG4, "relation created with oid %u", id);

src/backend/catalog/heap.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.260 2004/02/15 21:01:39 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/catalog/heap.c,v 1.261 2004/03/23 19:35:16 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -458,7 +458,9 @@ CheckAttributeType(const char *attname, Oid atttypid)
 static void
 AddNewAttributeTuples(Oid new_rel_oid,
 					  TupleDesc tupdesc,
-					  char relkind)
+					  char relkind,
+					  bool oidislocal,
+					  int oidinhcount)
 {
 	Form_pg_attribute *dpp;
 	int			i;
@@ -531,11 +533,18 @@ AddNewAttributeTuples(Oid new_rel_oid,
 									 false,
 									 ATTRIBUTE_TUPLE_SIZE,
 									 (void *) *dpp);
+				attStruct = (Form_pg_attribute) GETSTRUCT(tup);
 
 				/* Fill in the correct relation OID in the copied tuple */
-				attStruct = (Form_pg_attribute) GETSTRUCT(tup);
 				attStruct->attrelid = new_rel_oid;
 
+				/* Fill in correct inheritance info for the OID column */
+				if (attStruct->attnum == ObjectIdAttributeNumber)
+				{
+					attStruct->attislocal = oidislocal;
+					attStruct->attinhcount = oidinhcount;
+				}
+
 				/*
 				 * Unneeded since they should be OK in the constant data
 				 * anyway
@@ -713,6 +722,8 @@ heap_create_with_catalog(const char *relname,
 						 TupleDesc tupdesc,
 						 char relkind,
 						 bool shared_relation,
+						 bool oidislocal,
+						 int oidinhcount,
 						 OnCommitAction oncommit,
 						 bool allow_system_table_mods)
 {
@@ -786,7 +797,8 @@ heap_create_with_catalog(const char *relname,
 	 * now add tuples to pg_attribute for the attributes in our new
 	 * relation.
 	 */
-	AddNewAttributeTuples(new_rel_oid, new_rel_desc->rd_att, relkind);
+	AddNewAttributeTuples(new_rel_oid, new_rel_desc->rd_att, relkind,
+						  oidislocal, oidinhcount);
 
 	/*
 	 * make a dependency link to force the relation to be deleted if its
@@ -973,35 +985,46 @@ RemoveAttributeById(Oid relid, AttrNumber attnum)
 			 attnum, relid);
 	attStruct = (Form_pg_attribute) GETSTRUCT(tuple);
 
-	/* Mark the attribute as dropped */
-	attStruct->attisdropped = true;
+	if (attnum < 0)
+	{
+		/* System attribute (probably OID) ... just delete the row */
 
-	/*
-	 * Set the type OID to invalid.  A dropped attribute's type link
-	 * cannot be relied on (once the attribute is dropped, the type might
-	 * be too). Fortunately we do not need the type row --- the only
-	 * really essential information is the type's typlen and typalign,
-	 * which are preserved in the attribute's attlen and attalign.  We set
-	 * atttypid to zero here as a means of catching code that incorrectly
-	 * expects it to be valid.
-	 */
-	attStruct->atttypid = InvalidOid;
+		simple_heap_delete(attr_rel, &tuple->t_self);
+	}
+	else
+	{
+		/* Dropping user attributes is lots harder */
 
-	/* Remove any NOT NULL constraint the column may have */
-	attStruct->attnotnull = false;
+		/* Mark the attribute as dropped */
+		attStruct->attisdropped = true;
 
-	/* We don't want to keep stats for it anymore */
-	attStruct->attstattarget = 0;
+		/*
+		 * Set the type OID to invalid.  A dropped attribute's type link
+		 * cannot be relied on (once the attribute is dropped, the type might
+		 * be too). Fortunately we do not need the type row --- the only
+		 * really essential information is the type's typlen and typalign,
+		 * which are preserved in the attribute's attlen and attalign.  We set
+		 * atttypid to zero here as a means of catching code that incorrectly
+		 * expects it to be valid.
+		 */
+		attStruct->atttypid = InvalidOid;
 
-	/* Change the column name to something that isn't likely to conflict */
-	snprintf(newattname, sizeof(newattname),
-			 "........pg.dropped.%d........", attnum);
-	namestrcpy(&(attStruct->attname), newattname);
+		/* Remove any NOT NULL constraint the column may have */
+		attStruct->attnotnull = false;
 
-	simple_heap_update(attr_rel, &tuple->t_self, tuple);
+		/* We don't want to keep stats for it anymore */
+		attStruct->attstattarget = 0;
 
-	/* keep the system catalog indexes current */
-	CatalogUpdateIndexes(attr_rel, tuple);
+		/* Change the column name to something that isn't likely to conflict */
+		snprintf(newattname, sizeof(newattname),
+				 "........pg.dropped.%d........", attnum);
+		namestrcpy(&(attStruct->attname), newattname);
+
+		simple_heap_update(attr_rel, &tuple->t_self, tuple);
+
+		/* keep the system catalog indexes current */
+		CatalogUpdateIndexes(attr_rel, tuple);
+	}
 
 	/*
 	 * Because updating the pg_attribute row will trigger a relcache flush
@@ -1011,7 +1034,8 @@ RemoveAttributeById(Oid relid, AttrNumber attnum)
 
 	heap_close(attr_rel, RowExclusiveLock);
 
-	RemoveStatistics(rel, attnum);
+	if (attnum > 0)
+		RemoveStatistics(rel, attnum);
 
 	relation_close(rel, NoLock);
 }

src/backend/commands/cluster.c

@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.119 2003/11/29 19:51:47 pgsql Exp $
+ *	  $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.120 2004/03/23 19:35:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -501,6 +501,8 @@ make_new_heap(Oid OIDOldHeap, const char *NewName)
 										  tupdesc,
 										  OldHeap->rd_rel->relkind,
 										  OldHeap->rd_rel->relisshared,
+										  true,
+										  0,
 										  ONCOMMIT_NOOP,
 										  allowSystemTableMods);
 

src/backend/executor/execMain.c

@@ -26,7 +26,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.229 2004/03/02 18:56:15 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/executor/execMain.c,v 1.230 2004/03/23 19:35:16 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -797,6 +797,8 @@ InitPlan(QueryDesc *queryDesc, bool explainOnly)
 												  tupdesc,
 												  RELKIND_RELATION,
 												  false,
+												  true,
+												  0,
 												  ONCOMMIT_NOOP,
 												  allowSystemTableMods);
 

src/backend/tcop/utility.c

@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.210 2004/02/10 01:55:26 tgl Exp $
+ *	  $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.211 2004/03/23 19:35:17 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -608,10 +608,11 @@ ProcessUtility(Node *parsetree,
 					case 'L':	/* CLUSTER ON */
 						AlterTableClusterOn(relid, stmt->name);
 						break;
-					case 'o':	/* ADD OIDS */
+					case 'o':	/* SET WITHOUT OIDS */
 						AlterTableAlterOids(relid,
+											false,
 							  interpretInhOption(stmt->relation->inhOpt),
-											false);
+											DROP_RESTRICT);
 						break;
 					default:	/* oops */
 						elog(ERROR, "unrecognized alter table type: %d",

src/include/catalog/heap.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/catalog/heap.h,v 1.64 2004/02/15 21:01:39 tgl Exp $
+ * $PostgreSQL: pgsql/src/include/catalog/heap.h,v 1.65 2004/03/23 19:35:17 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -41,6 +41,8 @@ extern Oid heap_create_with_catalog(const char *relname,
 						 TupleDesc tupdesc,
 						 char relkind,
 						 bool shared_relation,
+						 bool oidislocal,
+						 int oidinhcount,
 						 OnCommitAction oncommit,
 						 bool allow_system_table_mods);
 

src/include/commands/tablecmds.h

@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.14 2003/11/29 22:40:59 pgsql Exp $
+ * $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.15 2004/03/23 19:35:17 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -49,7 +49,8 @@ extern void AlterTableCreateToastTable(Oid relOid, bool silent);
 
 extern void AlterTableOwner(Oid relationOid, int32 newOwnerSysId);
 
-extern void AlterTableAlterOids(Oid myrelid, bool recurse, bool setOid);
+extern void AlterTableAlterOids(Oid myrelid, bool setOid, bool recurse,
+								DropBehavior behavior);
 
 extern Oid	DefineRelation(CreateStmt *stmt, char relkind);
 

src/test/regress/expected/alter_table.out

@@ -688,9 +688,11 @@ delete from atacc1;
 -- try dropping a non-existent column, should fail
 alter table atacc1 drop bar;
 ERROR:  column "bar" of relation "atacc1" does not exist
--- try dropping the oid column, should fail
+-- try dropping the oid column, should succeed
 alter table atacc1 drop oid;
-ERROR:  cannot drop system column "oid"
+-- try dropping the xmin column, should fail
+alter table atacc1 drop xmin;
+ERROR:  cannot drop system column "xmin"
 -- try creating a view and altering that, should fail
 create view myview as select * from atacc1;
 select * from myview;

src/test/regress/sql/alter_table.sql

@@ -632,9 +632,12 @@ delete from atacc1;
 -- try dropping a non-existent column, should fail
 alter table atacc1 drop bar;
 
--- try dropping the oid column, should fail
+-- try dropping the oid column, should succeed
 alter table atacc1 drop oid;
 
+-- try dropping the xmin column, should fail
+alter table atacc1 drop xmin;
+
 -- try creating a view and altering that, should fail
 create view myview as select * from atacc1;
 select * from myview;