Re: [HACKERS] plpgsql - additional extra checks

On 07/19/2018 02:50 PM, Pavel Stehule wrote:
>
>
> 2018-07-15 1:38 GMT+02:00 Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com
> <mailto:tomas(dot)vondra(at)2ndquadrant(dot)com>>:
>
> Hi,
>
> I've been looking at the version submitted on Thursday, planning to
> commit it, but I think it needs a wee bit more work on the error
> messages.
>
> 1) The example in sgml docs does not work, because it's missing a
> semicolon after the CREATE FUNCTION. And the output was not updated
> after tweaking the messages, so it still has uppercase+comma.
>
>
> fixed
>  
>
> 2) The
>
>   /* translator: %s represent a name of extra check */
>
> comment should be
>
>   /* translator: %s represents a name of an extra check */
>
> 3) Both Andres and Alvaro suggested to pass extra_errors/extra_warnings
> as a variable too, turning the errhint into
>
>   errhint("%s check of %s is active.",
>           "too_many_rows",
>           (too_many_rows_level == ERROR) ? "extra_errors" :
>                                            "extra_checks");
>
> or something like that. Any particular reason not to do that?
>
>
> errdetail was used on first place already.
>
> Now, I skip detail in this first case, and elsewhere I move this info to
> detail, and hint has text that you proposed.
>
>
> Sorry for the bikeshedding, but I'd like my first commit not to be
> immediately torn apart by wolves ;-)
>
>
>  no problem
>
>
> 4) I think the errhint() is used incorrectly. The message should be
> about how to fix the issue, but messages like
>
>   HINT:  strict_multi_assignement check of extra_warnings is active.
>
> clearly does not help in this regard. This information should be either
> in errdetail() is deemed useful, or left out entirely. And the errhint()
> should say something like:
>
>   errdetail("Make sure the query returns a single row, or use LIMIT 1.")
>
> and
>
>   errdetail("Make sure the query returns the exact list of columns.")
>
>
> should be fixed too
>

Seems OK to me. I'll go over the patch once more tomorrow and I plan to
get it committed.

regards

--
Tomas Vondra http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services