From: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
---|---|
To: | "Alex Hunsaker" <badalex(at)gmail(dot)com> |
Cc: | "Alvaro Herrera" <alvherre(at)commandprompt(dot)com>, pgsql-patches(at)postgresql(dot)org |
Subject: | Re: Tentative patch for making DROP put dependency info in DETAIL |
Date: | 2008-06-12 23:35:12 |
Message-ID: | 15930.1213313712@sss.pgh.pa.us |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-patches |
"Alex Hunsaker" <badalex(at)gmail(dot)com> writes:
> Yep, I thought about doing the reverse. Namely Just passing the
> DropStmt to RemoveRelation(s). But then all the permission check
> functions are in utility.c. Splitting those out seemed to be about
> the same as splitting out all the ObjectAddress stuff...
Well, that might actually be a good approach: try to get ProcessUtility
back down to being just a dispatch switch. It's pretty much of a wart
that we're doing any permissions checking in utility.c at all. Possibly
those functions should be moved to aclchk.c and then used from
RemoveRelation(s) and friends, which would stay where they are but
change API.
I think the fundamental tension here is between two theories of
organizing the code: we've got the notion of "collect operations
on an object type together", which leads to eg putting
RemoveTSConfiguration in tsearchcmds.c, as against "collect similar
operations together", which leads to wanting all the DROP operations
in the same module.
In the abstract it's not too easy to choose between these, but I think
you'll probably find that the first one works better here, mainly
because each of those object-type modules knows how to work with a
particular system catalog. A DROP module is going to find itself
importing all the catalog headers plus probably utility routines from
all over. So that's leading me to lean towards keeping RemoveRelation
et al where they are and distributing the work currently done in
ProcessUtility out to them. This sounds duplicative, but about all that
really is there to duplicate is a foreach loop, which you're going to
need anyway if the routines are to handle multiple objects.
regards, tom lane
From | Date | Subject | |
---|---|---|---|
Next Message | Alex Hunsaker | 2008-06-12 23:44:50 | Re: Tentative patch for making DROP put dependency info in DETAIL |
Previous Message | Alex Hunsaker | 2008-06-12 22:27:02 | Re: Tentative patch for making DROP put dependency info in DETAIL |