Received: from malur.postgresql.org ([217.196.149.56])
	by arkaria.postgresql.org with esmtps
 (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256)
	(Exim 4.92)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1jxJ2e-00087b-JI
	for pgsql-hackers@arkaria.postgresql.org; Sun, 19 Jul 2020 23:48:24 +0000
Received: from localhost ([127.0.0.1] helo=malur.postgresql.org)
	by malur.postgresql.org with esmtp (Exim 4.92)
	(envelope-from <pgsql-hackers-owner+archive@lists.postgresql.org>)
	id 1jxJ2c-0003DJ-H7
	for pgsql-hackers@arkaria.postgresql.org; Sun, 19 Jul 2020 23:48:22 +0000
Received: from makus.postgresql.org ([2001:4800:3e1:1::229])
	by malur.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256)
	(Exim 4.92)
	(envelope-from <tgl@sss.pgh.pa.us>)
	id 1jxJ2c-0003DB-8f
	for pgsql-hackers@lists.postgresql.org; Sun, 19 Jul 2020 23:48:22 +0000
Received: from sss.pgh.pa.us ([66.207.139.130])
	by makus.postgresql.org with esmtps (TLS1.3:ECDHE_RSA_AES_256_GCM_SHA384:256)
	(Exim 4.92)
	(envelope-from <tgl@sss.pgh.pa.us>)
	id 1jxJ2Z-00084A-Vt
	for pgsql-hackers@lists.postgresql.org; Sun, 19 Jul 2020 23:48:21 +0000
Received: from sss1.sss.pgh.pa.us (localhost [127.0.0.1])
	by sss.pgh.pa.us (8.15.2/8.15.2) with ESMTP id 06JNmIjx367661
	for <pgsql-hackers@lists.postgresql.org>; Sun, 19 Jul 2020 19:48:18 -0400
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-hackers@lists.postgresql.org
Subject: Re: pg_subscription.subslotname is wrongly marked NOT NULL
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>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Sun, 19 Jul 2020 18:04:43 -0400"
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="----- =_aaaaaaaaaa0"
Content-ID: <367639.1595202459.0@sss.pgh.pa.us>
Date: Sun, 19 Jul 2020 19:48:18 -0400
Message-ID: <367660.1595202498@sss.pgh.pa.us>
List-Id: <pgsql-hackers.lists.postgresql.org>
List-Help: <https://lists.postgresql.org/manage/>
List-Subscribe: <https://lists.postgresql.org/manage/>
List-Post: <mailto:pgsql-hackers@lists.postgresql.org>
List-Owner: <mailto:pgsql-hackers-owner@lists.postgresql.org>
List-Archive: <https://www.postgresql.org/list/pgsql-hackers>
Precedence: bulk

------- =_aaaaaaaaaa0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <367639.1595202459.1@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


------- =_aaaaaaaaaa0
Content-Type: text/x-diff; name="fix-srsublsn-usage-for-nullness.patch";
	charset="us-ascii"
Content-ID: <367639.1595202459.2@sss.pgh.pa.us>
Content-Description: fix-srsublsn-usage-for-nullness.patch
Content-Transfer-Encoding: quoted-printable

diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml
index 80c183b235..13c871d2a8 100644
--- a/doc/src/sgml/catalogs.sgml
+++ b/doc/src/sgml/catalogs.sgml
@@ -7631,7 +7631,9 @@ SCRAM-SHA-256$<replaceable>&lt;iteration count&gt;</=
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 coordinati=
on
+       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/p=
g_subscription.c
index 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 =3D (Form_pg_subscription_rel) GETSTRUCT(tup);
 =

 		relstate =3D (SubscriptionRelState *) palloc(sizeof(SubscriptionRelStat=
e));
 		relstate->relid =3D subrel->srrelid;
 		relstate->state =3D subrel->srsubstate;
-		relstate->lsn =3D subrel->srsublsn;
+		d =3D SysCacheGetAttr(SUBSCRIPTIONRELMAP, tup,
+							Anum_pg_subscription_rel_srsublsn, &isnull);
+		if (isnull)
+			relstate->lsn =3D InvalidXLogRecPtr;
+		else
+			relstate->lsn =3D DatumGetLSN(d);
 =

 		res =3D lappend(res, relstate);
 	}
@@ -504,13 +511,20 @@ GetSubscriptionNotReadyRelations(Oid subid)
 	{
 		Form_pg_subscription_rel subrel;
 		SubscriptionRelState *relstate;
+		Datum		d;
+		bool		isnull;
 =

 		subrel =3D (Form_pg_subscription_rel) GETSTRUCT(tup);
 =

 		relstate =3D (SubscriptionRelState *) palloc(sizeof(SubscriptionRelStat=
e));
 		relstate->relid =3D subrel->srrelid;
 		relstate->state =3D subrel->srsubstate;
-		relstate->lsn =3D subrel->srsublsn;
+		d =3D SysCacheGetAttr(SUBSCRIPTIONRELMAP, tup,
+							Anum_pg_subscription_rel_srsublsn, &isnull);
+		if (isnull)
+			relstate->lsn =3D InvalidXLogRecPtr;
+		else
+			relstate->lsn =3D DatumGetLSN(d);
 =

 		res =3D lappend(res, relstate);
 	}
diff --git a/src/include/catalog/pg_subscription_rel.h b/src/include/catal=
og/pg_subscription_rel.h
index 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,SubscriptionRelRelatio=
nId)
 	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;

------- =_aaaaaaaaaa0--