FirebirdSQL / firebird-documentation

Firebird documentation
https://firebirdsql.org/en/documentation/
35 stars 15 forks source link

[FB4] 5.7.2. ALTER TRIGGER. There can skip declaration of body #190

Closed dmitry-lipetsk closed 1 year ago

dmitry-lipetsk commented 1 year ago

https://firebirdsql.org/file/documentation/html/en/refdocs/fblangref40/firebird-40-language-reference.html#fblangref40-ddl-trgr-alter

ALTER TRIGGER trigname
  [ACTIVE | INACTIVE]
  [{BEFORE | AFTER} <mutation_list>]
  [POSITION number]
  {<psql_trigger> | <external-module-body>}

FB4 can execute ALTER TRIGGER without any options.

image

So - {<psql_trigger> | <external-module-body>} is the optional part of this statement.

mrotteveel commented 1 year ago

This sounds like an implementation bug in Firebird to me, and not something which should be reflected in the syntax documentation.

mrotteveel commented 1 year ago

I have to correct my previous statement. This is already documented correctly (no matter how "odd" this behaviour is), you missed that the rule <psql_trigger> consists only of optional elements.

That is:

ALTER TRIGGER trigname
  [ACTIVE | INACTIVE]
  [{BEFORE | AFTER} <mutation_list>]
  [POSITION number]
  {<psql_trigger> | <external-module-body>}

<psql_trigger> ::=
  [<sql_security>]
  [<psql-module-body>]

<sql_security> ::=
    SQL SECURITY {INVOKER | DEFINER}
  | DROP SQL SECURITY
dmitry-lipetsk commented 1 year ago

Russian documentation defines "ALTER TRIGGER" more clearly:

ALTER TRIGGER trigname
[ACTIVE | INACTIVE]
[{BEFORE | AFTER} <mutation_list>]
[POSITION number]
[SQL SECURITY {DEFINER | INVOKER} | DROP SQL SECURITY]
[<routine-body>]

But it has another problem here :)

If I understand correctly, the declaration {<psql_trigger> | <external-module-body>} means that either you define psql_trigger or you define external-module-body

Of course, I can go to declare empty psql_trigger when I want to skip declaration of external-module-body

I'm not sure that the parser of FB will agree with it, but a human can say that it is a great idea :)

mrotteveel commented 1 year ago

That syntax documentation is incorrect, because the SQL SECURITY clause only applies to PSQL triggers, not for external module triggers

mrotteveel commented 1 year ago

The current syntax is a bit of a tradeoff, but is intended to reflect that you can only use <sql_security> for PSQL triggers.

(removed supposition which does not match historical syntax)

mrotteveel commented 1 year ago

I'll make it [{<psql_trigger> | <external-module-body>}].