Thanks for your reviewing. 2011/10/4 Robert Haas <robertmh...@gmail.com>: > On Wed, Sep 28, 2011 at 11:51 AM, Kohei KaiGai <kai...@kaigai.gr.jp> wrote: >> I rebased the patches towards the latest git master, so I believe these >> are available to apply. > > Reviewing away... > > I don't see why we need one struct called ObjectProperty and another > called CatalogProperty. Just fold CatalogProperty into ObjectProperty > and make it one big flat structure. > The reason of this separation is some of ObjectType shares same catalogs; such as pg_class for OBJECT_(TABLE|VIEW|INDEX|SEQUENCE), so I thought it is better to reference CatalogProperty by several ObjectProperty entries to avoid accidental inconsistence. However, it is just a matter of my preference. I'll modify it. The big flat structure provides a simple structure on the other hand.
> The get_object_property_waddawadda functions appear to be designed > under the assumption that each object type will be in the > ObjectProperty structure at the offset corresponding to (int) objtype. > Are these functions going to get called from any place that's > performance-critical enough to justify that optimization? I'd rather > just have these functions use linear search, so that if someone adds a > new OBJECT_* constant and doesn't add it to the array it doesn't > immediately break everything. > OK, I'll fix it. I assume these functions are used in DDL operations; mostly not a performance critical path. > get_object_namespace() looks like it ought to live in objectaddress.c, > not dropcmds.c. OK, I'll move it. > check_object_validation() doesn't seem like a very > good description of what that code actually does -- and that code > looks like it's copy-and-pasted from elsewhere. Can we avoid that? > I noticed that get_object_address() already applied same checks on pg_type object, and pg_class objects also. Due to the below reason, it does not invoke get_object_address() for the pg_class objects, so it seems to me reasonable to move whole of the logic in check_object_validation() to get_relation_address(). > get_relation_address() is surely a copy of some other bit of code; how > can we avoid duplication? > The get_relation_address() follows the logic in RemoveRelations() to be eliminated by this patch, so it is not a code duplication. The reason why we didn't consolidate this routine with get_object_address() was that remove-index requires locks on the table which owns the index to be removed, and it was ugly to add an ad-hoc if-block on the routine. I'll try to work on this. Please wait for a while... Thanks, -- KaiGai Kohei <kai...@kaigai.gr.jp> -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
