Re: pg_subscription.subslotname is wrongly marked NOT NULL

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;