From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Cc: Andres Freund <andres@anarazel.de>
Subject: Re: pg_subscription.subslotname is wrongly marked NOT NULL
Date: Sun, 19 Jul 2020 16:42:25 -0400
Message-ID: <229396.1595191345@sss.pgh.pa.us> (raw)
In-Reply-To: <34984.1595124581@sss.pgh.pa.us>
References: <4118109.1595096139@sss.pgh.pa.us>
<34984.1595124581@sss.pgh.pa.us>
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
Attachments:work-around-bogus-attnotnull-v12.patch (text/x-diff, 3.3K) downloaddiff --git a/src/backend/jit/llvm/llvmjit_deform.c b/src/backend/jit/llvm/llvmjit_deform.cindex 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.outindex 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.sqlindex 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;
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: tgl@sss.pgh.pa.us, pgsql-hackers@lists.postgresql.org, andres@anarazel.de
Subject: Re: pg_subscription.subslotname is wrongly marked NOT NULL
In-Reply-To: <229396.1595191345@sss.pgh.pa.us>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox