From: Mats Kindahl <mats.kindahl@gmail.com>
To: pgsql-hackers@lists.postgresql.org
Cc: David Rowley <dgrowleyml@gmail.com>
Subject: Refactor StringInfo usage in subscriptioncmds.c
Date: Thu, 6 Nov 2025 12:38:07 +0100
Message-ID: <0b381b02-cab9-41f9-a900-ad6c8d26c1fc@gmail.com> (raw)
Hi all,
As discussed in [1] the functions check_publications_origin_tables() and
check_publications_origin_sequences() are building error messages using
dynamically allocated StringInfo instances only to avoid duplicating a
call of ereport().
Attached is a proposal that instead of building error message and hints
dynamically, it will use ereport() directly and as a result does not
have to allocate the error message strings and error message hints
dynamically and these can be removed.
This also means that a previous use of gettext() (in the form of the "_"
macro) is not needed any more and we can use errmsg() and errhint()
rather than errmsg_internal() and errhint_internal() with ereport().
It also replaces the usage a dynamically allocated StringInfo of
pubnames containing the publication names with a stack allocated
StringInfoData, along the same line as in [2].
[1]:
https://www.postgresql.org/message-id/CAApHDvrZnM28wa2VY58cvtY0y9XbMhKJH4m%3Dga3c1wfsx%3DMF4Q%40mail...
[2]:
https://www.postgresql.org/message-id/flat/CAApHDvrZnM28wa2VY58cvtY0y9XbMhKJH4m%3Dga3c1wfsx%3DMF4Q%4...Attachments:0001-Refactor-StringInfo-usage-in-subscriptioncmds.c.v1.patch (text/x-patch, 5.3K) download
From 2436de272d39e664fa0c0010cf875cb34fcecd1e Mon Sep 17 00:00:00 2001
From: Mats Kindahl <mats@kindahl.net>
Date: Thu, 6 Nov 2025 11:47:50 +0100
Subject: [PATCH] Refactor StringInfo usage in subscriptioncmds.c
This commit removes some uses of StringInfo that was not necessary and was only
there to avoid having to write two calls of ereport() as well as replacing
dynamic allocation of StringInfo with a StringInfoData usage instead.
---
src/backend/commands/subscriptioncmds.c | 61 ++++++++++++-------------
1 file changed, 28 insertions(+), 33 deletions(-)
diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.cindex 3d29818badd..bd457dc6e62 100644--- a/src/backend/commands/subscriptioncmds.c+++ b/src/backend/commands/subscriptioncmds.c@@ -2599,33 +2599,32 @@ 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."));++ pfree(pubnames.data);
}
ExecDropSingleTupleTableSlot(slot);
@@ -2716,24 +2715,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."));+ pfree(pubnames.data);
}
ExecDropSingleTupleTableSlot(slot);
--
2.43.0
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: mats.kindahl@gmail.com, pgsql-hackers@lists.postgresql.org, dgrowleyml@gmail.com
Subject: Re: Refactor StringInfo usage in subscriptioncmds.c
In-Reply-To: <0b381b02-cab9-41f9-a900-ad6c8d26c1fc@gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox