I wrote:
> (2) In pre-v13 branches, hack the JIT tuple deconstruction code
> to be specifically aware that it can't trust attnotnull for
> pg_subscription.subslotname. Yeah, it's ugly, but at least it's
> not ugly going forwards.
Concretely, as attached for v12.
regards, tom lane
diff --git a/src/backend/jit/llvm/llvmjit_deform.c b/src/backend/jit/llvm/llvmjit_deform.c
index 835aea83e9..4b2144e1a7 100644
--- a/src/backend/jit/llvm/llvmjit_deform.c
+++ b/src/backend/jit/llvm/llvmjit_deform.c
@@ -22,11 +22,24 @@
#include "access/htup_details.h"
#include "access/tupdesc_details.h"
+#include "catalog/pg_subscription.h"
#include "executor/tuptable.h"
#include "jit/llvmjit.h"
#include "jit/llvmjit_emit.h"
+/*
+ * Through an embarrassing oversight, pre-v13 installations may have
+ * pg_subscription.subslotname marked as attnotnull, which it should not be.
+ * To avoid possible crashes, use this macro instead of testing attnotnull
+ * directly.
+ */
+#define ATTNOTNULL(att) \
+ ((att)->attnotnull && \
+ ((att)->attrelid != SubscriptionRelationId || \
+ (att)->attnum != Anum_pg_subscription_subslotname))
+
+
/*
* Create a function that deforms a tuple of type desc up to natts columns.
*/
@@ -121,7 +134,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc,
* combination of attisdropped && attnotnull combination shouldn't
* exist.
*/
- if (att->attnotnull &&
+ if (ATTNOTNULL(att) &&
!att->atthasmissing &&
!att->attisdropped)
guaranteed_column_number = attnum;
@@ -419,7 +432,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc,
* into account, because if they're present the heaptuple's natts
* would have indicated that a slot_getmissingattrs() is needed.
*/
- if (!att->attnotnull)
+ if (!ATTNOTNULL(att))
{
LLVMBasicBlockRef b_ifnotnull;
LLVMBasicBlockRef b_ifnull;
@@ -586,6 +599,9 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc,
* to generate better code. Without that LLVM can't figure out that
* the offset might be constant due to the jumps for previously
* decoded columns.
+ *
+ * Note: these two tests on attnotnull don't need the ATTNOTNULL hack,
+ * because they are harmless on pg_subscription anyway.
*/
if (attguaranteedalign)
{
diff --git a/src/test/regress/expected/subscription.out b/src/test/regress/expected/subscription.out
index e7add9d2b8..3ba1e5dcdd 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 9e234ab8b3..1bc58887f7 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;
