Re: DROP OWNED again

From: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Patches <pgsql-patches(at)postgresql(dot)org>
Subject: Re: DROP OWNED again
Date: 2005-11-19 22:55:15
Message-ID: 20051119225514.GA16420@surnet.cl
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-patches pgsql-www

Tom Lane wrote:
> Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org> writes:
> > /*
> > ! * Called to execute the utility commands GRANT and REVOKE.
> > ! *
> > ! * stmt may be a complete GrantStmt created by the parser, or it may be
> > ! * missing the "objects" list and the "grantees" list. In this case,
> > ! * they are taken from the second and third parameters, respectively.
> > */
> > void
> > ! ExecuteGrantStmt(GrantStmt *stmt, Oid object, Oid grantee)
>
> This seems like a really ugly API. What's so hard about expecting the
> caller to construct a valid GrantStmt?

Well, a valid GrantStmt wants to have textual names of objects, so I
figured it would be best to offer both alternatives to the caller, while
at the same time not having to look up some things twice: there are
several syscaches for both names and Oids, so if you can get the
object's tuple using the name syscache, why not use it directly?

The attached does not attempt to save the second lookup, but it seems
better code -- it offers two entry points, one with GrantStmt just as
it's not, and another one with only Oids. The former calls the latter
upon looking up the object's Oids. So it's somewhat more inefficient,
though probably negligibly so.

> (I get the impression from a quick scan of the code that the comment
> is a long way from telling the truth about what's really happening,
> either.)

Hmm, not sure why you would think so, but it doesn't matter now.

> > + static void AlterConversionOwner_int(Relation rel, Oid conversionOid,
> > + Oid newOwnerId);
>
> If these are supposed to mean "AlterConversionOwner_internal", please
> spell them that way. Sitting beside "AlterConversionOwner_oid", it
> sure looks like the "int" is meant to be read as "integer".

Sure -- done.

I have added the regression tests in this new version, which I'll apply
on monday barring any objections. (I do not attach the SGML files
because these didn't change since the last post.)

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

Attachment Content-Type Size
drop-owned-4.patch text/plain 107.0 KB

In response to

Responses

Browse pgsql-patches by date

  From Date Subject
Next Message Nick Addington 2005-11-20 07:53:50 BUG #2056: to_char no long takes time as input?
Previous Message Andrew Dunstan 2005-11-19 17:48:09 Re: drop if exists - first piece

Browse pgsql-www by date

  From Date Subject
Next Message Alvaro Herrera 2005-11-21 13:33:08 Re: DROP OWNED again
Previous Message Tom Lane 2005-11-19 00:59:04 Re: DROP OWNED again