Re: [HACKERS] plpgsql - additional extra checks - Mailing list pgsql-hackers

From Tomas Vondra
Subject Re: [HACKERS] plpgsql - additional extra checks
Date
Msg-id 457e1f54-57be-9140-ea76-0f0e9f12d78e@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  (Pavel Stehule <pavel.stehule@gmail.com>)
List pgsql-hackers
On 03/04/2018 07:07 PM, Pavel Stehule wrote:
> 
> ...
> 
> I am sending updated patch with Tomas changes
> 

Seems 2cf8c7aa48 broke this patch, as it tweaked a number of regression
tests. Other than that, I think the patch is pretty much ready.

One minor detail is that this bit from exec_stmt_execsql doesn't seem
particularly readable:

errlevel = stmt->strict || stmt->mod_stmt ? ERROR : too_many_rows_level;
use_errhint = !(stmt->strict || stmt->mod_stmt);

I had to think for a while if "||" takes precedence over "?", so I'd
suggest adding some () around the first condition. But that makes it
pretty much just (!use_errhint) so perhaps something like

    bool force_error;

    force_error = (stmt->strict || stmt->mod_stmt);
    errlevel = force_error ? ERROR : too_many_rows_level;
    use_errhint = !force_error;

would be better?

Actually, now that I'm looking at this part of the code again, I see the
change from

    errmsg("query returned more than one row"),

to

    errmsg("SELECT INTO query returned more than one row"),

is probably bogus, because this also deals with stmt->mod_stmt, not just
strict SELECT INTO queries. So let's just revert to the old wording.

regards

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


pgsql-hackers by date:

Previous
From: Michael Paquier
Date:
Subject: Re: pg_get_functiondef forgets about most GUC_LIST_INPUT GUCs
Next
From: leap
Date:
Subject: Re:Re: Re: [submit code] I develop a tool for pgsql, how can Isubmit it