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

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