postgres.git / summary / log / commit / refs

commit    807df31d19e7014df1d3621292589d3653f614f2
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 7ab4874c4bd..9e39e2f648c 100644
--- a/src/backend/commands/cluster.c
+++ b/src/backend/commands/cluster.c
@@ -559,6 +559,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;
@@ -568,7 +569,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 */
@@ -576,7 +578,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 */
@@ -590,7 +592,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: 2cc16380a46d]