On Wed, Jul 6, 2011 at 10:44 PM, Noah Misch <noah@2ndquadrant.com> wrote:
> Attached. I made the counter 64 bits wide, handled the nothing-found case per
> your idea, and improved a few comments cosmetically. I have not attempted to
> improve the search_path interposition case. We can recommend the workaround
> above, and doing better looks like an excursion much larger than the one
> represented by this patch.
I looked at this some more and started to get uncomfortable with the
whole idea of having RangeVarLockRelid() be a wrapper around
RangeVarGetRelid(). This hazard exists everywhere the latter function
gets called, not just in relation_open(). So it doesn't seem right to
fix the problem only in those places.
So I went through and incorporated the logic proposed for
RangeVarLockRelid() into RangeVarGetRelid() itself, and then went
through and examined all the callers of RangeVarGetRelid(). There are
some, such as has_table_privilege(), where it's really impractical to
take any lock, first because we might have no privileges at all on
that table and second because that could easily lead to a massive
amount of locking for no particular good reason. I believe Tom
suggested that the right fix for these functions is to have them
index-scan the system catalogs using the caller's MVCC snapshot, which
would be right at least for pg_dump. And there are other callers that
cannot acquire the lock as part of RangeVarGetRelid() for a variety of
other reasons. However, having said that, there do appear to be a
number of cases that are can be fixed fairly easily.
So here's a (heavily) updated patch that tries to do that, along with
adding comments to the places where things still need more fixing. In
addition to the problems corrected by your last version, this fixes
LOCK TABLE, ALTER SEQUENCE, ALTER TABLE .. RENAME, the whole-table
variant of REINDEX, CREATE CONSTRAINT TRIGGER (which is flat-out wrong
as it stands, since it acquires *no lock at all* on the table
specified in the FROM clause, never mind the question of doing so
atomically), CREATE RULE, and (partially) DROP TRIGGER and DROP RULE.
Regardless of exactly how we decide to proceed here, it strikes me
that there is a heck of a lot more work that could stand to be done in
this area... :-(
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company