Re: RangeVarGetRelid()

On Fri, Nov 18, 2011 at 9:12 AM, Noah Misch <noah(at)leadboat(dot)com> wrote:
> Good call.

All right, here's an updated patch, and a couple of follow-on patches.

I updated the main patch (rangevargetrelid-callback-v2.patch) per
previous discussion. I also added a "callback_arg" argument to the
RangeVarGetRelidExtended(), because it's a law of nature that all
callback functions need such a thing, and this case proved to be no
exception: the old version of the REINDEX INDEX callback was flaky,
since it assumed that the index it locked during one iteration would
still exist during the next iteration, which won't be true when the
retry is caused by the index having been concurrently dropped. There
were some other bugs as well, which I believe I've now fixed.

fix-lock-table.patch applies over rangevargetrelid-callback-v2.patch
and adjusts LOCK TABLE so that we never obtain a relation lock before
validating that the object is a table and we have permission to lock
it. fix-drop-relation.patch applies over that, and makes similar
corrections to the logic for DROP TABLE/INDEX/SEQUENCE/VIEW/FOREIGN
TABLE. This means that it's no longer possible to use these commands
to launch a denial-of-service attack against a table you have no
rights to. Sadly, there are a bunch of other commands that can be
used the same way. I'd like to fix them all, but it's a decent amount
of work, so I'm working through them one at a time.

In the case of DROP, this also improves handling of concurrent DROP,
DROP-and-CREATE, RENAME-and-CREATE, and similar situations. For
example, in unpatched master:

rhaas=# create table t (a int);
CREATE TABLE
rhaas=# begin;
BEGIN
rhaas=# drop table t;
DROP TABLE
rhaas=# create table t (b int);
CREATE TABLE

And then, from another session:

rhaas=# drop table t;

When the first session commits, you get something like this:

ERROR: cache lookup failed for relation 16401

With these patches, that goes away: the concurrent DROP correctly sees
the new relation and drops that one. Or, if the concurrent
transaction just does a DROP rather than a DROP-and-CREATE, then you
get an error - but instead of a somewhat incomprehensible complaint
about a cache lookup having failed, you get the same error you would
have gotten had the relation not existed in the first place:

ERROR: table "t" does not exist

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company