From 914d2383ae91918b359311a90d23c0d5862781ac Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 19 Jul 2020 12:37:23 -0400 Subject: [PATCH] Correctly mark pg_subscription.subslotname as nullable. Due to the layout of this catalog, subslotname has to be explicitly marked BKI_FORCE_NULL, else initdb will default to the assumption that it's non-nullable. Since, in fact, CREATE/ALTER SUBSCRIPTION will store null values there, the existing marking is just wrong, and has been since this catalog was invented. We haven't noticed because not much in the system actually depends on attnotnull being truthful. However, JIT'ed tuple deconstruction does depend on that in some cases, allowing crashes or wrong answers in queries that inspect pg_subscription. Commit 9de77b545 quite accidentally exposed this on the buildfarm members that force JIT activation. Back-patch to v13. The problem goes further back, but we cannot force initdb in released branches, so some klugier solution will be needed there. Before working on that, push this simple fix to try to get the buildfarm back to green. Discussion: https://postgr.es/m/4118109.1595096139@sss.pgh.pa.us --- doc/src/sgml/catalogs.sgml | 5 +++-- src/include/catalog/catversion.h | 2 +- src/include/catalog/pg_subscription.h | 2 +- src/test/regress/expected/subscription.out | 7 +++++++ src/test/regress/sql/subscription.sql | 2 ++ 5 files changed, 14 insertions(+), 4 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 8ff17d9b4e0..6cb9e8e29ad 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -7517,8 +7517,9 @@ SCRAM-SHA-256$<iteration count>:&l subslotname name - Name of the replication slot in the upstream database. Also used - for local replication origin name. + Name of the replication slot in the upstream database (also used + for the local replication origin name); + null represents NONE diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h index 6b3aa7c0063..9d9334272e8 100644 --- a/src/include/catalog/catversion.h +++ b/src/include/catalog/catversion.h @@ -53,6 +53,6 @@ */ /* yyyymmddN */ -#define CATALOG_VERSION_NO 202007131 +#define CATALOG_VERSION_NO 202007191 #endif diff --git a/src/include/catalog/pg_subscription.h b/src/include/catalog/pg_subscription.h index 0a756d42d84..e3288ad76e9 100644 --- a/src/include/catalog/pg_subscription.h +++ b/src/include/catalog/pg_subscription.h @@ -53,7 +53,7 @@ CATALOG(pg_subscription,6100,SubscriptionRelationId) BKI_SHARED_RELATION BKI_ROW text subconninfo BKI_FORCE_NOT_NULL; /* Slot name on publisher */ - NameData subslotname; + NameData subslotname BKI_FORCE_NULL; /* Synchronous commit setting for worker */ text subsynccommit BKI_FORCE_NOT_NULL; diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out index e7add9d2b81..3ba1e5dcdd5 100644 --- a/src/test/regress/expected/subscription.out +++ b/src/test/regress/expected/subscription.out @@ -147,6 +147,13 @@ DROP SUBSCRIPTION regress_testsub; ERROR: DROP SUBSCRIPTION cannot run inside a transaction block COMMIT; ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE); +\dRs+ + List of subscriptions + Name | Owner | Enabled | Publication | Synchronous commit | Conninfo +-----------------+----------------------------+---------+---------------------+--------------------+------------------------------ + regress_testsub | regress_subscription_user2 | f | {testpub2,testpub3} | local | dbname=regress_doesnotexist2 +(1 row) + -- now it works BEGIN; DROP SUBSCRIPTION regress_testsub; diff --git a/src/test/regress/sql/subscription.sql b/src/test/regress/sql/subscription.sql index 9e234ab8b3f..1bc58887f7e 100644 --- a/src/test/regress/sql/subscription.sql +++ b/src/test/regress/sql/subscription.sql @@ -109,6 +109,8 @@ COMMIT; ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE); +\dRs+ + -- now it works BEGIN; DROP SUBSCRIPTION regress_testsub;