Re: Tentative patch for making DROP put dependency info in DETAIL

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

In response to

Responses

Browse pgsql-patches by date

  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