]> granicus.if.org Git - postgresql/commitdiff
Seems like a bad idea that REINDEX TABLE supports (or thinks it does)
authorTom Lane <tgl@sss.pgh.pa.us>
Fri, 19 Sep 2003 19:57:42 +0000 (19:57 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Fri, 19 Sep 2003 19:57:42 +0000 (19:57 +0000)
reindexing system tables without ignoring system indexes, when the
other two varieties of REINDEX disallow it.  Make all three act the same,
and simplify downstream code accordingly.

src/backend/catalog/index.c
src/backend/commands/indexcmds.c

index 9c16e3e023dc2fe4f708720f463007ffeb1467a3..c3ff6ded3809c3f86e15a60aa8f1177033202a3a 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/catalog/index.c,v 1.214 2003/08/04 02:39:58 momjian Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/catalog/index.c,v 1.215 2003/09/19 19:57:42 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -76,7 +76,6 @@ static void UpdateIndexRelation(Oid indexoid, Oid heapoid,
                                        Oid *classOids,
                                        bool primary);
 static Oid     IndexGetRelation(Oid indexId);
-static bool activate_index(Oid indexId, bool activate, bool inplace);
 
 
 static bool reindexing = false;
@@ -1690,23 +1689,8 @@ IndexGetRelation(Oid indexId)
        return result;
 }
 
-/* ---------------------------------
- * activate_index -- activate/deactivate the specified index.
- *             Note that currently PostgreSQL doesn't hold the
- *             status per index
- * ---------------------------------
- */
-static bool
-activate_index(Oid indexId, bool activate, bool inplace)
-{
-       if (!activate)                          /* Currently does nothing */
-               return true;
-       return reindex_index(indexId, false, inplace);
-}
-
-/* --------------------------------
+/*
  * reindex_index - This routine is used to recreate an index
- * --------------------------------
  */
 bool
 reindex_index(Oid indexId, bool force, bool inplace)
@@ -1745,22 +1729,24 @@ reindex_index(Oid indexId, bool force, bool inplace)
         * the relcache can't cope with changing its relfilenode.
         *
         * In either of these cases, we are definitely processing a system index,
-        * so we'd better be ignoring system indexes.
+        * so we'd better be ignoring system indexes.  (These checks are just
+        * for paranoia's sake --- upstream code should have disallowed reindex
+        * in such cases already.)
         */
        if (iRel->rd_rel->relisshared)
        {
                if (!IsIgnoringSystemIndexes())
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-                                  errmsg("the target relation %u is shared", indexId)));
+                       elog(ERROR,
+                                "must be ignoring system indexes to reindex shared index %u",
+                                indexId);
                inplace = true;
        }
        if (iRel->rd_isnailed)
        {
                if (!IsIgnoringSystemIndexes())
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-                                  errmsg("the target relation %u is nailed", indexId)));
+                       elog(ERROR,
+                                "must be ignoring system indexes to reindex nailed index %u",
+                                indexId);
                inplace = true;
        }
 
@@ -1801,13 +1787,12 @@ reindex_index(Oid indexId, bool force, bool inplace)
        return true;
 }
 
+#ifdef NOT_USED
 /*
- * ----------------------------
  * activate_indexes_of_a_table
  *     activate/deactivate indexes of the specified table.
  *
  * Caller must already hold exclusive lock on the table.
- * ----------------------------
  */
 bool
 activate_indexes_of_a_table(Relation heaprel, bool activate)
@@ -1829,11 +1814,11 @@ activate_indexes_of_a_table(Relation heaprel, bool activate)
        }
        return true;
 }
+#endif   /* NOT_USED */
 
-/* --------------------------------
- * reindex_relation - This routine is used to recreate indexes
+/*
+ * reindex_relation - This routine is used to recreate all indexes
  * of a relation.
- * --------------------------------
  */
 bool
 reindex_relation(Oid relid, bool force)
@@ -1844,11 +1829,10 @@ reindex_relation(Oid relid, bool force)
        HeapTuple       indexTuple;
        bool            old,
                                reindexed;
-       bool            deactivate_needed,
-                               overwrite;
+       bool            overwrite;
        Relation        rel;
 
-       overwrite = deactivate_needed = false;
+       overwrite = false;
 
        /*
         * Ensure to hold an exclusive lock throughout the transaction. The
@@ -1858,12 +1842,14 @@ reindex_relation(Oid relid, bool force)
        rel = heap_open(relid, AccessExclusiveLock);
 
        /*
-        * ignore the indexes of the target system relation while processing
-        * reindex.
+        * Should be ignoring system indexes if we are reindexing a system table.
+        * (This is elog not ereport because caller should have caught it.)
         */
        if (!IsIgnoringSystemIndexes() &&
                IsSystemRelation(rel) && !IsToastRelation(rel))
-               deactivate_needed = true;
+               elog(ERROR,
+                        "must be ignoring system indexes to reindex system table %u",
+                        relid);
 
        /*
         * Shared system indexes must be overwritten because it's impossible
@@ -1871,49 +1857,35 @@ reindex_relation(Oid relid, bool force)
         */
        if (rel->rd_rel->relisshared)
        {
-               if (IsIgnoringSystemIndexes())
-               {
-                       overwrite = true;
-                       deactivate_needed = true;
-               }
-               else
-                       ereport(ERROR,
-                                       (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-                                        errmsg("the target relation %u is shared", relid)));
+               if (!IsIgnoringSystemIndexes())         /* shouldn't happen */
+                       elog(ERROR,
+                                "must be ignoring system indexes to reindex shared table %u",
+                                relid);
+               overwrite = true;
        }
 
        old = SetReindexProcessing(true);
 
