Kluge slot_compile_deform() to ignore incorrect attnotnull markings.
authorTom Lane <tgl@sss.pgh.pa.us>
Mon, 20 Jul 2020 19:54:24 +0000 (15:54 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Mon, 20 Jul 2020 19:54:24 +0000 (15:54 -0400)
Since we mustn't force an initdb in released branches, there is no
simple way to correct the markings of pg_subscription.subslotname
and pg_subscription_rel.srsublsn as attnotnull in existing pre-v13
installations.

Fortunately, released branches don't rely on attnotnull being correct
for much.  The planner looks at it in relation_excluded_by_constraints,
but it'd be difficult to get that to matter for a query on a system
catalog.  The only place where it's really problematic is in JIT's
slot_compile_deform(), which can produce incorrect code that crashes
if there are NULLs in an allegedly not-null column.

Hence, hack up slot_compile_deform() to be specifically aware of
these two incorrect markings and not trust them.

This applies to v11 and v12; the JIT code didn't exist before that,
and we've fixed the markings in v13.

Discussion: https://postgr.es/m/229396.1595191345@sss.pgh.pa.us

src/backend/jit/llvm/llvmjit_deform.c
src/test/regress/expected/subscription.out
src/test/regress/sql/subscription.sql

index e406d392d4e16e325d647a877b63b8afb4bedc11..6384ac940d803597221146ad33479a0cfad596b0 100644 (file)
 
 #include "access/htup_details.h"
 #include "access/tupdesc_details.h"
+#include "catalog/pg_subscription.h"
+#include "catalog/pg_subscription_rel.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 and pg_subscription_rel.srsublsn marked as
+ * attnotnull, which they 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) || \
+      ((att)->attrelid == SubscriptionRelRelationId && \
+       (att)->attnum == Anum_pg_subscription_rel_srsublsn)))
+
+
 /*
  * Create a function that deforms a tuple of type desc up to natts columns.
  */
@@ -113,7 +129,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, int natts)
         * it's guaranteed that all columns up to here exist at least in the
         * NULL bitmap.
         */
-       if (att->attnotnull)
+       if (ATTNOTNULL(att))
            guaranteed_column_number = attnum;
    }
 
@@ -376,7 +392,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, int natts)
         * into account, because in case 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;
@@ -557,7 +573,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, int natts)
            known_alignment = -1;
            attguaranteedalign = false;
        }
-       else if (att->attnotnull && attguaranteedalign && known_alignment >= 0)
+       else if (ATTNOTNULL(att) && attguaranteedalign && known_alignment >= 0)
        {
            /*
             * If the offset to the column was previously known a NOT NULL &
@@ -567,7 +583,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc, int natts)
            Assert(att->attlen > 0);
            known_alignment += att->attlen;
        }
-       else if (att->attnotnull && (att->attlen % alignto) == 0)
+       else if (ATTNOTNULL(att) && (att->attlen % alignto) == 0)
        {
            /*
             * After a NOT NULL fixed-width column with a length that is a
index 4fcbf7efe97fa63b267bf0fc6fea9527aac92804..9aa0b702d79ae16b770d27283b472b87a826b234 100644 (file)
@@ -147,6 +147,13 @@ DROP SUBSCRIPTION testsub;
 ERROR:  DROP SUBSCRIPTION cannot run inside a transaction block
 COMMIT;
 ALTER SUBSCRIPTION testsub SET (slot_name = NONE);
+\dRs+
+                                              List of subscriptions
+  Name   |           Owner            | Enabled |     Publication     | Synchronous commit |       Conninfo       
+---------+----------------------------+---------+---------------------+--------------------+----------------------
+ testsub | regress_subscription_user2 | f       | {testpub2,testpub3} | local              | dbname=doesnotexist2
+(1 row)
+
 -- now it works
 BEGIN;
 DROP SUBSCRIPTION testsub;
index 36fa1bbac806639bbb6871934bd3890e1f59f147..5efff08c6d04345bd1f841bb3a9a68d5cba04dcb 100644 (file)
@@ -109,6 +109,8 @@ COMMIT;
 
 ALTER SUBSCRIPTION testsub SET (slot_name = NONE);
 
+\dRs+
+
 -- now it works
 BEGIN;
 DROP SUBSCRIPTION testsub;