From: David Rowley <drowley@postgresql.org>
Date: Thu, 6 Nov 2025 20:50:02 +0000 (+1300)
Subject: Tidyup WARNING ereports in subscriptioncmds.c
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=448b6a4173d007c75ba30fed666b60f0bd1afe8b;p=thirdparty%2Fpostgresql.git

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 2b0c0ca8d05..5930e8c5816 100644
--- 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);