Seems like a bad idea that REINDEX TABLE supports (or thinks it does)
authorTom Lane
Fri, 19 Sep 2003 19:57:42 +0000 (19:57 +0000)
committerTom Lane
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