Re: pg_subscription.subslotname is wrongly marked NOT NULL - Mailing list pgsql-hackers

From Tom Lane
Subject Re: pg_subscription.subslotname is wrongly marked NOT NULL
Date
Msg-id 367660.1595202498@sss.pgh.pa.us
Whole thread Raw
In response to Re: pg_subscription.subslotname is wrongly marked NOT NULL  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: pg_subscription.subslotname is wrongly marked NOT NULL  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-hackers
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

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><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.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 = (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.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,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;

pgsql-hackers by date:

Previous
From: Tom Lane
Date:
Subject: Re: pg_subscription.subslotname is wrongly marked NOT NULL
Next
From: Tomas Vondra
Date:
Subject: Re: Default setting for enable_hashagg_disk