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.gmail.com
[2]:
https://www.postgresql.org/message-id/flat/CAApHDvrZnM28wa2VY58cvtY0y9XbMhKJH4m%3Dga3c1wfsx%3DMF4Q%40mail.gmail.com#ba34970e59f9fd847f1ab52777d57edc
From 2436de272d39e664fa0c0010cf875cb34fcecd1e Mon Sep 17 00:00:00 2001
From: Mats Kindahl <[email protected]>
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.c
index 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
