Re: [HACKERS] plpgsql - additional extra checks - Mailing list pgsql-hackers
From | Tomas Vondra |
---|---|
Subject | Re: [HACKERS] plpgsql - additional extra checks |
Date | |
Msg-id | b66f5e6a-ed23-4fad-8c2b-a8a3ca172087@2ndquadrant.com 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
(Tomas Vondra <tomas.vondra@2ndquadrant.com>)
|
List | pgsql-hackers |
On 07/19/2018 02:50 PM, Pavel Stehule wrote: > > > 2018-07-15 1:38 GMT+02:00 Tomas Vondra <tomas.vondra@2ndquadrant.com > <mailto:tomas.vondra@2ndquadrant.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
pgsql-hackers by date: