Some minor tweaks of REINDEX processing: grab exclusive lock a little

earlier, make error checks more uniform.

src/backend/catalog/index.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/catalog/index.c,v 1.169 2001/11/05 17:46:24 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/catalog/index.c,v 1.170 2001/11/20 02:46:13 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -1874,10 +1874,27 @@ reindex_index(Oid indexId, bool force, bool inplace)
 	 * REINDEX within a transaction block is dangerous, because if the
 	 * transaction is later rolled back we have no way to undo truncation
 	 * of the index's physical file.  Disallow it.
+	 *
+	 * XXX if we're not doing an inplace rebuild, wouldn't this be okay?
 	 */
 	if (IsTransactionBlock())
 		elog(ERROR, "REINDEX cannot run inside a transaction block");
 
+	/*
+	 * Open our index relation and get an exclusive lock on it.
+	 *
+	 * Note: doing this before opening the parent heap relation means
+	 * there's a possibility for deadlock failure against another xact
+	 * that is doing normal accesses to the heap and index.  However,
+	 * it's not real clear why you'd be needing to do REINDEX on a table
+	 * that's in active use, so I'd rather have the protection of making
+	 * sure the index is locked down.
+	 */
+	iRel = index_open(indexId);
+	if (iRel == NULL)
+		elog(ERROR, "reindex_index: can't open index relation");
+	LockRelation(iRel, AccessExclusiveLock);
+
 	old = SetReindexProcessing(true);
 
 	/* Scan pg_index to find the index's pg_index entry */
@@ -1898,22 +1915,17 @@ reindex_index(Oid indexId, bool force, bool inplace)
 	heap_endscan(scan);
 	heap_close(indexRelation, AccessShareLock);
 
-	/* Open our index relation */
+	/* Open the parent heap relation */
 	heapRelation = heap_open(heapId, ExclusiveLock);
 	if (heapRelation == NULL)
 		elog(ERROR, "reindex_index: can't open heap relation");
-	iRel = index_open(indexId);
-	if (iRel == NULL)
-		elog(ERROR, "reindex_index: can't open index relation");
 
-	if (!inplace)
-	{
-		inplace = iRel->rd_rel->relisshared;
-		if (!inplace)
-			setNewRelfilenode(iRel);
-	}
-	/* Obtain exclusive lock on it, just to be sure */
-	LockRelation(iRel, AccessExclusiveLock);
+	/*
+	 * Force inplace processing if it's a shared index.  Necessary because
+	 * we have no way to update relfilenode in other databases.
+	 */
+	if (iRel->rd_rel->relisshared)
+		inplace = true;
 
 	if (inplace)
 	{
@@ -1928,6 +1940,13 @@ reindex_index(Oid indexId, bool force, bool inplace)
 		iRel->rd_nblocks = 0;
 		iRel->rd_targblock = InvalidBlockNumber;
 	}
+	else
+	{
+		/*
+		 * We'll build a new physical relation for the index.
+		 */
+		setNewRelfilenode(iRel);
+	}
 
 	/* Initialize the index and rebuild */
 	index_build(heapRelation, iRel, indexInfo);
@@ -1982,11 +2001,9 @@ reindex_relation(Oid relid, bool force)
 	HeapTuple	indexTuple;
 	bool		old,
 				reindexed;
-
 	bool		deactivate_needed,
 				overwrite,
 				upd_pg_class_inplace;
-
 	Relation	rel;
 
 	overwrite = upd_pg_class_inplace = deactivate_needed = false;

src/backend/commands/indexcmds.c

@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/commands/indexcmds.c,v 1.60 2001/10/25 05:49:25 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/commands/indexcmds.c,v 1.61 2001/11/20 02:46:13 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -637,6 +637,9 @@ ReindexDatabase(const char *dbname, bool force, bool all)
 											ALLOCSET_DEFAULT_INITSIZE,
 											ALLOCSET_DEFAULT_MAXSIZE);
 
+	/*
+	 * Scan pg_class to build a list of the relations we need to reindex.
+	 */
 	relationRelation = heap_openr(RelationRelationName, AccessShareLock);
 	scan = heap_beginscan(relationRelation, false, SnapshotNow, 0, NULL);
 	relcnt = relalc = 0;
@@ -646,8 +649,6 @@ ReindexDatabase(const char *dbname, bool force, bool all)
 		{
 			if (!IsSystemRelationName(NameStr(((Form_pg_class) GETSTRUCT(tuple))->relname)))
 				continue;
-			if (((Form_pg_class) GETSTRUCT(tuple))->relhasrules)
-				continue;
 		}
 		if (((Form_pg_class) GETSTRUCT(tuple))->relkind == RELKIND_RELATION)
 		{
@@ -670,6 +671,7 @@ ReindexDatabase(const char *dbname, bool force, bool all)
 	heap_endscan(scan);
 	heap_close(relationRelation, AccessShareLock);
 
+	/* Now reindex each rel in a separate transaction */
 	CommitTransactionCommand();
 	for (i = 0; i < relcnt; i++)
 	{

src/backend/tcop/utility.c

@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *	  $Header: /cvsroot/pgsql/src/backend/tcop/utility.c,v 1.122 2001/10/25 05:49:43 momjian Exp $
+ *	  $Header: /cvsroot/pgsql/src/backend/tcop/utility.c,v 1.123 2001/11/20 02:46:13 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -865,7 +865,7 @@ ProcessUtility(Node *parsetree,
 						relname = (char *) stmt->name;
 						if (IsSystemRelationName(relname))
 						{
-							if (!allowSystemTableMods && IsSystemRelationName(relname))
+							if (!allowSystemTableMods)
 								elog(ERROR, "\"%s\" is a system index. call REINDEX under standalone postgres with -O -P options",
 									 relname);
 							if (!IsIgnoringSystemIndexes())
@@ -878,6 +878,15 @@ ProcessUtility(Node *parsetree,
 						break;
 					case TABLE:
 						relname = (char *) stmt->name;
+						if (IsSystemRelationName(relname))
+						{
+							if (!allowSystemTableMods)
+								elog(ERROR, "\"%s\" is a system table. call REINDEX under standalone postgres with -O -P options",
+									 relname);
+							if (!IsIgnoringSystemIndexes())
+								elog(ERROR, "\"%s\" is a system table. call REINDEX under standalone postgres with -P -O options",
+									 relname);
+						}
 						if (!pg_ownercheck(GetUserId(), relname, RELNAME))
 							elog(ERROR, "%s: %s", relname, aclcheck_error_strings[ACLCHECK_NOT_OWNER]);
 						ReindexTable(relname, stmt->force);