Re: Re: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist

From: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
To: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andres Freund <andres(at)2ndquadrant(dot)com>, Tomas Vondra <tv(at)fuzzy(dot)cz>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Re: [BUGS] BUG #7873: pg_restore --clean tries to drop tables that don't exist
Date: 2013-11-19 12:08:39
Message-ID: CAFj8pRAXwmOp4Bx=U+W3Hahq1mu3ASYE3wDVptDp=9BtCGpCFg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

Hello

I am thinking so @2 is not good idea. Using well known idiom "IF EXISTS"
once before table name and second after table name can be difficult and
messy for users. If you like it, use different idiom or different keyword,
please.

My person favourite is @1 - fault tolerant version - but I understand to
objection (what was reason, why I wrote a last version @3) - @1 and @3 are
good decision.

Regards

Pavel

2013/11/19 Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>

> On 12 November 2013 16:00, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> wrote:
> > Hello
> >
> > here is patch with fault tolerant drop trigger and drop rule support
> >
> > drop trigger [if exists] trgname on [if exists] tablename;
> > drop rule [if exists] trgname on [if exists] tablename;
> >
> > Regards
> >
> > Pavel
> >
>
> Hi,
>
> I have just started looking at this patch.
>
> It applies cleanly to head, and appears to work as intended. I have a
> question though about the syntax. Looking back over this thread, there
> seem to have been 3 different possibilities discussed:
>
>
> 1). Keep the existing syntax:
>
> DROP TRIGGER [ IF EXISTS ] name ON table_name [ CASCADE | RESTRICT ];
>
> but make it tolerate a non-existent table when "IF EXISTS" is specified.
>
>
> 2). Support 2 independent levels of "IF EXISTS" using the syntax:
>
> DROP TRIGGER [ IF EXISTS ] name ON table_name [ IF EXISTS ] [ CASCADE
> | RESTRICT ]
>
> There was some consensus for this, but then Pavel pointed out that it
> is inconsistent with other DROP commands, which all have the "IF
> EXISTS" before the object to which it refers.
>
>
> 3). Support 2 independent levels of "IF EXISTS" using the syntax:
>
> DROP TRIGGER [ IF EXISTS ] name ON [ IF EXISTS ] table_name [ CASCADE
> | RESTRICT ]
>
> which is what the latest patch does.
>
>
> The syntax in option (3) is certainly more consistent with other DROP
> commands, but it feels pretty clunky from a grammar point-of-view. It
> also feels overly complex for the use cases discussed.
>
> Personally I would prefer option (1). The SQL standard syntax is
> simply "DROP TRIGGER name". The only reason we have the "ON
> table_name" part is that our trigger names aren't globally unique, so
> "trigger_name ON table_name" is required to uniquely identify the
> trigger to drop, which would seem to be directly analogous to
> specifying a schema in DROP TABLE, and we've already made that
> tolerate a non-existent schema if "IF EXISTS" is used.
>
> This seems rather different from ALTER TABLE, which allows multiple
> sub-commands on the same table, so naturally lends itself to multiple
> independent DROP <objtype> [IF EXISTS] sub-commands underneath the
> top-level ALTER TABLE [IF EXISTS], for example:
>
> ALTER TABLE IF EXISTS table_name
> DROP COLUMN IF EXISTS col_name,
> DROP CONSTRAINT IF EXISTS constr_name;
>
> So what we currently have can be summarised as 2 classes of
> commands/sub-commands to which "IF EXISTS" applies:
>
> ALTER <objtype> [IF EXISTS] ...
> DROP <objtype> [IF EXISTS] ...
>
> We don't yet have multiple levels of "IF EXISTS" within the same DROP,
> and I don't think it is necessary. For example, no one seems to be
> asking for
>
> DROP TABLE [IF EXISTS] table_name IN [IF EXISTS] schema_name
>
> Anyway, that's just my opinion. Clearly there is at least one person
> with a different opinion. What do other people think?
>
> Regards,
> Dean
>

In response to

Browse pgsql-bugs by date

  From Date Subject
Next Message Ronan Dunklau 2013-11-19 14:24:44 Server may segfault when using slices on int2vector
Previous Message atoriwork 2013-11-19 10:56:53 BUG #8605: Regular expression lazy quantification issue

Browse pgsql-hackers by date

  From Date Subject
Next Message Rohit Goyal 2013-11-19 12:11:12 Re: Call flow of btinsert(PG_FUNCTION_ARGS)
Previous Message Craig Ringer 2013-11-19 12:03:11 Re: Windows build patch