Re: Improve errhint for ALTER SUBSCRIPTION ADD/DROP PUBLICATION

Hello

On 2022-Oct-17, houzj.fnst@fujitsu.com wrote:

> alter subscription sub add publication pub2;

> Because I was executing the ADD PUBLICATION command, I feel the hint should
> also mention it instead of SET PUBLICATION.

Hmm, ok.  But:


> @@ -1236,8 +1237,9 @@ AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
>                          ereport(ERROR,
>                                  (errcode(ERRCODE_SYNTAX_ERROR),
>                                   errmsg("ALTER SUBSCRIPTION with refresh and copy_data is not allowed when two_phase
isenabled"),
 
> -                                 errhint("Use ALTER SUBSCRIPTION ... SET PUBLICATION with refresh = false, or with
copy_data= false"
 
> -                                         ", or use DROP/CREATE SUBSCRIPTION.")));
> +                                 errhint("Use ALTER SUBSCRIPTION ... %s PUBLICATION with refresh = false, or with
copy_data= false"
 
> +                                         ", or use DROP/CREATE SUBSCRIPTION.",
> +                                         isadd ? "ADD" : "DROP")));

This looks confusing for translators.  I propose to move the whole
command out of the message, not just one piece of it:

+    /*- translator: %s is an ALTER DDL 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")

I'm not sure that ERRCODE_SYNTAX_ERROR is the right thing here; sounds
like ERRCODE_FEATURE_NOT_SUPPORTED might be more appropriate.

-- 
Álvaro Herrera               48°01'N 7°57'E  —  https://www.EnterpriseDB.com/
"You don't solve a bad join with SELECT DISTINCT" #CupsOfFail
https://twitter.com/connor_mc_d/status/1431240081726115845