Re: [PATCHES] drop if exists - first piece
Tom Lane wrote: New test seems reasonable. done. 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. done. * 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". done. 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. cheers andrew ---(end of broadcast)--- TIP 4: Have you searched our list archives? http://archives.postgresql.org
Re: [PATCHES] drop if exists - first piece
Andrew Dunstan <[EMAIL PROTECTED]> writes: > I will apply the attached patch tomorrow, barring objection. This covers > drop if exists for the following objects: > table view index sequence schema type domain conversion > I have a simple test script as show below, but I am not sure where to > put it in the regression tests - add a new one maybe? 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. * 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". regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
