From: | Marko Tiikkaja <marko(at)joh(dot)to> |
---|---|
To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
Cc: | PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: PL/PgSQL: RAISE and the number of parameters |
Date: | 2014-08-12 11:23:26 |
Message-ID: | 53E9F92E.3020704@joh.to |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi Fabien,
On 8/12/14 1:09 PM, Fabien COELHO wrote:
>> Here's a patch for making PL/PgSQL throw an error during compilation (instead
>> of runtime) if the number of parameters passed to RAISE don't match the
>> number of placeholders in error message. I'm sure people can see the pros of
>> doing it this way.
>
> Patch scanned, applied & tested without problem on head.
Thanks!
> The compile-time raise parameter checking is a good move.
>
> 3 minor points:
>
> - I would suggest to avoid "continue" within a loop so that the code is
> simpler to understand, at least for me.
I personally find the code easier to read with the continue.
> - I would suggest to update the documentation accordingly.
I scanned the docs trying to find any mentions of the run-time error but
couldn't find any. I don't object to adding this, though.
> - The execution code now contains error detections which should never be
> raised, but I suggest to keep it in place anyway. However I would suggest
> to add comments about the fact that it should not be triggered.
Good point. I actually realized I hadn't sent the latest version of the
patch, sorry about that. I did this:
https://github.com/johto/postgres/commit/1acab6fc5387c893ca29fed9284e09258e0c5c56
to also turn the ereport()s into elog()s since the user should never see
them.
> See the attached patch which implements these suggestions on top of your
> patch.
Thanks for reviewing! I'll incorporate the doc changes, but I'm going
to let others decide on the logic in the check_raise_parameters() loop
before doing any changes.
Will send an updated patch shortly.
.marko
From | Date | Subject | |
---|---|---|---|
Next Message | Andres Freund | 2014-08-12 11:28:55 | Re: SSL regression test suite |
Previous Message | Fabien COELHO | 2014-08-12 11:09:44 | Re: PL/PgSQL: RAISE and the number of parameters |