Teach pg_dump about the new pg_subscription.subrunasowner option.
authorTom Lane <tgl@sss.pgh.pa.us>
Sun, 29 Oct 2023 16:56:24 +0000 (12:56 -0400)
committerTom Lane <tgl@sss.pgh.pa.us>
Sun, 29 Oct 2023 16:56:35 +0000 (12:56 -0400)
Among numerous other oversights, commit 482675987 neglected to fix
pg_dump to dump this new subscription option.  Since the new default
is "false" while the previous behavior corresponds to "true", this
would cause legacy subscriptions to silently change behavior during
dump/reload or pg_upgrade.  That seems like a bad idea.  Even if it
was intended, failing to preserve the option once set in a new
installation is certainly not OK.

While here, reorder associated stanzas in pg_dump to match the
field order in pg_subscription, in hopes of reducing the impression
that all this code was written with the aid of a dartboard.

Back-patch to v16 where this new field was added.

Philip Warner (cosmetic tweaks by me)

Discussion: https://postgr.es/m/20231027042539.01A3A220F0A@thebes.rime.com.au

src/bin/pg_dump/pg_dump.c
src/bin/pg_dump/pg_dump.h

index 7afdbf4d9de22475bf2f6b340ed021dcbee39d70..e863913849240f3fff3d72acfdea570c43aba979 100644 (file)
@@ -4596,16 +4596,17 @@ getSubscriptions(Archive *fout)
    int         i_oid;
    int         i_subname;
    int         i_subowner;
+   int         i_subbinary;
    int         i_substream;
    int         i_subtwophasestate;
    int         i_subdisableonerr;
-   int         i_suborigin;
+   int         i_subpasswordrequired;
+   int         i_subrunasowner;
    int         i_subconninfo;
    int         i_subslotname;
    int         i_subsynccommit;
    int         i_subpublications;
-   int         i_subbinary;
-   int         i_subpasswordrequired;
+   int         i_suborigin;
    int         i,
                ntups;
 
