Kluge slot_compile_deform() to ignore incorrect attnotnull markings.

tglsfdc · tglsfdc · commit 798b4faefd20 · 2020-07-20T15:54:24.000-04:00
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
diff --git a/src/backend/jit/llvm/llvmjit_deform.c b/src/backend/jit/llvm/llvmjit_deform.c
@@ -22,11 +22,27 @@
 
 #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.
  */
@@ -121,7 +137,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 +435,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;
@@ -600,7 +616,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc,
 			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 &
@@ -610,7 +626,7 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc,
 			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
diff --git 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
@@ -109,6 +109,8 @@ COMMIT;
 
 ALTER SUBSCRIPTION regress_testsub SET (slot_name = NONE);
 
+\dRs+
+
 -- now it works
 BEGIN;
 DROP SUBSCRIPTION regress_testsub;
