diff options
author | Noah Misch | 2025-04-17 12:00:30 +0000 |
---|---|---|
committer | Noah Misch | 2025-04-17 12:00:30 +0000 |
commit | f4ece891fc2f3f96f0571720a1ae30db8030681b (patch) | |
tree | 61437530ceb8f305390574162aa6903cd057f0e6 /src/test | |
parent | b669293e3432ee8fdcd44854a945837bb18eea5c (diff) |
Assert lack of hazardous buffer locks before possible catalog read.
Commit 0bada39c83a150079567a6e97b1a25a198f30ea3 fixed a bug of this kind,
which existed in all branches for six days before detection. While the
probability of reaching the trouble was low, the disruption was extreme. No
new backends could start, and service restoration needed an immediate
shutdown. Hence, add this to catch the next bug like it.
The new check in RelationIdGetRelation() suffices to make autovacuum detect
the bug in commit 243e9b40f1b2dd09d6e5bf91ebf6e822a2cd3704 that led to commit
0bada39. This also checks in a number of similar places. It replaces each
Assert(IsTransactionState()) that pertained to a conditional catalog read.
No back-patch for now, but a back-patch of commit 243e9b4 should back-patch
this, too. A back-patch could omit the src/test/regress changes, since back
branches won't gain new index columns.
Reported-by: Alexander Lakhin <exclusion@gmail.com>
Discussion: https://postgr.es/m/20250410191830.0e.nmisch@google.com
Discussion: https://postgr.es/m/10ec0bc3-5933-1189-6bb8-5dec4114558e@gmail.com
Diffstat (limited to 'src/test')
-rw-r--r-- | src/test/regress/expected/type_sanity.out | 19 | ||||
-rw-r--r-- | src/test/regress/regress.c | 8 | ||||
-rw-r--r-- | src/test/regress/sql/type_sanity.sql | 20 |
3 files changed, 47 insertions, 0 deletions
diff --git a/src/test/regress/expected/type_sanity.out b/src/test/regress/expected/type_sanity.out index 8eff3d10d27..dd0c52ab08b 100644 --- a/src/test/regress/expected/type_sanity.out +++ b/src/test/regress/expected/type_sanity.out @@ -11,6 +11,10 @@ -- that is OID or REGPROC fields that are not zero and do not match some -- row in the linked-to table. However, if we want to enforce that a link -- field can't be 0, we have to check it here. +-- directory paths and dlsuffix are passed to us in environment variables +\getenv libdir PG_LIBDIR +\getenv dlsuffix PG_DLSUFFIX +\set regresslib :libdir '/regress' :dlsuffix -- **************** pg_type **************** -- Look for illegal values in pg_type fields. SELECT t1.oid, t1.typname @@ -587,6 +591,21 @@ WHERE a1.atttypid = t1.oid AND ----------+---------+-----+--------- (0 rows) +-- Look for IsCatalogTextUniqueIndexOid() omissions. +CREATE FUNCTION is_catalog_text_unique_index_oid(oid) RETURNS bool + AS :'regresslib', 'is_catalog_text_unique_index_oid' + LANGUAGE C STRICT; +SELECT indexrelid::regclass +FROM pg_index +WHERE (is_catalog_text_unique_index_oid(indexrelid) <> + (indisunique AND + indexrelid < 16384 AND + EXISTS (SELECT 1 FROM pg_attribute + WHERE attrelid = indexrelid AND atttypid = 'text'::regtype))); + indexrelid +------------ +(0 rows) + -- **************** pg_range **************** -- Look for illegal values in pg_range fields. SELECT r.rngtypid, r.rngsubtype diff --git a/src/test/regress/regress.c b/src/test/regress/regress.c index 837fab6b290..3dbba069024 100644 --- a/src/test/regress/regress.c +++ b/src/test/regress/regress.c @@ -21,6 +21,7 @@ #include "access/detoast.h" #include "access/htup_details.h" +#include "catalog/catalog.h" #include "catalog/namespace.h" #include "catalog/pg_operator.h" #include "catalog/pg_type.h" @@ -722,6 +723,13 @@ test_fdw_handler(PG_FUNCTION_ARGS) PG_RETURN_NULL(); } +PG_FUNCTION_INFO_V1(is_catalog_text_unique_index_oid); +Datum +is_catalog_text_unique_index_oid(PG_FUNCTION_ARGS) +{ + return IsCatalogTextUniqueIndexOid(PG_GETARG_OID(0)); +} + PG_FUNCTION_INFO_V1(test_support_func); Datum test_support_func(PG_FUNCTION_ARGS) diff --git a/src/test/regress/sql/type_sanity.sql b/src/test/regress/sql/type_sanity.sql index 303f90955d1..c94dd83d306 100644 --- a/src/test/regress/sql/type_sanity.sql +++ b/src/test/regress/sql/type_sanity.sql @@ -12,6 +12,12 @@ -- row in the linked-to table. However, if we want to enforce that a link -- field can't be 0, we have to check it here. +-- directory paths and dlsuffix are passed to us in environment variables +\getenv libdir PG_LIBDIR +\getenv dlsuffix PG_DLSUFFIX + +\set regresslib :libdir '/regress' :dlsuffix + -- **************** pg_type **************** -- Look for illegal values in pg_type fields. @@ -425,6 +431,20 @@ WHERE a1.atttypid = t1.oid AND a1.attbyval != t1.typbyval OR (a1.attstorage != t1.typstorage AND a1.attstorage != 'p')); +-- Look for IsCatalogTextUniqueIndexOid() omissions. + +CREATE FUNCTION is_catalog_text_unique_index_oid(oid) RETURNS bool + AS :'regresslib', 'is_catalog_text_unique_index_oid' + LANGUAGE C STRICT; + +SELECT indexrelid::regclass +FROM pg_index +WHERE (is_catalog_text_unique_index_oid(indexrelid) <> + (indisunique AND + indexrelid < 16384 AND + EXISTS (SELECT 1 FROM pg_attribute + WHERE attrelid = indexrelid AND atttypid = 'text'::regtype))); + -- **************** pg_range **************** -- Look for illegal values in pg_range fields. |