Fix CLUSTER ... or at least undo the bit-rot it's suffered since 6.5.
authorTom Lane <[email protected]>
Thu, 11 May 2000 03:54:18 +0000 (03:54 +0000)
committerTom Lane <[email protected]>
Thu, 11 May 2000 03:54:18 +0000 (03:54 +0000)
It's still pretty fundamentally bogus :-(.
Freebie side benefit: ALTER TABLE RENAME works on indexes now.

src/backend/commands/cluster.c
src/backend/commands/rename.c
src/backend/tcop/utility.c

index 0cbf4a2177b29698af4ad906ceccd06f53481847..688bd5148f6049fed0bda380e4a922f1dc2ecccb 100644 (file)
@@ -15,7 +15,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/commands/cluster.c,v 1.51 2000/04/12 17:14:57 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/commands/cluster.c,v 1.52 2000/05/11 03:54:17 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -27,7 +27,6 @@
 #include "catalog/heap.h"
 #include "catalog/index.h"
 #include "catalog/pg_proc.h"
-#include "catalog/pg_type.h"
 #include "commands/cluster.h"
 #include "commands/rename.h"
 #include "optimizer/internal.h"
@@ -80,26 +79,13 @@ cluster(char *oldrelname, char *oldindexname)
        char            saveoldrelname[NAMEDATALEN];
        char            saveoldindexname[NAMEDATALEN];
 
-
        /*
-        * Save the old names because they will get lost when the old
-        * relations are destroyed.
+        * Copy the arguments into local storage, because they are probably
+        * in palloc'd storage that will go away when we commit a transaction.
         */
        strcpy(saveoldrelname, oldrelname);
        strcpy(saveoldindexname, oldindexname);
 
-       /*
-        * I'm going to force all checking back into the commands.c function.
-        *
-        * Get the list if indicies for this relation. If the index we want is
-        * among them, do not add it to the 'kill' list, as it will be handled
-        * by the 'clean up' code which commits this transaction.
-        *
-        * I'm not using the SysCache, because this will happen but once, and the
-        * slow way is the sure way in this case.
-        *
-        */
-
        /*
         * Like vacuum, cluster spans transactions, so I'm going to handle it
         * in the same way: commit and restart transactions where needed.
@@ -108,13 +94,17 @@ cluster(char *oldrelname, char *oldindexname)
         * of the initial transaction.
         */
 
-       OldHeap = heap_openr(oldrelname, AccessExclusiveLock);
+       OldHeap = heap_openr(saveoldrelname, AccessExclusiveLock);
        OIDOldHeap = RelationGetRelid(OldHeap);
 
-       OldIndex = index_openr(oldindexname);           /* Open old index relation      */
+       OldIndex = index_openr(saveoldindexname); /* Open old index relation    */
        LockRelation(OldIndex, AccessExclusiveLock);
        OIDOldIndex = RelationGetRelid(OldIndex);
 
+       /*
+        * XXX Should check that index is in fact an index on this relation?
+        */
+
        heap_close(OldHeap, NoLock);/* do NOT give up the locks */
        index_close(OldIndex);
 
@@ -130,7 +120,6 @@ cluster(char *oldrelname, char *oldindexname)
        OIDNewHeap = RelationGetRelid(NewHeap);
        strcpy(NewHeapName, RelationGetRelationName(NewHeap));
 
-
        /* To make the new heap visible (which is until now empty). */
        CommandCounterIncrement();
 
@@ -150,23 +139,14 @@ cluster(char *oldrelname, char *oldindexname)
        CommitTransactionCommand();
        StartTransactionCommand();
 
-
        /* Destroy old heap (along with its index) and rename new. */
-       heap_drop_with_catalog(oldrelname);
+       heap_drop_with_catalog(saveoldrelname);
 
        CommitTransactionCommand();
        StartTransactionCommand();
 
        renamerel(NewHeapName, saveoldrelname);
-       TypeRename(NewHeapName, saveoldrelname);
-
        renamerel(NewIndexName, saveoldindexname);
-
-       /*
-        * Again flush all the buffers.  XXX perhaps not needed?
-        */
-       CommitTransactionCommand();
-       StartTransactionCommand();
 }
 
 static Relation
@@ -298,6 +278,8 @@ copy_index(Oid OIDOldIndex, Oid OIDNewHeap)
                                 Old_pg_index_Form->indisunique,
                                 Old_pg_index_Form->indisprimary);
 
+       setRelhasindexInplace(OIDNewHeap, true, false);
+
        index_close(OldIndex);
        heap_close(NewHeap, AccessExclusiveLock);
 }
@@ -311,9 +293,6 @@ rebuildheap(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex)
                                LocalOldIndex;
        IndexScanDesc ScanDesc;
        RetrieveIndexResult ScanResult;
-       HeapTupleData LocalHeapTuple;
-       Buffer          LocalBuffer;
-       Oid                     OIDNewHeapInsert;
 
        /*
         * Open the relations I need. Scan through the OldHeap on the OldIndex
@@ -321,21 +300,24 @@ rebuildheap(Oid OIDNewHeap, Oid OIDOldHeap, Oid OIDOldIndex)
         */
        LocalNewHeap = heap_open(OIDNewHeap, AccessExclusiveLock);
        LocalOldHeap = heap_open(OIDOldHeap, AccessExclusiveLock);
