Remove heap_mark4update from AlterTableCreateToastTable. This has

tglsfdc · tglsfdc · commit 9a9825f96ad7 · 2002-09-06T00:01:53.000Z
never been the correct procedure for locking a relation, and the
recently-found ALTER TABLE bug with adding a constraint and a toast
table in the same command shows why it's a bad idea.
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v 1.39 2002/09/04 20:31:15 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/tablecmds.c,v 1.40 2002/09/06 00:01:53 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -3325,11 +3325,9 @@ AlterTableCreateToastTable(Oid relOid, bool silent)
 {
 	Relation	rel;
 	HeapTuple	reltup;
-	HeapTupleData classtuple;
 	TupleDesc	tupdesc;
 	bool		shared_relation;
 	Relation	class_rel;
-	Buffer		buffer;
 	Oid			toast_relid;
 	Oid			toast_idxid;
 	char		toast_relname[NAMEDATALEN];
@@ -3366,42 +3364,14 @@ AlterTableCreateToastTable(Oid relOid, bool silent)
 	if (shared_relation && IsUnderPostmaster)
 		elog(ERROR, "Shared relations cannot be toasted after initdb");
 
-	/*
-	 * lock the pg_class tuple for update (is that really needed?)
-	 */
-	class_rel = heap_openr(RelationRelationName, RowExclusiveLock);
-
-	reltup = SearchSysCache(RELOID,
-							ObjectIdGetDatum(relOid),
-							0, 0, 0);
-	if (!HeapTupleIsValid(reltup))
-		elog(ERROR, "ALTER TABLE: relation \"%s\" not found",
-			 RelationGetRelationName(rel));
-	classtuple.t_self = reltup->t_self;
-	ReleaseSysCache(reltup);
-
-	switch (heap_mark4update(class_rel, &classtuple, &buffer,
-							 GetCurrentCommandId()))
-	{
-		case HeapTupleSelfUpdated:
-		case HeapTupleMayBeUpdated:
-			break;
-		default:
-			elog(ERROR, "couldn't lock pg_class tuple");
-	}
-	reltup = heap_copytuple(&classtuple);
-	ReleaseBuffer(buffer);
-
 	/*
 	 * Is it already toasted?
 	 */
-	if (((Form_pg_class) GETSTRUCT(reltup))->reltoastrelid != InvalidOid)
+	if (rel->rd_rel->reltoastrelid != InvalidOid)
 	{
 		if (silent)
 		{
 			heap_close(rel, NoLock);
-			heap_close(class_rel, NoLock);
-			heap_freetuple(reltup);
 			return;
 		}
 
@@ -3417,8 +3387,6 @@ AlterTableCreateToastTable(Oid relOid, bool silent)
 		if (silent)
 		{
 			heap_close(rel, NoLock);
-			heap_close(class_rel, NoLock);
-			heap_freetuple(reltup);
 			return;
 		}
 
@@ -3509,8 +3477,17 @@ AlterTableCreateToastTable(Oid relOid, bool silent)
 	setRelhasindex(toast_relid, true, true, toast_idxid);
 
 	/*
-	 * Store the toast table's OID in the parent relation's tuple
+	 * Store the toast table's OID in the parent relation's pg_class row
 	 */
+	class_rel = heap_openr(RelationRelationName, RowExclusiveLock);
+
+	reltup = SearchSysCacheCopy(RELOID,
+								ObjectIdGetDatum(relOid),
+								0, 0, 0);
+	if (!HeapTupleIsValid(reltup))
+		elog(ERROR, "ALTER TABLE: relation \"%s\" not found",
+			 RelationGetRelationName(rel));
+
 	((Form_pg_class) GETSTRUCT(reltup))->reltoastrelid = toast_relid;
 
 	simple_heap_update(class_rel, &reltup->t_self, reltup);
@@ -3520,6 +3497,8 @@ AlterTableCreateToastTable(Oid relOid, bool silent)
 
 	heap_freetuple(reltup);
 
+	heap_close(class_rel, RowExclusiveLock);
+
 	/*
 	 * Register dependency from the toast table to the master, so that the
 	 * toast table will be deleted if the master is.
@@ -3534,9 +3513,8 @@ AlterTableCreateToastTable(Oid relOid, bool silent)
 	recordDependencyOn(&toastobject, &baseobject, DEPENDENCY_INTERNAL);
 
 	/*
-	 * Close relations and make changes visible
+	 * Clean up and make changes visible
 	 */
-	heap_close(class_rel, NoLock);
 	heap_close(rel, NoLock);
 
 	CommandCounterIncrement();
