Re: Improve errhint for ALTER SUBSCRIPTION ADD/DROP PUBLICATION - Mailing list pgsql-hackers

From Peter Smith
Subject Re: Improve errhint for ALTER SUBSCRIPTION ADD/DROP PUBLICATION
Date
Msg-id CAHut+Psxa5CSgzJCvP_oMMtkUMKnH1i+3x3a_7kn+zDTS+XQYw@mail.gmail.com
Whole thread Raw
In response to Re: Improve errhint for ALTER SUBSCRIPTION ADD/DROP PUBLICATION  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
Responses Re: Improve errhint for ALTER SUBSCRIPTION ADD/DROP PUBLICATION  (Alvaro Herrera <alvherre@alvh.no-ip.org>)
List pgsql-hackers
On Mon, Oct 17, 2022 at 6:43 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> 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_datais not allowed when two_phase is enabled"),
 
> > -                                                              errhint("Use ALTER SUBSCRIPTION ... SET PUBLICATION
withrefresh = false, or with copy_data = false"
 
> > -                                                                              ", or use DROP/CREATE
SUBSCRIPTION.")));
> > +                                                              errhint("Use ALTER SUBSCRIPTION ... %s PUBLICATION
withrefresh = 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.
>

I thought maybe ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE, which would
make it the same as similar messages in the same function when
incompatible parameters are specified.

------
Kind Regards,
Peter Smith.
Fujitsu Australia.



pgsql-hackers by date:

Previous
From: Nikita Malakhov
Date:
Subject: RFC: multi TOAST-tables support
Next
From: Alvaro Herrera
Date:
Subject: Re: Improve errhint for ALTER SUBSCRIPTION ADD/DROP PUBLICATION