Refactor code converting a publication name List to a StringInfo

The existing get_publications_str() is renamed to GetPublicationsStr()
and is moved to pg_subscription.c, so as it is possible to reuse it at
two locations of the tablesync code where the same logic was duplicated.

fetch_remote_table_info() was doing two List->StringInfo conversions
when dealing with a server of version 15 or newer.  The conversion
happens only once now.

This refactoring leads to less code overall.

Author: Peter Smith
Reviewed-by: Michael Paquier, Masahiko Sawada
Discussion: https://postgr.es/m/CAHut+PtJMk4bKXqtpvqVy9ckknCgK9P6=FeG8zHF=6+Em_Snpw@mail.gmail.com

diff --git a/src/backend/catalog/pg_subscription.c b/src/backend/catalog/pg_subscription.c
index 9efc9159f2c22eb791317d8d2c0c862c59e90e4a..89bf5ec9337ec9e5d3aef65a9dac3dfa3ab89ff8 100644 (file)
--- a/src/backend/catalog/pg_subscription.c
+++ b/src/backend/catalog/pg_subscription.c
@@ -34,6 +34,37 @@
 
 static List *textarray_to_stringlist(ArrayType *textarray);
 
+/*
+ * Add a comma-separated list of publication names to the 'dest' string.
+ */
+void
+GetPublicationsStr(List *publications, StringInfo dest, bool quote_literal)
+{
+       ListCell   *lc;
+       bool            first = true;
+
+       Assert(publications != NIL);
+
+       foreach(lc, publications)
+       {
+               char       *pubname = strVal(lfirst(lc));
+
+               if (first)
+                       first = false;
+               else
+                       appendStringInfoString(dest, ", ");
+
+               if (quote_literal)
+                       appendStringInfoString(dest, quote_literal_cstr(pubname));
+               else
+               {
+                       appendStringInfoChar(dest, '"');
+                       appendStringInfoString(dest, pubname);
+                       appendStringInfoChar(dest, '"');
+               }
+       }
+}
+
 /*
  * Fetch the subscription from the syscache.
  */

diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 02ccc636b807c973172a1f247259918ead19d6fb..33575614e727e8c07713367f32e7c1b81cd001a8 100644 (file)
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -439,37 +439,6 @@ parse_subscription_options(ParseState *pstate, List *stmt_options,
        }
 }
 
-/*
- * Add publication names from the list to a string.
- */
-static void
-get_publications_str(List *publications, StringInfo dest, bool quote_literal)
-{
-       ListCell   *lc;
-       bool            first = true;
-
-       Assert(publications != NIL);
-
-       foreach(lc, publications)
-       {
-               char       *pubname = strVal(lfirst(lc));
-
-               if (first)
-                       first = false;
-               else
-                       appendStringInfoString(dest, ", ");
-
-               if (quote_literal)
-                       appendStringInfoString(dest, quote_literal_cstr(pubname));
-               else
-               {
-                       appendStringInfoChar(dest, '"');
-                       appendStringInfoString(dest, pubname);
-                       appendStringInfoChar(dest, '"');
-               }
-       }
-}
-
 /*
  * Check that the specified publications are present on the publisher.
  */
@@ -486,7 +455,7 @@ check_publications(WalReceiverConn *wrconn, List *publications)
        appendStringInfoString(cmd, "SELECT t.pubname FROM\n"
                                                   " pg_catalog.pg_publication t WHERE\n"
                                                   " t.pubname IN (");
-       get_publications_str(publications, cmd, true);
+       GetPublicationsStr(publications, cmd, true);
        appendStringInfoChar(cmd, ')');
 
        res = walrcv_exec(wrconn, cmd->data, 1, tableRow);
@@ -523,7 +492,7 @@ check_publications(WalReceiverConn *wrconn, List *publications)
                /* Prepare the list of non-existent publication(s) for error message. */
                StringInfo      pubnames = makeStringInfo();
 
-               get_publications_str(publicationsCopy, pubnames, false);
+               GetPublicationsStr(publicationsCopy, pubnames, false);
                ereport(WARNING,
                                errcode(ERRCODE_UNDEFINED_OBJECT),
                                errmsg_plural("publication %s does not exist on the publisher",
@@ -2151,7 +2120,7 @@ check_publications_origin(WalReceiverConn *wrconn, List *publications,
                                                   "     JOIN pg_subscription_rel PS ON (GPT.relid = PS.srrelid),\n"
                                                   "     pg_class C JOIN pg_namespace N ON (N.oid = C.relnamespace)\n"
                                                   "WHERE C.oid = GPT.relid AND P.pubname IN (");
-       get_publications_str(publications, &cmd, true);
+       GetPublicationsStr(publications, &cmd, true);
        appendStringInfoString(&cmd, ")\n");
 
        /*
@@ -2208,7 +2177,7 @@ check_publications_origin(WalReceiverConn *wrconn, List *publications,
                StringInfo      pubnames = makeStringInfo();
 
                /* Prepare the list of publication(s) for warning message. */
-               get_publications_str(publist, pubnames, false);
+               GetPublicationsStr(publist, pubnames, false);
                ereport(WARNING,
                                errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
                                errmsg("subscription \"%s\" requested copy_data with origin = NONE but might copy data that had a different origin",
@@ -2243,17 +2212,17 @@ fetch_table_list(WalReceiverConn *wrconn, List *publications)
        List       *tablelist = NIL;
        int                     server_version = walrcv_server_version(wrconn);
        bool            check_columnlist = (server_version >= 150000);
+       StringInfo      pub_names = makeStringInfo();
 
        initStringInfo(&cmd);
 
+       /* Build the pub_names comma-separated string. */
+       GetPublicationsStr(publications, pub_names, true);
+
        /* Get the list of tables from the publisher. */
        if (server_version >= 160000)
        {
-               StringInfoData pub_names;
-
                tableRow[2] = INT2VECTOROID;
-               initStringInfo(&pub_names);
-               get_publications_str(publications, &pub_names, true);
 
                /*
                 * From version 16, we allowed passing multiple publications to the
@@ -2275,9 +2244,7 @@ fetch_table_list(WalReceiverConn *wrconn, List *publications)
                                                 "                FROM pg_publication\n"
                                                 "                WHERE pubname IN ( %s )) AS gpt\n"
                                                 "             ON gpt.relid = c.oid\n",
-                                                pub_names.data);
-
-               pfree(pub_names.data);
+                                                pub_names->data);
        }
        else
        {
@@ -2288,12 +2255,13 @@ fetch_table_list(WalReceiverConn *wrconn, List *publications)
                if (check_columnlist)
                        appendStringInfoString(&cmd, ", t.attnames\n");
 
-               appendStringInfoString(&cmd, "FROM pg_catalog.pg_publication_tables t\n"
-                                                          " WHERE t.pubname IN (");
-               get_publications_str(publications, &cmd, true);
-               appendStringInfoChar(&cmd, ')');
+               appendStringInfo(&cmd, "FROM pg_catalog.pg_publication_tables t\n"
+                                                " WHERE t.pubname IN ( %s )",
+                                                pub_names->data);
        }
 
+       destroyStringInfo(pub_names);
+
        res = walrcv_exec(wrconn, cmd.data, check_columnlist ? 3 : 2, tableRow);
        pfree(cmd.data);
 

diff --git a/src/backend/replication/logical/tablesync.c b/src/backend/replication/logical/tablesync.c
index e03e761392667c6e01b8abd2e8f6c73f03212337..d4b5d210e3ebc51de600ec17aca750259b7f65a5 100644 (file)
--- a/src/backend/replication/logical/tablesync.c
+++ b/src/backend/replication/logical/tablesync.c
@@ -802,7 +802,7 @@ fetch_remote_table_info(char *nspname, char *relname,
        Oid                     qualRow[] = {TEXTOID};
        bool            isnull;
        int                     natt;
-       ListCell   *lc;
+       StringInfo      pub_names = NULL;
        Bitmapset  *included_cols = NULL;
 
        lrel->nspname = nspname;
@@ -856,15 +856,10 @@ fetch_remote_table_info(char *nspname, char *relname,
                WalRcvExecResult *pubres;
                TupleTableSlot *tslot;
                Oid                     attrsRow[] = {INT2VECTOROID};
-               StringInfoData pub_names;
 
-               initStringInfo(&pub_names);
-               foreach(lc, MySubscription->publications)
-               {
-                       if (foreach_current_index(lc) > 0)
-                               appendStringInfoString(&pub_names, ", ");
-                       appendStringInfoString(&pub_names, quote_literal_cstr(strVal(lfirst(lc))));
-               }
+               /* Build the pub_names comma-separated string. */
+               pub_names = makeStringInfo();
+               GetPublicationsStr(MySubscription->publications, pub_names, true);
 
                /*
                 * Fetch info about column lists for the relation (from all the
@@ -881,7 +876,7 @@ fetch_remote_table_info(char *nspname, char *relname,
                                                 " WHERE gpt.relid = %u AND c.oid = gpt.relid"
                                                 "   AND p.pubname IN ( %s )",
                                                 lrel->remoteid,
-                                                pub_names.data);
+                                                pub_names->data);
 
                pubres = walrcv_exec(LogRepWorkerWalRcvConn, cmd.data,
                                                         lengthof(attrsRow), attrsRow);
@@ -936,8 +931,6 @@ fetch_remote_table_info(char *nspname, char *relname,
                ExecDropSingleTupleTableSlot(tslot);
 
                walrcv_clear_result(pubres);
-
-               pfree(pub_names.data);
        }
 
        /*
@@ -1039,19 +1032,8 @@ fetch_remote_table_info(char *nspname, char *relname,
         */
        if (walrcv_server_version(LogRepWorkerWalRcvConn) >= 150000)
        {
-               StringInfoData pub_names;
-
-               /* Build the pubname list. */
-               initStringInfo(&pub_names);
-               foreach_node(String, pubstr, MySubscription->publications)
-               {
-                       char       *pubname = strVal(pubstr);
-
-                       if (foreach_current_index(pubstr) > 0)
-                               appendStringInfoString(&pub_names, ", ");
-
-                       appendStringInfoString(&pub_names, quote_literal_cstr(pubname));
-               }
+               /* Reuse the already-built pub_names. */
+               Assert(pub_names != NULL);
 
                /* Check for row filters. */
                resetStringInfo(&cmd);
@@ -1062,7 +1044,7 @@ fetch_remote_table_info(char *nspname, char *relname,
                                                 " WHERE gpt.relid = %u"
                                                 "   AND p.pubname IN ( %s )",
                                                 lrel->remoteid,
-                                                pub_names.data);
+                                                pub_names->data);
 
                res = walrcv_exec(LogRepWorkerWalRcvConn, cmd.data, 1, qualRow);
 
@@ -1101,6 +1083,7 @@ fetch_remote_table_info(char *nspname, char *relname,
                ExecDropSingleTupleTableSlot(slot);
 
                walrcv_clear_result(res);
+               destroyStringInfo(pub_names);
        }
 
        pfree(cmd.data);

diff --git a/src/include/catalog/pg_subscription.h b/src/include/catalog/pg_subscription.h
index 0aa14ec4a27ed168180648b5bc6874bfd49118db..b25f3fea566a0b6f992ed687b23d40bad99f37a2 100644 (file)
--- a/src/include/catalog/pg_subscription.h
+++ b/src/include/catalog/pg_subscription.h
@@ -20,7 +20,7 @@
 #include "access/xlogdefs.h"
 #include "catalog/genbki.h"
 #include "catalog/pg_subscription_d.h"
-
+#include "lib/stringinfo.h"
 #include "nodes/pg_list.h"
 
 /*
@@ -180,4 +180,7 @@ extern void DisableSubscription(Oid subid);
 
 extern int     CountDBSubscriptions(Oid dbid);
 
+extern void GetPublicationsStr(List *publications, StringInfo dest,
+                                                          bool quote_literal);
+
 #endif                                                 /* PG_SUBSCRIPTION_H */