postgres.git / summary / log / commit / refs

commit    dbca84f04ed5debe748029699aa44fa86beca32d
Author:   Tom Lane <tgl@sss.pgh.pa.us>
Date:     Sat Mar 04 21:09:33 2017 +0000

    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 f41872d0c, and made substantially worse by
    commit 85b506bbf, 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


 src/backend/commands/cluster.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c
index 330385ba0a6..ef1abf34abb 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -557,6 +557,7 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
 	Oid			tableOid = RelationGetRelid(OldHeap);
 	Oid			tableSpace = OldHeap->rd_rel->reltablespace;
 	Oid			OIDNewHeap;
+	char		relpersistence;
 	bool		is_system_catalog;
 	bool		swap_toast_by_content;
 	TransactionId frozenXid;
@@ -566,7 +567,8 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
 	if (OidIsValid(indexOid))
 		mark_index_clustered(OldHeap, indexOid, true);
 
-	/* Remember if it's a system catalog */
+	/* Remember info about rel before closing OldHeap */
+	relpersistence = OldHeap->rd_rel->relpersistence;
 	is_system_catalog = IsSystemRelation(OldHeap);
 
 	/* Close relcache entry, but keep lock until transaction commit */
@@ -574,7 +576,7 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
 
 	/* Create the transient table that will receive the re-ordered data */
 	OIDNewHeap = make_new_heap(tableOid, tableSpace,
-							   OldHeap->rd_rel->relpersistence,
+							   relpersistence,
 							   AccessExclusiveLock);
 
 	/* Copy the heap data into the new table in the desired order */
@@ -588,7 +590,7 @@ rebuild_relation(Relation OldHeap, Oid indexOid, bool verbose)
 	finish_heap_swap(tableOid, OIDNewHeap, is_system_catalog,
 					 swap_toast_by_content, false, true,
 					 frozenXid, cutoffMulti,
-					 OldHeap->rd_rel->relpersistence);
+					 relpersistence);
 }
 
 


[parent: d77ff69341f2]