Re: [HACKERS] plpgsql - additional extra checks - Mailing list pgsql-hackers

From Alvaro Herrera
Subject Re: [HACKERS] plpgsql - additional extra checks
Date
Msg-id 20180709194436.cwlddy5iiukeinjb@alvherre.pgsql
Whole thread Raw
In response to Re: [HACKERS] plpgsql - additional extra checks  (Pavel Stehule <pavel.stehule@gmail.com>)
Responses Re: [HACKERS] plpgsql - additional extra checks  (Andres Freund <andres@anarazel.de>)
Re: [HACKERS] plpgsql - additional extra checks  (Pavel Stehule <pavel.stehule@gmail.com>)
Re: [HACKERS] plpgsql - additional extra checks  (Pavel Stehule <pavel.stehule@gmail.com>)
List pgsql-hackers
> +                ereport(errlevel,
>                          (errcode(ERRCODE_TOO_MANY_ROWS),
>                           errmsg("query returned more than one row"),
> -                         errdetail ? errdetail_internal("parameters: %s", errdetail) : 0));
> +                         errdetail ? errdetail_internal("parameters: %s", errdetail) : 0,
> +                         use_errhint ? errhint("too_many_rows check of extra_%s is active.",
> +                                      too_many_rows_level == ERROR ? "errors" : "warnings") : 0));

Please write this in a way that results in less translatable messages,
and don't build setting names at runtime.  Concretely I suggest this:

errhint(too_many_rows_level == ERROR ? 
    gettext_noop("%s check of extra_errors is active.") : 
    gettext_noop("%s check of extra_warnings is active."),
    "too_many_rows");
    
This way,
1. only two messages need translation, not one per type of warning/error
2. the translator doesn't need to scratch their head to figure out what
   the second %s is
3. they don't have to worry about introducing a typo in the string
   "too_many_rows" or the strings "extra_errors", "extra_warnings".
   (Laugh all you want. It's a real problem).

If you can add a /* translator: */ comment to indicate what the first %s
is, all the better.  I'm just not sure *where* it needs to go.  I'm not
100% sure the gettext_noop() is really needed either.

> +                            ereport(strict_multiassignment_level,
> +                                    (errcode(ERRCODE_DATATYPE_MISMATCH),
> +                                     errmsg("Number of source and target fields in assignment do not match."),

Please, no uppercase in errmsg(), and no ending period.

-- 
Álvaro Herrera                https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services


pgsql-hackers by date:

Previous
From: Tomas Vondra
Date:
Subject: Re: [HACKERS] plpgsql - additional extra checks
Next
From: Andres Freund
Date:
Subject: Re: [HACKERS] plpgsql - additional extra checks