Re: [HACKERS] plpgsql - additional extra checks

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>
Cc: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, 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-19 12:50:35
Message-ID: CAFj8pRAsxHBteQRmJ8U8Mn_P66ozhQ8hJkavuEu-MG9UT0B8Yw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

2018-07-15 1:38 GMT+02:00 Tomas Vondra <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

Regards

Pavel

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

Attachment Content-Type Size
plpgsql-extra-check-180719-1.patch text/x-patch 18.7 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Haribabu Kommi 2018-07-19 12:55:44 Re: [bug fix] Produce a crash dump before main() on Windows
Previous Message Alexander Korotkov 2018-07-19 12:50:23 Re: Bug in gin insert redo code path during re-compression of empty gin data leaf pages