Re: ALTER TABLE lock strength reduction patch is unsafe

Hi,

I took a quick peek at this, and noticed the following things:
* I am pretty sure this patch doesn't compile anymore after the latest
set of releases.
* This definitely should include isolationtester tests actually
performing concurrent ALTER TABLEs. All that's currently there is
tests that the locklevel isn't too high, but not that it actually works.
* So far no DDL uses ShareRowExclusiveLocks, why do so now? Not sure if
there aren't relevant cases for with foreign key checks (which afair
*do* acquire SRE locks).
* Why is AddConstraint "so complicated" when it's all used SRE locks?
* Are you sure AlterConstraint is generally safe without an AEL? It
should be safe to change whether something is deferred, but not
necessarily whether it's deferrable?
* Why does ChangeOwner need AEL?
* You changed several places to take out lower locks, which in itself is
fine, but doesn't that lead to lock upgrade hazard if a later step
acquires a stronger lock? Or do we take out a strong enough lock from
the beginning.
* There's no explanation why the EOXact TupleDesc stuff is needed?

That's it for now,

Andres

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services