@@ -4659,12 +4660,14 @@ getSubscriptions(Archive *fout)
 
    if (fout->remoteVersion >= 160000)
        appendPQExpBufferStr(query,
-                            " s.suborigin,\n"
-                            " s.subpasswordrequired\n");
+                            " s.subpasswordrequired,\n"
+                            " s.subrunasowner,\n"
+                            " s.suborigin\n");
    else
        appendPQExpBuffer(query,
-                         " '%s' AS suborigin,\n"
-                         " 't' AS subpasswordrequired\n",
+                         " 't' AS subpasswordrequired,\n"
+                         " 't' AS subrunasowner,\n"
+                         " '%s' AS suborigin\n",
                          LOGICALREP_ORIGIN_ANY);
 
    appendPQExpBufferStr(query,
@@ -4684,16 +4687,17 @@ getSubscriptions(Archive *fout)
    i_oid = PQfnumber(res, "oid");
    i_subname = PQfnumber(res, "subname");
    i_subowner = PQfnumber(res, "subowner");
-   i_subconninfo = PQfnumber(res, "subconninfo");
-   i_subslotname = PQfnumber(res, "subslotname");
-   i_subsynccommit = PQfnumber(res, "subsynccommit");
-   i_subpublications = PQfnumber(res, "subpublications");
    i_subbinary = PQfnumber(res, "subbinary");
    i_substream = PQfnumber(res, "substream");
    i_subtwophasestate = PQfnumber(res, "subtwophasestate");
    i_subdisableonerr = PQfnumber(res, "subdisableonerr");
-   i_suborigin = PQfnumber(res, "suborigin");
    i_subpasswordrequired = PQfnumber(res, "subpasswordrequired");
+   i_subrunasowner = PQfnumber(res, "subrunasowner");
+   i_subconninfo = PQfnumber(res, "subconninfo");
+   i_subslotname = PQfnumber(res, "subslotname");
+   i_subsynccommit = PQfnumber(res, "subsynccommit");
+   i_subpublications = PQfnumber(res, "subpublications");
+   i_suborigin = PQfnumber(res, "suborigin");
 
    subinfo = pg_malloc(ntups * sizeof(SubscriptionInfo));
 
@@ -4706,15 +4710,7 @@ getSubscriptions(Archive *fout)
        AssignDumpId(&subinfo[i].dobj);
        subinfo[i].dobj.name = pg_strdup(PQgetvalue(res, i, i_subname));
        subinfo[i].rolname = getRoleName(PQgetvalue(res, i, i_subowner));
-       subinfo[i].subconninfo = pg_strdup(PQgetvalue(res, i, i_subconninfo));
-       if (PQgetisnull(res, i, i_subslotname))
-           subinfo[i].subslotname = NULL;
-       else
-           subinfo[i].subslotname = pg_strdup(PQgetvalue(res, i, i_subslotname));
-       subinfo[i].subsynccommit =
-           pg_strdup(PQgetvalue(res, i, i_subsynccommit));
-       subinfo[i].subpublications =
-           pg_strdup(PQgetvalue(res, i, i_subpublications));
+
        subinfo[i].subbinary =
            pg_strdup(PQgetvalue(res, i, i_subbinary));
        subinfo[i].substream =
@@ -4723,9 +4719,22 @@ getSubscriptions(Archive *fout)
            pg_strdup(PQgetvalue(res, i, i_subtwophasestate));
        subinfo[i].subdisableonerr =
            pg_strdup(PQgetvalue(res, i, i_subdisableonerr));
-       subinfo[i].suborigin = pg_strdup(PQgetvalue(res, i, i_suborigin));
        subinfo[i].subpasswordrequired =
            pg_strdup(PQgetvalue(res, i, i_subpasswordrequired));
+       subinfo[i].subrunasowner =
+           pg_strdup(PQgetvalue(res, i, i_subrunasowner));
+       subinfo[i].subconninfo =
+           pg_strdup(PQgetvalue(res, i, i_subconninfo));
+       if (PQgetisnull(res, i, i_subslotname))
+           subinfo[i].subslotname = NULL;
+       else
+           subinfo[i].subslotname =
+               pg_strdup(PQgetvalue(res, i, i_subslotname));
+       subinfo[i].subsynccommit =
+           pg_strdup(PQgetvalue(res, i, i_subsynccommit));
+       subinfo[i].subpublications =
+           pg_strdup(PQgetvalue(res, i, i_subpublications));
+       subinfo[i].suborigin = pg_strdup(PQgetvalue(res, i, i_suborigin));
 
        /* Decide whether we want to dump it */
        selectDumpableObject(&(subinfo[i].dobj), fout);
@@ -4801,14 +4810,17 @@ dumpSubscription(Archive *fout, const SubscriptionInfo *subinfo)
    if (strcmp(subinfo->subdisableonerr, "t") == 0)
        appendPQExpBufferStr(query, ", disable_on_error = true");
 
-   if (pg_strcasecmp(subinfo->suborigin, LOGICALREP_ORIGIN_ANY) != 0)
-       appendPQExpBuffer(query, ", origin = %s", subinfo->suborigin);
+   if (strcmp(subinfo->subpasswordrequired, "t") != 0)
+       appendPQExpBuffer(query, ", password_required = false");
+
+   if (strcmp(subinfo->subrunasowner, "t") == 0)
+       appendPQExpBufferStr(query, ", run_as_owner = true");
 
    if (strcmp(subinfo->subsynccommit, "off") != 0)
        appendPQExpBuffer(query, ", synchronous_commit = %s", fmtId(subinfo->subsynccommit));
 
-   if (strcmp(subinfo->subpasswordrequired, "t") != 0)
-       appendPQExpBuffer(query, ", password_required = false");
+   if (pg_strcasecmp(subinfo->suborigin, LOGICALREP_ORIGIN_ANY) != 0)
+       appendPQExpBuffer(query, ", origin = %s", subinfo->suborigin);
 
    appendPQExpBufferStr(query, ");\n");
 
index d8f27f187cb1567af79f33e0dfcb760a5b64d3f4..2fe3cbed9ac2b335529631fb89236747d851f0af 100644 (file)
@@ -660,16 +660,17 @@ typedef struct _SubscriptionInfo
 {
    DumpableObject dobj;
    const char *rolname;
-   char       *subconninfo;
-   char       *subslotname;
    char       *subbinary;
    char       *substream;
    char       *subtwophasestate;
    char       *subdisableonerr;
-   char       *suborigin;
+   char       *subpasswordrequired;
+   char       *subrunasowner;
+   char       *subconninfo;
+   char       *subslotname;
    char       *subsynccommit;
    char       *subpublications;
-   char       *subpasswordrequired;
+   char       *suborigin;
 } SubscriptionInfo;
 
 /*