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

Re: RangeVarGetRelid()

From: Noah Misch <noah(at)leadboat(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Alvaro Herrera <alvherre(at)commandprompt(dot)com>,Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: RangeVarGetRelid()
Date: 2011-12-05 07:09:20
Message-ID: 20111205070920.GD10035@tornado.leadboat.com (view raw or flat)
Thread:
Lists: pgsql-hackers
On Thu, Nov 24, 2011 at 10:54:50AM -0500, Robert Haas wrote:
> All right, here's an updated patch, and a couple of follow-on patches.

Your committed patch looks great overall.  A few cosmetic points:

> --- a/src/backend/catalog/namespace.c
> +++ b/src/backend/catalog/namespace.c

> @@ -309,6 +313,19 @@ RangeVarGetRelid(const RangeVar *relation, LOCKMODE lockmode, bool missing_ok,
>  		}
>  
>  		/*
> +		 * Invoke caller-supplied callback, if any.
> +		 *
> +		 * This callback is a good place to check permissions: we haven't taken
> +		 * the table lock yet (and it's really best to check permissions before
> +		 * locking anything!), but we've gotten far enough to know what OID we
> +		 * think we should lock.  Of course, concurrent DDL might things while

That last sentence needs a word around "might things".

> --- a/src/backend/commands/tablecmds.c
> +++ b/src/backend/commands/tablecmds.c

> @@ -767,15 +775,13 @@ RemoveRelations(DropStmt *drop)
>  		 */
>  		AcceptInvalidationMessages();

The above call can go away, now.

> @@ -843,6 +804,74 @@ RemoveRelations(DropStmt *drop)
>  }
>  
>  /*
> + * Before acquiring a table lock, check whether we have sufficient rights.
> + * In the case of DROP INDEX, also try to lock the table before the index.
> + */
> +static void
> +RangeVarCallbackForDropRelation(const RangeVar *rel, Oid relOid, Oid oldRelOid,
> +								void *arg)
> +{
> +	HeapTuple	tuple;
> +	struct DropRelationCallbackState *state;
> +	char		relkind;
> +	Form_pg_class classform;
> +
> +	state = (struct DropRelationCallbackState *) arg;
> +	relkind = state->relkind;
> +
> +	/*
> +	 * If we previously locked some other index's heap, and the name we're
> +	 * looking up no longer refers to that relation, release the now-useless
> +	 * lock.
> +	 */
> +	if (relOid != oldRelOid && OidIsValid(state->heapOid))
> +	{
> +		UnlockRelationOid(state->heapOid, AccessExclusiveLock);
> +		state->heapOid = InvalidOid;
> +	}
> +
> +	/* Didn't find a relation, so need for locking or permission checks. */

That sentence needs a word around "so need".

Thanks,
nm

In response to

Responses

pgsql-hackers by date

Next:From: Heikki LinnakangasDate: 2011-12-05 08:53:40
Subject: Detecting loss of client connection (was Re: Bug in walsender when calling out to do_pg_stop_backup (and others?))
Previous:From: Tomas VondraDate: 2011-12-05 00:16:20
Subject: Re: why local_preload_libraries does require a separate directory ?

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