Re: Reducing some DDL Locks to ShareLock

On Tue, 2008-10-07 at 11:46 -0400, Tom Lane wrote: 
> Simon Riggs <simon@2ndQuadrant.com> writes:
> > On Tue, 2008-10-07 at 10:35 -0400, Tom Lane wrote:
> >> I wonder whether this could be helped if we refactored pg_constraint.
> 
> > Sounds better. Doesn't make much sense as it is now.
> 
> I looked at the code a bit, and it seems the only place where the
> current design makes any sense is in ChooseConstraintName, which
> explains itself thusly:
> 
>  * Select a nonconflicting name for a new constraint.
>  *
>  * The objective here is to choose a name that is unique within the
>  * specified namespace.  Postgres does not require this, but the SQL
>  * spec does, and some apps depend on it.  Therefore we avoid choosing
>  * default names that so conflict.
>  *
>  * Note: it is theoretically possible to get a collision anyway, if someone
>  * else chooses the same name concurrently.  This is fairly unlikely to be
>  * a problem in practice, especially if one is holding an exclusive lock on
>  * the relation identified by name1.
> 
> (The last bit of the comment falls flat when you consider constraints
> on domains...)
> 
> Note that this policy is used for system-selected constraint names;
> it's not enforced against user-selected names.  We do attempt (in
> ConstraintNameIsUsed) to reject duplicate user-selected constraint names
> *on the same object*, but that test is not bulletproof against
> concurrent additions.  The refactoring I suggested would make for
> bulletproof enforcement via the unique indexes.

Thought about this some more and examined the way CREATE INDEX works.

Current patch allows concurrent duplicate name creation. Putting a
unique index on it makes it wait for the first command to finish, then
causes it to fail. (That in itself is painful, surely DDL should fail if
it sees another DDL statement in progress trying to do same thing).

Simply creating two FKs concurrently doesn't give any kind of a problem.
Only problem occurs if we have duplicate names.

Problems won't go away if we have unique index. We will still fail to
get concurrent ALTER TABLE, since similar named constraints will wait
behind others, then fail. That is actually worse behaviour than now,
because now we wait behind first statement, then select new unique name.
So currently, we wait then succeed. If we have a unique index we will
wait and then fail.

So the problem AFAICS isn't the lack of a unique index, it is the name
selection itself. If we have more unique naming for constraints the
problem goes away.

Right now the name is selected based upon the column in the FK. The
relation referenced doesn't form any part of the name. So if you have a
column that references two separate tables (very weird) you get the
problem because the name doesn't uniquely describe the situation.

Currently we name things according to the pattern:
<tablename>_<columname1>_fkey. If we have multiple columns we still only
use the first column. That makes it much more likely to have duplicates.

An alternative would be
<tablename>_<referencedtablename>_<columname1>_fkey
which is more useful as well as more likely to be unique. This is a much
better situation than just appending a number to the end of the name.
That's always been annoying since if you assume that all constraints end
with "_fkey" you miss out some valid constraints. So it seems better to
have a longer, more unique name and enforce the idea that it always ends
with "_fkey".

If we *don't* do this, there's not much point adding a unique index,
because it will prevent concurrency in some cases and that's the whole
point here.

-- Simon Riggs           www.2ndQuadrant.comPostgreSQL Training, Services and Support