diff options
| author | Andres Freund <andres@anarazel.de> | 2022-12-02 17:50:26 -0800 |
|---|---|---|
| committer | Andres Freund <andres@anarazel.de> | 2022-12-02 18:07:47 -0800 |
| commit | c6a60471a1a5e21bf901ad24cac27874a1886306 (patch) | |
| tree | f4b65d86afbaa3cd8469a22644f5e77fc7baabae /src/backend/utils/cache/relcache.c | |
| parent | 97299cf99df086af847a641161e8b925fdf840b9 (diff) | |
Prevent pgstats from getting confused when relkind of a relation changes
When the relkind of a relache entry changes, because a table is converted into
a view, pgstats can get confused in 15+, leading to crashes or assertion
failures.
For HEAD, Tom fixed this in b23cd185fd5, by removing support for converting a
table to a view, removing the source of the inconsistency. This commit just
adds an assertion that a relcache entry's relkind does not change, just in
case we end up with another case of that in the future. As there's no cases of
changing relkind anymore, we can't add a test that that's handled correctly.
For 15, fix the problem by not maintaining the association with the old pgstat
entry when the relkind changes during a relcache invalidation processing. In
that case the pgstat entry needs to be unlinked first, to avoid
PgStat_TableStatus->relation getting out of sync. Also add a test reproducing
the issues.
No known problem exists in 11-14, so just add the test there.
Reported-by: vignesh C <vignesh21@gmail.com>
Author: Andres Freund <andres@anarazel.de>
Reviewed-by: Tom Lane <tgl@sss.pgh.pa.us>
Discussion: https://postgr.es/m/CALDaNm2yXz+zOtv7y5zBd5WKT8O0Ld3YxikuU3dcyCvxF7gypA@mail.gmail.com
Discussion: https://postgr.es/m/CALDaNm3oZA-8Wbps2Jd1g5_Gjrr-x3YWrJPek-mF5Asrrvz2Dg@mail.gmail.com
Backpatch: 15-
Diffstat (limited to 'src/backend/utils/cache/relcache.c')
| -rw-r--r-- | src/backend/utils/cache/relcache.c | 25 |
1 files changed, 23 insertions, 2 deletions
diff --git a/src/backend/utils/cache/relcache.c b/src/backend/utils/cache/relcache.c index f8d9f5fa485..52fe7a74453 100644 --- a/src/backend/utils/cache/relcache.c +++ b/src/backend/utils/cache/relcache.c @@ -2626,6 +2626,7 @@ RelationClearRelation(Relation relation, bool rebuild) bool keep_rules; bool keep_policies; bool keep_partkey; + bool keep_pgstats; /* Build temporary entry, but don't link it into hashtable */ newrel = RelationBuildDesc(save_relid, false); @@ -2668,6 +2669,21 @@ RelationClearRelation(Relation relation, bool rebuild) keep_partkey = (relation->rd_partkey != NULL); /* + * Keep stats pointers, except when the relkind changes (e.g. when + * converting tables into views). Different kinds of relations might + * have different types of stats. + * + * If we don't want to keep the stats, unlink the stats and relcache + * entry (and do so before entering the "critical section" + * below). This is important because otherwise + * PgStat_TableStatus->relation would get out of sync with + * relation->pgstat_info. + */ + keep_pgstats = relation->rd_rel->relkind == newrel->rd_rel->relkind; + if (!keep_pgstats) + pgstat_unlink_relation(relation); + + /* * Perform swapping of the relcache entry contents. Within this * process the old entry is momentarily invalid, so there *must* be no * possibility of CHECK_FOR_INTERRUPTS within this sequence. Do it in @@ -2720,9 +2736,14 @@ RelationClearRelation(Relation relation, bool rebuild) SWAPFIELD(RowSecurityDesc *, rd_rsdesc); /* toast OID override must be preserved */ SWAPFIELD(Oid, rd_toastoid); + /* pgstat_info / enabled must be preserved */ - SWAPFIELD(struct PgStat_TableStatus *, pgstat_info); - SWAPFIELD(bool, pgstat_enabled); + if (keep_pgstats) + { + SWAPFIELD(struct PgStat_TableStatus *, pgstat_info); + SWAPFIELD(bool, pgstat_enabled); + } + /* preserve old partition key if we have one */ if (keep_partkey) { |