Re: A recent message added to pg_upgade

On Fri, Oct 27, 2023 at 8:28 AM Kyotaro Horiguchi
<horikyota.ntt@gmail.com> wrote:
>
> Hello.
>
> Some messages recently introduced by commit 29d0a77fa6 seem to deviate
> slightly from our standards.
>
> +               if (*invalidated && SlotIsLogical(s) && IsBinaryUpgrade)
> +               {
> +                       ereport(ERROR,
> +                                       errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> +                                       errmsg("replication slots must not be invalidated during the upgrade"),
> +                                       errhint("\"max_slot_wal_keep_size\" must be set to -1 during the upgrade"));
>
> The message for errhint is not a complete sentence.

Yeah, the hint message should have ended with a period -
https://www.postgresql.org/docs/current/error-style-guide.html#ERROR-STYLE-GUIDE-GRAMMAR-PUNCTUATION.

> The second attached does this.
>
> What do you think about those?

+                    errmsg("replication slot is invalidated during upgrade"),
+                    errhint("Set \"max_slot_wal_keep_size\" to -1 to
avoid invalidation."));
         }

The above errhint LGTM. How about a slightly different errmsg, like
the following?

+                    errmsg("cannot invalidate replication slots when
in binary upgrade mode"),
+                    errhint("Set \"max_slot_wal_keep_size\" to -1 to
avoid invalidation."));

".... when in binary upgrade mode" is being used in many places.

-- 
Bharath Rupireddy
PostgreSQL Contributors Team
RDS Open Source Databases
Amazon Web Services: https://aws.amazon.com