On 2018/07/18 18:30, Peter Eisentraut wrote: > On 06.07.18 04:00, Amit Langote wrote: >> On 2018/07/05 23:02, Robert Haas wrote: >>> On Wed, Jul 4, 2018 at 3:09 AM, Amit Langote >>> <langote_amit...@lab.ntt.co.jp> wrote: >>>> I wonder why we mention on the following page that CREATE COLLATION >>>> requires SHARE ROW EXCLUSIVE lock >>>> >>>> https://www.postgresql.org/docs/devel/static/explicit-locking.html >>>> >>>> I know that's the lock taken on the pg_collation catalog, but do we need >>>> to mention locks taken by a DDL command on the catalogs it affects? All >>>> other commands mentioned on the page require to specify the table name >>>> that the lock will be taken on. >>> >>> Yes, that looks odd. >> >> OK, here is a patch. >> >> I see that it was one of Peter E's commits that added that, so cc'd him. > > The reason this is mentioned is that CREATE COLLATION takes a SHARE ROW > EXCLUSIVE lock on pg_collation whereas similar CREATE commands only take > a ROW EXCLUSIVE lock on their catalogs. (So you can only have one > CREATE COLLATION running at a time. The reasons for this are explained > in pg_collation.c.) I think mentioning this was requested during patch > review.
I see. Although, which lock we take on the system catalog for implementing a particular command seems to be an internal detail. What's clearly user-visible in this case is that CREATE COLLATION command cannot be used simultaneously by concurrent sessions, so it should be pointed out in the CREATE COLLATION command's documentation. On a quick check, it doesn't seem to be. So, I have updated my patch to also add a line about that on CREATE COLLATION page. What do you think? When playing with this, I observed that a less user-friendly error message is emitted if multiple sessions race to create the same collation. Session 1: begin; create collation collname (...); Session 2: create collation collname (...); <blocks for lock on pg_collation> Session 1: commit; Session 2: ERROR: duplicate key value violates unique constraint "pg_collation_name_enc_nsp_index" DETAIL: Key (collname, collencoding, collnamespace)=(collname, 6, 2200) already exists. I figured that's because the order in CollationCreate of locking the catalog and checking in syscache whether a duplicate exists. I think we should check the syscache for duplicate *after* we have locked the catalog, as done in the other patch that's attached. Thanks, Amit
diff --git a/doc/src/sgml/mvcc.sgml b/doc/src/sgml/mvcc.sgml index 24613e3c75..4ec853b77a 100644 --- a/doc/src/sgml/mvcc.sgml +++ b/doc/src/sgml/mvcc.sgml @@ -970,8 +970,7 @@ ERROR: could not serialize access due to read/write dependencies among transact </para> <para> - Acquired by <command>CREATE COLLATION</command>, - <command>CREATE TRIGGER</command>, and many forms of + Acquired by <command>CREATE TRIGGER</command>, and many forms of <command>ALTER TABLE</command> (see <xref linkend="sql-altertable"/>). </para> </listitem> diff --git a/doc/src/sgml/ref/create_collation.sgml b/doc/src/sgml/ref/create_collation.sgml index 5bc9af5499..84ff418e6d 100644 --- a/doc/src/sgml/ref/create_collation.sgml +++ b/doc/src/sgml/ref/create_collation.sgml @@ -163,6 +163,11 @@ CREATE COLLATION [ IF NOT EXISTS ] <replaceable>name</replaceable> FROM <replace <title>Notes</title> <para> + Note that only one of the concurrent sessions can run + <command>CREATE COLLATION</command> at a time. + </para> + + <para> Use <command>DROP COLLATION</command> to remove user-defined collations. </para>
diff --git a/src/backend/catalog/pg_collation.c b/src/backend/catalog/pg_collation.c index ce7e5fb5cc..33a5510bf9 100644 --- a/src/backend/catalog/pg_collation.c +++ b/src/backend/catalog/pg_collation.c @@ -70,6 +70,9 @@ CollationCreate(const char *collname, Oid collnamespace, AssertArg(collcollate); AssertArg(collctype); + /* open pg_collation; see below about the lock level */ + rel = heap_open(CollationRelationId, ShareRowExclusiveLock); + /* * Make sure there is no existing collation of same name & encoding. * @@ -83,9 +86,13 @@ CollationCreate(const char *collname, Oid collnamespace, ObjectIdGetDatum(collnamespace))) { if (quiet) + { + heap_close(rel, NoLock); return InvalidOid; + } else if (if_not_exists) { + heap_close(rel, NoLock); ereport(NOTICE, (errcode(ERRCODE_DUPLICATE_OBJECT), collencoding == -1 @@ -105,9 +112,6 @@ CollationCreate(const char *collname, Oid collnamespace, collname, pg_encoding_to_char(collencoding)))); } - /* open pg_collation; see below about the lock level */ - rel = heap_open(CollationRelationId, ShareRowExclusiveLock); - /* * Also forbid a specific-encoding collation shadowing an any-encoding * collation, or an any-encoding collation being shadowed (see