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 c6b92041d, 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

diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 8241ba8f31218d08599f04982905bb497e2920b9..cf13e6002bb086cdf64f275590eec7a304175db5 100644 (file)
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -323,10 +323,10 @@ end_heap_rewrite(RewriteState state)
                        state->rs_blockno,
                        state->rs_buffer,
                        true);
-       RelationOpenSmgr(state->rs_new_rel);
 
        PageSetChecksumInplace(state->rs_buffer, state->rs_blockno);
 
+       RelationOpenSmgr(state->rs_new_rel);
        smgrextend(state->rs_new_rel->rd_smgr, MAIN_FORKNUM, state->rs_blockno,
                   (char *) state->rs_buffer, true);
    }
@@ -339,7 +339,11 @@ end_heap_rewrite(RewriteState state)
     * wrote before the checkpoint.
     */
    if (RelationNeedsWAL(state->rs_new_rel))
+   {
+       /* for an empty table, this could be first smgr access */
+       RelationOpenSmgr(state->rs_new_rel);
        smgrimmedsync(state->rs_new_rel->rd_smgr, MAIN_FORKNUM);
+   }
 
    logical_end_heap_rewrite(state);
 

diff --git a/src/test/regress/expected/cluster.out b/src/test/regress/expected/cluster.out
index bdae8fe00cd1fd9db03a90f3ac0e3f2015a00f64..e46a66952f0c408c98c67f4096df499653b92b1d 100644 (file)
--- a/src/test/regress/expected/cluster.out
+++ b/src/test/regress/expected/cluster.out
@@ -439,6 +439,11 @@ select * from clstr_temp;
 
 drop table clstr_temp;
 RESET SESSION AUTHORIZATION;
+-- check clustering an empty table
+DROP TABLE clustertest;
+CREATE TABLE clustertest (f1 int PRIMARY KEY);
+CLUSTER clustertest USING clustertest_pkey;
+CLUSTER clustertest;
 -- Check that partitioned tables cannot be clustered
 CREATE TABLE clstrpart (a int) PARTITION BY RANGE (a);
 CREATE INDEX clstrpart_idx ON clstrpart (a);

diff --git a/src/test/regress/sql/cluster.sql b/src/test/regress/sql/cluster.sql
index 188183647c9101ed60ca9d13dcf872b551e94678..aee9cf83e04c8433d2d300081baa481a12d06fa7 100644 (file)
--- a/src/test/regress/sql/cluster.sql
+++ b/src/test/regress/sql/cluster.sql
@@ -196,6 +196,12 @@ drop table clstr_temp;
 
 RESET SESSION AUTHORIZATION;
 
+-- check clustering an empty table
+DROP TABLE clustertest;
+CREATE TABLE clustertest (f1 int PRIMARY KEY);
+CLUSTER clustertest USING clustertest_pkey;
+CLUSTER clustertest;
+
 -- Check that partitioned tables cannot be clustered
 CREATE TABLE clstrpart (a int) PARTITION BY RANGE (a);
 CREATE INDEX clstrpart_idx ON clstrpart (a);