From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Subject: Re: pg_subscription.subslotname is wrongly marked NOT NULL
Date: Sun, 19 Jul 2020 19:48:18 -0400
Message-ID: <367660.1595202498@sss.pgh.pa.us> (raw)
In-Reply-To: <298837.1595196283@sss.pgh.pa.us>
References: <4118109.1595096139@sss.pgh.pa.us>
<34984.1595124581@sss.pgh.pa.us>
<298837.1595196283@sss.pgh.pa.us>
I wrote:
> pg_subscription_rel.srsublsn also needs to be marked nullable.
Not only is it wrongly marked attnotnull, but two of the three places
that read it are doing so unsafely (ie, as though it *were*
non-nullable). So I think we'd better fix it as attached.
regards, tom lane
Attachments:fix-srsublsn-usage-for-nullness.patch (text/x-diff, 3.2K) downloaddiff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgmlindex 80c183b235..13c871d2a8 100644--- a/doc/src/sgml/catalogs.sgml+++ b/doc/src/sgml/catalogs.sgml@@ -7631,7 +7631,9 @@ SCRAM-SHA-256$<replaceable><iteration count></replaceable>:<replaceable>&l
<structfield>srsublsn</structfield> <type>pg_lsn</type>
</para>
<para>
- End LSN for <literal>s</literal> and <literal>r</literal> states.+ Remote LSN of the state change used for synchronization coordination+ when in <literal>s</literal> or <literal>r</literal> states,+ otherwise null
</para></entry>
</row>
</tbody>
diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.cindex e6afb3203e..90bf5cf0c6 100644--- a/src/backend/catalog/pg_subscription.c+++ b/src/backend/catalog/pg_subscription.c@@ -452,13 +452,20 @@ GetSubscriptionRelations(Oid subid)
{
Form_pg_subscription_rel subrel;
SubscriptionRelState *relstate;
+ Datum d;+ bool isnull;
subrel = (Form_pg_subscription_rel) GETSTRUCT(tup);
relstate = (SubscriptionRelState *) palloc(sizeof(SubscriptionRelState));
relstate->relid = subrel->srrelid;
relstate->state = subrel->srsubstate;
- relstate->lsn = subrel->srsublsn;+ d = SysCacheGetAttr(SUBSCRIPTIONRELMAP, tup,+ Anum_pg_subscription_rel_srsublsn, &isnull);+ if (isnull)+ relstate->lsn = InvalidXLogRecPtr;+ else+ relstate->lsn = DatumGetLSN(d);
res = lappend(res, relstate);
}
@@ -504,13 +511,20 @@ GetSubscriptionNotReadyRelations(Oid subid)
{
Form_pg_subscription_rel subrel;
SubscriptionRelState *relstate;
+ Datum d;+ bool isnull;
subrel = (Form_pg_subscription_rel) GETSTRUCT(tup);
relstate = (SubscriptionRelState *) palloc(sizeof(SubscriptionRelState));
relstate->relid = subrel->srrelid;
relstate->state = subrel->srsubstate;
- relstate->lsn = subrel->srsublsn;+ d = SysCacheGetAttr(SUBSCRIPTIONRELMAP, tup,+ Anum_pg_subscription_rel_srsublsn, &isnull);+ if (isnull)+ relstate->lsn = InvalidXLogRecPtr;+ else+ relstate->lsn = DatumGetLSN(d);
res = lappend(res, relstate);
}
diff --git a/src/include/catalog/pg_subscription_rel.h b/src/include/catalog/pg_subscription_rel.hindex c44df04c72..f384f4e7fa 100644--- a/src/include/catalog/pg_subscription_rel.h+++ b/src/include/catalog/pg_subscription_rel.h@@ -33,8 +33,18 @@ CATALOG(pg_subscription_rel,6102,SubscriptionRelRelationId)
Oid srsubid; /* Oid of subscription */
Oid srrelid; /* Oid of relation */
char srsubstate; /* state of the relation in subscription */
- XLogRecPtr srsublsn; /* remote lsn of the state change used for- * synchronization coordination */++ /*+ * Although srsublsn is a fixed-width type, it is allowed to be NULL, so+ * we prevent direct C code access to it just as for a varlena field.+ */+#ifdef CATALOG_VARLEN /* variable-length fields start here */++ XLogRecPtr srsublsn BKI_FORCE_NULL; /* remote LSN of the state change+ * used for synchronization+ * coordination, or NULL if not+ * valid */+#endif
} FormData_pg_subscription_rel;
typedef FormData_pg_subscription_rel *Form_pg_subscription_rel;
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
Subject: Re: pg_subscription.subslotname is wrongly marked NOT NULL
In-Reply-To: <367660.1595202498@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