Re: [HACKERS] plpgsql - additional extra checks

From: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Cc: nospam-abuse(at)bloodgate(dot)com, David Steele <david(at)pgmasters(dot)net>, Stephen Frost <sfrost(at)snowman(dot)net>, Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Daniel Gustafsson <daniel(at)yesql(dot)se>, Marko Tiikkaja <marko(at)joh(dot)to>, Jim Nasby <jim(dot)nasby(at)bluetreble(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] plpgsql - additional extra checks
Date: 2018-07-14 23:38:12
Message-ID: a9b33e4f-ff30-25c5-beb8-73ddf08ce362@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

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?

Sorry for the bikeshedding, but I'd like my first commit not to be
immediately torn apart by wolves ;-)

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.")

regards

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2018-07-15 08:34:32 Re: Speeding up INSERTs and UPDATEs to partitioned tables
Previous Message Tomas Vondra 2018-07-14 21:20:41 Re: GSOC 2018 Project - A New Sorting Routine