Tidyup WARNING ereports in subscriptioncmds.c

A couple of ereports were making use of StringInfos as temporary storage
for the portions of the WARNING message.  One was doing this to avoid
having 2 separate ereports.  This was all fairly unnecessary and
resulted in more code rather than less code.

Refactor out the additional StringInfos and make
check_publications_origin_tables() use 2 ereports.

In passing, adjust pubnames to become a stack-allocated StringInfoData to
avoid having to palloc the temporary StringInfoData.  This follows on
from the efforts made in 6d0eba662.

Author: Mats Kindahl <mats.kindahl@gmail.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Amit Kapila <amit.kapila16@gmail.com>
Reviewed-by: Álvaro Herrera <alvherre@kurilemu.de>
Discussion: https://postgr.es/m/0b381b02-cab9-41f9-a900-ad6c8d26c1fc%40gmail.com

diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c
index 2b0c0ca8d05fd46ef926633cb4181f4a653df4eb..5930e8c581624600143e410f8115dcb80a257ee2 100644 (file)
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -2599,33 +2599,30 @@ check_publications_origin_tables(WalReceiverConn *wrconn, List *publications,
         */
        if (publist)
        {
-               StringInfo      pubnames = makeStringInfo();
-               StringInfo      err_msg = makeStringInfo();
-               StringInfo      err_hint = makeStringInfo();
+               StringInfoData pubnames;
 
                /* Prepare the list of publication(s) for warning message. */
-               GetPublicationsStr(publist, pubnames, false);
+               initStringInfo(&pubnames);
+               GetPublicationsStr(publist, &pubnames, false);
 
                if (check_table_sync)
-               {
-                       appendStringInfo(err_msg, _("subscription \"%s\" requested copy_data with origin = NONE but might copy data that had a different origin"),
-                                                        subname);
-                       appendStringInfoString(err_hint, _("Verify that initial data copied from the publisher tables did not come from other origins."));
-               }
+                       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",
+                                                  subname),
+                                       errdetail_plural("The subscription subscribes to a publication (%s) that contains tables that are written to by other subscriptions.",
+                                                                        "The subscription subscribes to publications (%s) that contain tables that are written to by other subscriptions.",
+                                                                        list_length(publist), pubnames.data),
+                                       errhint("Verify that initial data copied from the publisher tables did not come from other origins."));
                else
-               {
-                       appendStringInfo(err_msg, _("subscription \"%s\" enabled retain_dead_tuples but might not reliably detect conflicts for changes from different origins"),
-                                                        subname);
-                       appendStringInfoString(err_hint, _("Consider using origin = NONE or disabling retain_dead_tuples."));
-               }
-
-               ereport(WARNING,
-                               errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-                               errmsg_internal("%s", err_msg->data),
-                               errdetail_plural("The subscription subscribes to a publication (%s) that contains tables that are written to by other subscriptions.",
-                                                                "The subscription subscribes to publications (%s) that contain tables that are written to by other subscriptions.",
-                                                                list_length(publist), pubnames->data),
-                               errhint_internal("%s", err_hint->data));
+                       ereport(WARNING,
+                                       errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
+                                       errmsg("subscription \"%s\" enabled retain_dead_tuples but might not reliably detect conflicts for changes from different origins",
+                                                  subname),
+                                       errdetail_plural("The subscription subscribes to a publication (%s) that contains tables that are written to by other subscriptions.",
+                                                                        "The subscription subscribes to publications (%s) that contain tables that are written to by other subscriptions.",
+                                                                        list_length(publist), pubnames.data),
+                                       errhint("Consider using origin = NONE or disabling retain_dead_tuples."));
        }
 
        ExecDropSingleTupleTableSlot(slot);
@@ -2716,24 +2713,20 @@ check_publications_origin_sequences(WalReceiverConn *wrconn, List *publications,
         */
        if (publist)
        {
-               StringInfo      pubnames = makeStringInfo();
-               StringInfo      err_msg = makeStringInfo();
-               StringInfo      err_hint = makeStringInfo();
+               StringInfoData pubnames;
 
                /* Prepare the list of publication(s) for warning message. */
-               GetPublicationsStr(publist, pubnames, false);
-
-               appendStringInfo(err_msg, _("subscription \"%s\" requested copy_data with origin = NONE but might copy data that had a different origin"),
-                                                subname);
-               appendStringInfoString(err_hint, _("Verify that initial data copied from the publisher sequences did not come from other origins."));
+               initStringInfo(&pubnames);
+               GetPublicationsStr(publist, &pubnames, false);
 
                ereport(WARNING,
                                errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
-                               errmsg_internal("%s", err_msg->data),
+                               errmsg("subscription \"%s\" requested copy_data with origin = NONE but might copy data that had a different origin",
+                                          subname),
                                errdetail_plural("The subscription subscribes to a publication (%s) that contains sequences that are written to by other subscriptions.",
                                                                 "The subscription subscribes to publications (%s) that contain sequences that are written to by other subscriptions.",
-                                                                list_length(publist), pubnames->data),
-                               errhint_internal("%s", err_hint->data));
+                                                                list_length(publist), pubnames.data),
+                               errhint("Verify that initial data copied from the publisher sequences did not come from other origins."));
        }
 
        ExecDropSingleTupleTableSlot(slot);