]> granicus.if.org Git - postgresql/commitdiff
Fix brain fade in DefineIndex(): it was continuing to access the table's
authorTom Lane <tgl@sss.pgh.pa.us>
Sat, 25 Aug 2007 19:08:19 +0000 (19:08 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Sat, 25 Aug 2007 19:08:19 +0000 (19:08 +0000)
relcache entry after having heap_close'd it.  This could lead to misbehavior
if a relcache flush wiped out the cache entry meanwhile.  In 8.2 there is a
very real risk of CREATE INDEX CONCURRENTLY using the wrong relid for locking
and waiting purposes.  I think the bug is only cosmetic in 8.0 and 8.1,
because their transgression is limited to using RelationGetRelationName(rel)
in an ereport message immediately after heap_close, and there's no way (except
with special debugging options) for a cache flush to occur in that interval.
Not quite sure that it's cosmetic in 7.4, but seems best to patch anyway.

Found by trying to run the regression tests with CLOBBER_CACHE_ALWAYS enabled.
Maybe we should try to do that on a regular basis --- it's awfully slow,
but perhaps some fast buildfarm machine could do it once in awhile.

src/backend/commands/indexcmds.c

index 943662f8f8b2665a454bcd30d8fe3923657c50ef..d79e73f59d8c29c4afb5f86517d34041bb3e219f 100644 (file)
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *       $PostgreSQL: pgsql/src/backend/commands/indexcmds.c,v 1.161 2007/07/17 05:02:00 neilc Exp $
+ *       $PostgreSQL: pgsql/src/backend/commands/indexcmds.c,v 1.162 2007/08/25 19:08:19 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -428,8 +428,6 @@ DefineIndex(RangeVar *heapRelation,
                                          relationId, accessMethodName, accessMethodId,
                                          amcanorder, isconstraint);
 
-       heap_close(rel, NoLock);
-
        /*
         * Report index creation if appropriate (delay this till after most of the
         * error checks)
@@ -441,6 +439,10 @@ DefineIndex(RangeVar *heapRelation,
                                  primary ? "PRIMARY KEY" : "UNIQUE",
                                  indexRelationName, RelationGetRelationName(rel))));
 
+       /* save lockrelid for below, then close rel */
+       heaprelid = rel->rd_lockInfo.lockRelId;
+       heap_close(rel, NoLock);
+
        indexRelationId =
                index_create(relationId, indexRelationName, indexRelationId,
                                         indexInfo, accessMethodId, tablespaceId, classObjectId,
@@ -468,7 +470,6 @@ DefineIndex(RangeVar *heapRelation,
         * because there are no operations that could change its state while we
         * hold lock on the parent table.  This might need to change later.
         */
-       heaprelid = rel->rd_lockInfo.lockRelId;
        LockRelationIdForSession(&heaprelid, ShareUpdateExclusiveLock);
 
        CommitTransactionCommand();