Re: Prevent concurrent DROP SCHEMA when certain objects are being initially created in the namespace

On Thu, Sep 6, 2018 at 4:22 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Wed, Sep 05, 2018 at 09:23:37AM -0700, Andres Freund wrote:
> > Well, we kinda do, during some of their own DDL. CF
> > AcquireDeletionLock(), RangeVarGetAndCheckCreationNamespace(), and other
> > LockDatabaseObject() callers. The
> > RangeVarGetAndCheckCreationNamespace() even locks the schema an object
> > is created in , which is pretty much what we're discussing here.
> >
> > I think the problem with the current logic is more that the
> > findDependentObjects() scan doesn't use a "dirty" scan, so it doesn't
> > ever get to seeing conflicting operations.
>
> Well, it seems to me that we don't necessarily want to go down to that
> for sure on back-branches. What's actually striking me as a very bad
> thing is the inconsistent behavior when we need to get a namespace OID
> after calling QualifiedNameGetCreationNamespace using a list of defnames
> which does not lock the schema the DDL is working on. And this happens
> for basically all the object types Jimmy has mentioned.
>
> For example, when creating a composite type, the namespace lock is taken
> through RangeVarGetAndCheckCreationNamespace. If a user tries to create
> a root type, then we simply don't lock it, which leads to an
> inconsistency of behavior between composite types and root types. In my
> opinion the inconsistency of behavior for the same DDL is not logic.
>
> So I have been looking at that, and finished with the attached, which
> actually fixes the set of problems reported. Thoughts?

Hi,

I applied to current master and run "make check-world" and everything is
ok...

I also run some similar tests as Jimmy pointed and using PLpgSQL to execute
DDLs and the new consistent behavior is ok. Also I run one session using
DROP SCHEMA at end and after COMMIT the session 2 report 'ERROR: schema
"testschema" does not exist', so avoiding concerns about lock overhead
seems the proposed patch is ok.

Regards,

--
Fabrízio de Royes Mello Timbira - http://www.timbira.com.br/
PostgreSQL: Consultoria, Desenvolvimento, Suporte 24x7 e Treinamento