Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address - Mailing list pgsql-hackers
From | Robert Haas |
---|---|
Subject | Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address |
Date | |
Msg-id | BANLkTinOWRAfGuYKgOhOR1w6aUqSM=qvHQ@mail.gmail.com Whole thread Raw |
In response to | Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address (Noah Misch <noah@leadboat.com>) |
Responses |
Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address
Re: [v9.2] DROP Reworks Part.0 - 'missing_ok' support of get_object_address |
List | pgsql-hackers |
On Wed, Jun 22, 2011 at 6:18 AM, Noah Misch <noah@leadboat.com> wrote: > On Tue, Jun 21, 2011 at 11:11:41PM -0400, Robert Haas wrote: >> On Tue, Jun 21, 2011 at 11:04 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> > Robert Haas <robertmhaas@gmail.com> writes: >> >> Some of the refactoring you've done here seems likely to break things, >> >> because you're basically making the relation locking happen later than >> >> it does not, and that's going to cause problems. >> >> get_object_address_relobject() is a particularly egregious >> >> rearrangement. ?It seems to me that the right formula is to call >> >> relation_openrv() if missing_ok is false, and try_relation_openrv() if >> >> missing_ok is true. ?But that's sort of a pain, so I propose to first >> >> apply the attached patch, which gets rid of try_relation_openrv() and >> >> try_heap_openrv() and instead adds a missing_ok argument to >> >> relation_openrv() and heap_openrv(). ?If we do this, then the >> >> missing_ok argument can just be passed through all the way down. >> > >> >> Thoughts? ?Comments? ?Objections? >> > >> > At least the last hunk (in pltcl.c) seems to have the flag backwards. >> >> Ah, nuts. Sorry. I think that and parse_relation.c are the only >> places where the try variants are used; nobody else is willing to >> fail, and everyone else is passing false. >> >> Revised patch attached. > > All existing call sites updated by this patch hardcode the flag, and only 3? > proposed call sites would take advantage of the ability to not do so. The > try_relation_openrv() case is fairly rare and likely to remain that way. Given > that, I mildly prefer the code as it is, even if that means doing "missing_ok ? > try_relation_openrv() : relation_openrv()" in a few places. Could always wrap > that in a static function of objectaddress.c. I don't really like the idea of having a static function in objectaddress.c, because I think there will eventually be more callers who sometimes want to pass missing_ok = true and other times pass missing_ok = false. Plus, it seems a little nutty to have a function that, depending on whether its argument is true or false, calls one of two other functions which are virtually cut-and-paste copies of each other, except that one internally has true and the other false. I just work here, though. Another option might be to leave heap_openrv() and relation_openrv() alone and add a missing_ok argument to try_heap_openrv() and try_relation_openrv(). Passing true would give the same behavior as presently; passing false would make them behave like the non-try version. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
pgsql-hackers by date: