From 4240e429d0c2d889d0cda23c618f94e12c13ade7 Mon Sep 17 00:00:00 2001 From: Robert Haas Date: Fri, 8 Jul 2011 22:19:30 -0400 Subject: Try to acquire relation locks in RangeVarGetRelid. In the previous coding, we would look up a relation in RangeVarGetRelid, lock the resulting OID, and then AcceptInvalidationMessages(). While this was sufficient to ensure that we noticed any changes to the relation definition before building the relcache entry, it didn't handle the possibility that the name we looked up no longer referenced the same OID. This was particularly problematic in the case where a table had been dropped and recreated: we'd latch on to the entry for the old relation and fail later on. Now, we acquire the relation lock inside RangeVarGetRelid, and retry the name lookup if we notice that invalidation messages have been processed meanwhile. Many operations that would previously have failed with an error in the presence of concurrent DDL will now succeed. There is a good deal of work remaining to be done here: many callers of RangeVarGetRelid still pass NoLock for one reason or another. In addition, nothing in this patch guards against the possibility that the meaning of an unqualified name might change due to the creation of a relation in a schema earlier in the user's search path than the one where it was previously found. Furthermore, there's nothing at all here to guard against similar race conditions for non-relations. For all that, it's a start. Noah Misch and Robert Haas --- src/backend/commands/sequence.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'src/backend/commands/sequence.c') diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c index be04177a2ee..de6e2a3e33f 100644 --- a/src/backend/commands/sequence.c +++ b/src/backend/commands/sequence.c @@ -427,8 +427,8 @@ AlterSequence(AlterSeqStmt *stmt) FormData_pg_sequence new; List *owned_by; - /* open and AccessShareLock sequence */ - relid = RangeVarGetRelid(stmt->sequence, false); + /* Open and lock sequence. */ + relid = RangeVarGetRelid(stmt->sequence, AccessShareLock, false, false); init_sequence(relid, &elm, &seqrel); /* allow ALTER to sequence owner only */ @@ -507,7 +507,16 @@ nextval(PG_FUNCTION_ARGS) Oid relid; sequence = makeRangeVarFromNameList(textToQualifiedNameList(seqin)); - relid = RangeVarGetRelid(sequence, false); + + /* + * XXX: This is not safe in the presence of concurrent DDL, but + * acquiring a lock here is more expensive than letting nextval_internal + * do it, since the latter maintains a cache that keeps us from hitting + * the lock manager more than once per transaction. It's not clear + * whether the performance penalty is material in practice, but for now, + * we do it this way. + */ + relid = RangeVarGetRelid(sequence, NoLock, false, false); PG_RETURN_INT64(nextval_internal(relid)); } -- cgit v1.2.3