Tom Lane wrote:

New test seems reasonable.


A few other minor comments:

* The NOTICEs should probably not carry an ERRCODE; usually you just
want a notice to go out with the default "not an error" SQLSTATE.


* The "Assert(missing_ok)"s are a waste of code space --- if
ereport(ERROR) ever returned, there would be vast swaths of the backend
that fail, so there's no point in asserting it only here.  They also
make the reader stop to wonder why they are there, which is probably a
bigger objection.

done, but I left one with a modified test to reflect what we are really testing, i.e. that we didn't fall of the end of a message table.

* It's probably not a good idea to assume that "IF" is a safe name for
a parser symbol --- too much risk of collision with other macros.
I'd suggest "IF_P".


Thanks for the review.

Committed as above. I will probably do DROP DATABASE IF EXISTS shortly, but anyone who wants to jump in on others is welcome to.



---------------------------(end of broadcast)---------------------------
TIP 4: Have you searched our list archives?


Reply via email to