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


--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to