Fix misapplication of pgstat_count_truncate to wrong relation.
authorTom Lane
Fri, 7 Dec 2018 17:12:00 +0000 (12:12 -0500)
committerTom Lane
Fri, 7 Dec 2018 17:12:00 +0000 (12:12 -0500)
The stanza of ExecuteTruncate[Guts] that truncates a target table's toast
relation re-used the loop local variable "rel" to reference the toast rel.
This was safe enough when written, but commit d42358efb added code below
that that supposed "rel" still pointed to the parent table.  Therefore,
the stats counter update was applied to the wrong relcache entry (the
toast rel not the user rel); and if we were unlucky and that relcache
entry had been flushed during reindex_relation, very bad things could
ensue.

(I'm surprised that CLOBBER_CACHE_ALWAYS testing hasn't found this.
I'm even more surprised that the problem wasn't detected during the
development of d42358efb; it must not have been tested in any case
with a toast table, as the incorrect stats counts are very obvious.)

To fix, replace use of "rel" in that code branch with a more local
variable.  Adjust test cases added by d42358efb so that some of them
use tables with toast tables.

Per bug #15540 from Pan Bian.  Back-patch to 9.5 where d42358efb came in.

Discussion: https://postgr.es/m/15540-01078812338195c0@postgresql.org

src/backend/commands/tablecmds.c
src/test/regress/expected/stats.out
src/test/regress/sql/stats.sql

index 9e52945ca416b24e1207cbcc4996149033773388..4dfa4a4c49cb8472af6800796a994d2c4056aecb 100644 (file)
@@ -1427,19 +1427,22 @@ ExecuteTruncate(TruncateStmt *stmt)
                heap_create_init_fork(rel);
 
            heap_relid = RelationGetRelid(rel);
-           toast_relid = rel->rd_rel->reltoastrelid;
 
            /*
             * The same for the toast table, if any.
             */
+           toast_relid = rel->rd_rel->reltoastrelid;
            if (OidIsValid(toast_relid))
            {
-               rel = relation_open(toast_relid, AccessExclusiveLock);
-               RelationSetNewRelfilenode(rel, rel->rd_rel->relpersistence,
+               Relation    toastrel = relation_open(toast_relid,
+                                                    AccessExclusiveLock);
+
+               RelationSetNewRelfilenode(toastrel,
+                                         toastrel->rd_rel->relpersistence,
                                          RecentXmin, minmulti);
-               if (rel->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED)
-                   heap_create_init_fork(rel);
-               heap_close(rel, NoLock);
+               if (toastrel->rd_rel->relpersistence == RELPERSISTENCE_UNLOGGED)
+                   heap_create_init_fork(toastrel);
+               heap_close(toastrel, NoLock);
            }
 
            /*
index fc91f3ce3652f9ce2a00201f0d5c43fa598486dd..ab6ec7db36773bd5a32bc146132749416133914d 100644 (file)
@@ -79,9 +79,9 @@ end
 $$ language plpgsql;
 -- test effects of TRUNCATE on n_live_tup/n_dead_tup counters
 CREATE TABLE trunc_stats_test(id serial);
-CREATE TABLE trunc_stats_test1(id serial);
+CREATE TABLE trunc_stats_test1(id serial, stuff text);
 CREATE TABLE trunc_stats_test2(id serial);
-CREATE TABLE trunc_stats_test3(id serial);
+CREATE TABLE trunc_stats_test3(id serial, stuff text);
 CREATE TABLE trunc_stats_test4(id serial);
 -- check that n_live_tup is reset to 0 after truncate
 INSERT INTO trunc_stats_test DEFAULT VALUES;
index 6e882bf3acade32df1599c9be3f031be6dc9c8d2..a4e47d011a9010ace07788cc2782bb6b915db385 100644 (file)
@@ -79,9 +79,9 @@ $$ language plpgsql;
 
 -- test effects of TRUNCATE on n_live_tup/n_dead_tup counters
 CREATE TABLE trunc_stats_test(id serial);
-CREATE TABLE trunc_stats_test1(id serial);
+CREATE TABLE trunc_stats_test1(id serial, stuff text);
 CREATE TABLE trunc_stats_test2(id serial);
-CREATE TABLE trunc_stats_test3(id serial);
+CREATE TABLE trunc_stats_test3(id serial, stuff text);
 CREATE TABLE trunc_stats_test4(id serial);
 
 -- check that n_live_tup is reset to 0 after truncate