-       if (deactivate_needed)
-       {
-               if (IndexesAreActive(rel))
-               {
-                       if (!force)
-                       {
-                               SetReindexProcessing(old);
-                               heap_close(rel, NoLock);
-                               return false;
-                       }
-                       activate_indexes_of_a_table(rel, false);
-                       CommandCounterIncrement();
-               }
-       }
-
        /*
         * Continue to hold the lock.
         */
        heap_close(rel, NoLock);
 
+       /*
+        * Find table's indexes by looking in pg_index (not trusting indexes...)
+        */
        indexRelation = heap_openr(IndexRelationName, AccessShareLock);
-       ScanKeyEntryInitialize(&entry, 0, Anum_pg_index_indrelid,
-                                                  F_OIDEQ, ObjectIdGetDatum(relid));
+       ScanKeyEntryInitialize(&entry, 0,
+                                                  Anum_pg_index_indrelid,
+                                                  F_OIDEQ,
+                                                  ObjectIdGetDatum(relid));
        scan = heap_beginscan(indexRelation, SnapshotNow, 1, &entry);
        reindexed = false;
        while ((indexTuple = heap_getnext(scan, ForwardScanDirection)) != NULL)
        {
                Form_pg_index index = (Form_pg_index) GETSTRUCT(indexTuple);
 
-               if (activate_index(index->indexrelid, true, overwrite))
+               if (reindex_index(index->indexrelid, false, overwrite))
                        reindexed = true;
                else
                {
@@ -1923,31 +1895,7 @@ reindex_relation(Oid relid, bool force)
        }
        heap_endscan(scan);
        heap_close(indexRelation, AccessShareLock);
-       if (reindexed)
-       {
-               /*
-                * Ok,we could use the reindexed indexes of the target system
-                * relation now.
-                */
-               if (deactivate_needed)
-               {
-                       if (!overwrite && relid == RelOid_pg_class)
-                       {
-                               /*
-                                * For pg_class, relhasindex should be set to true here in
-                                * place.
-                                */
-                               setRelhasindex(relid, true, false, InvalidOid);
-                               CommandCounterIncrement();
 
-                               /*
-                                * However the following setRelhasindex() is needed to
-                                * keep consistency with WAL.
-                                */
-                       }
-                       setRelhasindex(relid, true, false, InvalidOid);
-               }
-       }
        SetReindexProcessing(old);
 
        return reindexed;
index 45f4c3b4f7d82db8bd35c813bf8dd6fd5e0cda3d..3021eeaf4c3c927c95064eaccace574c6efebff4 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $Header: /cvsroot/pgsql/src/backend/commands/indexcmds.c,v 1.106 2003/08/17 19:58:04 tgl Exp $
+ *       $Header: /cvsroot/pgsql/src/backend/commands/indexcmds.c,v 1.107 2003/09/19 19:57:42 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -658,17 +658,41 @@ void
 ReindexTable(RangeVar *relation, bool force)
 {
        Oid                     heapOid;
-       char            relkind;
+       HeapTuple       tuple;
 
        heapOid = RangeVarGetRelid(relation, false);
-       relkind = get_rel_relkind(heapOid);
+       tuple = SearchSysCache(RELOID,
+                                                  ObjectIdGetDatum(heapOid),
+                                                  0, 0, 0);
+       if (!HeapTupleIsValid(tuple))           /* shouldn't happen */
+               elog(ERROR, "cache lookup failed for relation %u", heapOid);
 
-       if (relkind != RELKIND_RELATION && relkind != RELKIND_TOASTVALUE)
+       if (((Form_pg_class) GETSTRUCT(tuple))->relkind != RELKIND_RELATION &&
+               ((Form_pg_class) GETSTRUCT(tuple))->relkind != RELKIND_TOASTVALUE)
                ereport(ERROR,
                                (errcode(ERRCODE_WRONG_OBJECT_TYPE),
                                 errmsg("relation \"%s\" is not a table",
                                                relation->relname)));
 
+       if (IsSystemClass((Form_pg_class) GETSTRUCT(tuple)) &&
+               !IsToastClass((Form_pg_class) GETSTRUCT(tuple)))
+       {
+               if (!allowSystemTableMods)
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                                        errmsg("permission denied: \"%s\" is a system table",
+                                                       relation->relname),
+                                        errhint("Do REINDEX in standalone postgres with -O -P options.")));
+               if (!IsIgnoringSystemIndexes())
+                       ereport(ERROR,
+                                       (errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
+                                        errmsg("permission denied: \"%s\" is a system table",
+                                                       relation->relname),
+                                        errhint("Do REINDEX in standalone postgres with -P -O options.")));
+       }
+
+       ReleaseSysCache(tuple);
+
        /*
         * In-place REINDEX within a transaction block is dangerous, because
         * if the transaction is later rolled back we have no way to undo