Re: DROP OWNED again - Mailing list pgsql-patches

From Alvaro Herrera
Subject Re: DROP OWNED again
Date
Msg-id 20051119225514.GA16420@surnet.cl
Whole thread Raw
In response to Re: DROP OWNED again  (Tom Lane <tgl@sss.pgh.pa.us>)
Responses Re: DROP OWNED again
List pgsql-patches
Tom Lane wrote:
> Alvaro Herrera <alvherre@alvh.no-ip.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

pgsql-patches by date:

Previous
From: Andrew Dunstan
Date:
Subject: Re: drop if exists - first piece
Next
From: Bruce Momjian
Date:
Subject: Re: Numeric 508 datatype