Skip site navigation (1) Skip section navigation (2)

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 (view raw or flat)
Thread:
Lists: pgsql-patchespgsql-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

In response to

Responses

pgsql-www by date

Next:From: Alvaro HerreraDate: 2005-11-21 13:33:08
Subject: Re: DROP OWNED again
Previous:From: Tom LaneDate: 2005-11-19 00:59:04
Subject: Re: DROP OWNED again

pgsql-patches by date

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

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group