Re: [HACKERS] plpgsql - additional extra checks

> + 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