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
>>> <[email protected]> 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
