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

Commit d4791ac

Browse files
committed
Avoid possible crash while finishing up a heap rewrite.
end_heap_rewrite was not careful to ensure that the target relation is open at the smgr level before performing its final smgrimmedsync. In ordinary cases this is no problem, because it would have been opened earlier during the rewrite. However a crash can be reproduced by re-clustering an empty table with CLOBBER_CACHE_ALWAYS enabled. Although that exact scenario does not crash in v13, I think that's a chance result of unrelated planner changes, and the problem is likely still reachable with other test cases. The true proximate cause of this failure is commit c6b9204, which replaced a call to heap_sync (which was careful about opening smgr) with a direct call to smgrimmedsync. Hence, back-patch to v13. Amul Sul, per report from Neha Sharma; cosmetic changes and test case by me. Discussion: https://postgr.es/m/CANiYTQsU7yMFpQYnv=BrcRVqK_3U3mtAzAsJCaqtzsDHfsUbdQ@mail.gmail.com
1 parent 1faaad2 commit d4791ac

File tree

3 files changed

+16
-1
lines changed

3 files changed

+16
-1
lines changed

src/backend/access/heap/rewriteheap.c

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -324,10 +324,10 @@ end_heap_rewrite(RewriteState state)
324324
state->rs_blockno,
325325
state->rs_buffer,
326326
true);
327-
RelationOpenSmgr(state->rs_new_rel);
328327

329328
PageSetChecksumInplace(state->rs_buffer, state->rs_blockno);
330329

330+
RelationOpenSmgr(state->rs_new_rel);
331331
smgrextend(state->rs_new_rel->rd_smgr, MAIN_FORKNUM, state->rs_blockno,
332332
(char *) state->rs_buffer, true);
333333
}
@@ -340,7 +340,11 @@ end_heap_rewrite(RewriteState state)
340340
* wrote before the checkpoint.
341341
*/
342342
if (RelationNeedsWAL(state->rs_new_rel))
343+
{
344+
/* for an empty table, this could be first smgr access */
345+
RelationOpenSmgr(state->rs_new_rel);
343346
smgrimmedsync(state->rs_new_rel->rd_smgr, MAIN_FORKNUM);
347+
}
344348

345349
logical_end_heap_rewrite(state);
346350

src/test/regress/expected/cluster.out

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -439,6 +439,11 @@ select * from clstr_temp;
439439

440440
drop table clstr_temp;
441441
RESET SESSION AUTHORIZATION;
442+
-- check clustering an empty table
443+
DROP TABLE clustertest;
444+
CREATE TABLE clustertest (f1 int PRIMARY KEY);
445+
CLUSTER clustertest USING clustertest_pkey;
446+
CLUSTER clustertest;
442447
-- Check that partitioned tables cannot be clustered
443448
CREATE TABLE clstrpart (a int) PARTITION BY RANGE (a);
444449
CREATE INDEX clstrpart_idx ON clstrpart (a);

src/test/regress/sql/cluster.sql

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -196,6 +196,12 @@ drop table clstr_temp;
196196

197197
RESET SESSION AUTHORIZATION;
198198

199+
-- check clustering an empty table
200+
DROP TABLE clustertest;
201+
CREATE TABLE clustertest (f1 int PRIMARY KEY);
202+
CLUSTER clustertest USING clustertest_pkey;
203+
CLUSTER clustertest;
204+
199205
-- Check that partitioned tables cannot be clustered
200206
CREATE TABLE clstrpart (a int) PARTITION BY RANGE (a);
201207
CREATE INDEX clstrpart_idx ON clstrpart (a);

0 commit comments

Comments
 (0)