Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
Skip to content

Commit 68f7b91

Browse files
committed
In rebuild_relation(), don't access an already-closed relcache entry.
This reliably fails with -DRELCACHE_FORCE_RELEASE, as reported by Andrew Dunstan, and could sometimes fail in normal operation, resulting in a wrong persistence value being used for the transient table. It's not immediately clear to me what effects that might have beyond the risk of a crash while accessing OldHeap->rd_rel->relpersistence, but it's probably not good. Bug introduced by commit f41872d, and made substantially worse by commit 85b506b, which added a second such access significantly later than the heap_close. I doubt the first reference could fail in a production scenario, but the second one definitely could. Discussion: https://postgr.es/m/7b52f900-0579-cda9-ae2e-de5da17090e6@2ndQuadrant.com
1 parent b0344f8 commit 68f7b91

File tree

1 file changed

+5
-3
lines changed

1 file changed

+5
-3
lines changed

src/backend/commands/cluster.c

+5-3
Original file line numberDiff line numberDiff line change
@@ -559,6 +559,7 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
559559
Oid tableOid = RelationGetRelid(OldHeap);
560560
Oid tableSpace = OldHeap->rd_rel->reltablespace;
561561
Oid OIDNewHeap;
562+
char relpersistence;
562563
bool is_system_catalog;
563564
bool swap_toast_by_content;
564565
TransactionId frozenXid;
@@ -568,15 +569,16 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
568569
if (OidIsValid(indexOid))
569570
mark_index_clustered(OldHeap, indexOid, true);
570571

571-
/* Remember if it's a system catalog */
572+
/* Remember info about rel before closing OldHeap */
573+
relpersistence = OldHeap->rd_rel->relpersistence;
572574
is_system_catalog = IsSystemRelation(OldHeap);
573575

574576
/* Close relcache entry, but keep lock until transaction commit */
575577
heap_close(OldHeap, NoLock);
576578

577579
/* Create the transient table that will receive the re-ordered data */
578580
OIDNewHeap = make_new_heap(tableOid, tableSpace,
579-
OldHeap->rd_rel->relpersistence,
581+
relpersistence,
580582
AccessExclusiveLock);
581583

582584
/* Copy the heap data into the new table in the desired order */
@@ -590,7 +592,7 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
590592
finish_heap_swap(tableOid, OIDNewHeap, is_system_catalog,
591593
swap_toast_by_content, false, true,
592594
frozenXid, cutoffMulti,
593-
OldHeap->rd_rel->relpersistence);
595+
relpersistence);
594596
}
595597

596598

0 commit comments

Comments
 (0)