Remove heap_mark4update from AlterTableCreateToastTable. This has
authorTom Lane
Fri, 6 Sep 2002 00:01:53 +0000 (00:01 +0000)
committerTom Lane
Fri, 6 Sep 2002 00:01:53 +0000 (00:01 +0000)
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.

src/backend/commands/tablecmds.c

index 3b06c6915b288718c10ac7932f7c4437899a0618..7b49ba720aac69408a42aa429898140dcae3cfa5 100644 (file)
@@ -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();