-       LocalOldIndex = (Relation) index_open(OIDOldIndex);
+       LocalOldIndex = index_open(OIDOldIndex);
 
        ScanDesc = index_beginscan(LocalOldIndex, false, 0, (ScanKey) NULL);
 
        while ((ScanResult = index_getnext(ScanDesc, ForwardScanDirection)) != NULL)
        {
+               HeapTupleData LocalHeapTuple;
+               Buffer          LocalBuffer;
 
                LocalHeapTuple.t_self = ScanResult->heap_iptr;
                LocalHeapTuple.t_datamcxt = NULL;
                LocalHeapTuple.t_data = NULL;
                heap_fetch(LocalOldHeap, SnapshotNow, &LocalHeapTuple, &LocalBuffer);
-               OIDNewHeapInsert = heap_insert(LocalNewHeap, &LocalHeapTuple);
-               pfree(ScanResult);
+               heap_insert(LocalNewHeap, &LocalHeapTuple);
                ReleaseBuffer(LocalBuffer);
+               pfree(ScanResult);
        }
+
        index_endscan(ScanDesc);
 
        index_close(LocalOldIndex);
index 6a9c92b1e63ec752226ac0bf9c7882355ceb338c..48454b128d3b4e05108b543935e925b2f9a6cfc2 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/commands/Attic/rename.c,v 1.42 2000/04/12 17:14:59 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/commands/Attic/rename.c,v 1.43 2000/05/11 03:54:18 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -18,6 +18,7 @@
 
 #include "access/heapam.h"
 #include "catalog/catname.h"
+#include "catalog/pg_type.h"
 #include "utils/syscache.h"
 #include "catalog/heap.h"
 #include "catalog/indexing.h"
@@ -27,6 +28,8 @@
 #include "storage/smgr.h"
 #include "optimizer/prep.h"
 #include "utils/acl.h"
+#include "utils/relcache.h"
+
 
 /*
  *             renameatt               - changes the name of a attribute in a relation
@@ -180,6 +183,7 @@ renamerel(const char *oldrelname, const char *newrelname)
        Relation        targetrelation;
        Relation        relrelation;    /* for RELATION relation */
        HeapTuple       oldreltup;
+       char            relkind;
        char            oldpath[MAXPGPATH],
                                newpath[MAXPGPATH],
                                toldpath[MAXPGPATH + 10],
@@ -194,11 +198,20 @@ renamerel(const char *oldrelname, const char *newrelname)
                elog(ERROR, "renamerel: Illegal class name: \"%s\" -- pg_ is reserved for system catalogs",
                         newrelname);
 
+       /*
+        * Instead of using heap_openr(), do it the hard way, so that we
+        * can rename indexes as well as regular relations.
+        */
+       targetrelation = RelationNameGetRelation(oldrelname);
+
+       if (!RelationIsValid(targetrelation))
+               elog(ERROR, "Relation '%s' does not exist", oldrelname);
+
        /*
         * Grab an exclusive lock on the target table, which we will NOT
         * release until end of transaction.
         */
-       targetrelation = heap_openr(oldrelname, AccessExclusiveLock);
+       LockRelation(targetrelation, AccessExclusiveLock);
 
        /* ----------------
         *      RENAME TABLE within a transaction block is dangerous, because
@@ -215,6 +228,8 @@ renamerel(const char *oldrelname, const char *newrelname)
        if (IsTransactionBlock() && !targetrelation->rd_myxactonly)
                elog(NOTICE, "Caution: RENAME TABLE cannot be rolled back, so don't abort now");
 
+       relkind = targetrelation->rd_rel->relkind;
+
        /*
         * Flush all blocks of the relation out of the buffer pool.  We need
         * this because the blocks are marked with the relation's name as well
@@ -304,4 +319,10 @@ renamerel(const char *oldrelname, const char *newrelname)
        CatalogCloseIndices(Num_pg_class_indices, irelations);
 
        heap_close(relrelation, RowExclusiveLock);
+
+       /*
+        * Also rename the associated type, if any.
+        */
+       if (relkind != RELKIND_INDEX)
+               TypeRename(oldrelname, newrelname);
 }
index 89ac3a635ac588df2819734d555d71101697db77..c0a8352de43cc13ca4c272d94066b11c2f14259b 100644 (file)
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/tcop/utility.c,v 1.87 2000/04/25 10:38:38 inoue Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/tcop/utility.c,v 1.88 2000/05/11 03:54:18 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -18,7 +18,6 @@
 
 #include "access/heapam.h"
 #include "catalog/catalog.h"
-#include "catalog/pg_type.h"
 #include "commands/async.h"
 #include "commands/cluster.h"
 #include "commands/command.h"
@@ -315,8 +314,6 @@ ProcessUtility(Node *parsetree,
                                         */
                                        renamerel(relname,      /* old name */
                                                          stmt->newname);       /* new name */
-                                       TypeRename(relname, /* old name */
-                                                          stmt->newname);      /* new name */
                                }
                                else
                                {