From 915a6c4e22ecc4775b9ed18312bf12d896cd2b11 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Tue, 18 Oct 2022 11:46:58 +0200 Subject: [PATCH] Improve errhint for ALTER SUBSCRIPTION ADD/DROP PUBLICATION The original hint says to use SET PUBLICATION when really ADD/DROP PUBLICATION is called for, so this is arguably a bug fix. Also, a very similar message elsewhere was using an inconsistent SQLSTATE. While at it, unwrap some strings. Backpatch to 15. Author: Hou zj Discussion: https://postgr.es/m/OS0PR01MB57160AD0E7386547BA978EB394299@OS0PR01MB5716.jpnprd01.prod.outlook.com --- src/backend/commands/subscriptioncmds.c | 27 +++++++++++++++---------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 8fb89a9392c..f0cec2ad5e7 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -1182,10 +1182,9 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, */ if (sub->twophasestate == LOGICALREP_TWOPHASE_STATE_ENABLED && opts.copy_data) ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("ALTER SUBSCRIPTION with refresh and copy_data is not allowed when two_phase is enabled"), - errhint("Use ALTER SUBSCRIPTION ... SET PUBLICATION with refresh = false, or with copy_data = false" - ", or use DROP/CREATE SUBSCRIPTION."))); + errhint("Use ALTER SUBSCRIPTION ... SET PUBLICATION with refresh = false, or with copy_data = false, or use DROP/CREATE SUBSCRIPTION."))); PreventInTransactionBlock(isTopLevel, "ALTER SUBSCRIPTION with refresh"); @@ -1226,7 +1225,11 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, ereport(ERROR, (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("ALTER SUBSCRIPTION with refresh is not allowed for disabled subscriptions"), - errhint("Use ALTER SUBSCRIPTION ... SET PUBLICATION ... WITH (refresh = false)."))); + /* translator: %s is an SQL ALTER command */ + errhint("Use %s instead.", + isadd ? + "ALTER SUBSCRIPTION ... ADD PUBLICATION ... WITH (refresh = false)" : + "ALTER SUBSCRIPTION ... DROP PUBLICATION ... WITH (refresh = false)"))); /* * See ALTER_SUBSCRIPTION_REFRESH for details why this is @@ -1234,10 +1237,13 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, */ if (sub->twophasestate == LOGICALREP_TWOPHASE_STATE_ENABLED && opts.copy_data) ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), + (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("ALTER SUBSCRIPTION with refresh and copy_data is not allowed when two_phase is enabled"), - errhint("Use ALTER SUBSCRIPTION ... SET PUBLICATION with refresh = false, or with copy_data = false" - ", or use DROP/CREATE SUBSCRIPTION."))); + /* translator: %s is an SQL ALTER command */ + errhint("Use %s with refresh = false, or with copy_data = false, or use DROP/CREATE SUBSCRIPTION.", + isadd ? + "ALTER SUBSCRIPTION ... ADD PUBLICATION" : + "ALTER SUBSCRIPTION ... DROP PUBLICATION"))); PreventInTransactionBlock(isTopLevel, "ALTER SUBSCRIPTION with refresh"); @@ -1282,8 +1288,7 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, ereport(ERROR, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("ALTER SUBSCRIPTION ... REFRESH with copy_data is not allowed when two_phase is enabled"), - errhint("Use ALTER SUBSCRIPTION ... REFRESH with copy_data = false" - ", or use DROP/CREATE SUBSCRIPTION."))); + errhint("Use ALTER SUBSCRIPTION ... REFRESH with copy_data = false, or use DROP/CREATE SUBSCRIPTION."))); PreventInTransactionBlock(isTopLevel, "ALTER SUBSCRIPTION ... REFRESH"); @@ -2011,8 +2016,8 @@ ReportSlotConnectionError(List *rstates, Oid subid, char *slotname, char *err) ereport(ERROR, (errcode(ERRCODE_CONNECTION_FAILURE), - errmsg("could not connect to publisher when attempting to " - "drop replication slot \"%s\": %s", slotname, err), + errmsg("could not connect to publisher when attempting to drop replication slot \"%s\": %s", + slotname, err), /* translator: %s is an SQL ALTER command */ errhint("Use %s to disassociate the subscription from the slot.", "ALTER SUBSCRIPTION ... SET (slot_name = NONE)")));