Re: Command Triggers, patch v11

Hi,

Please find attached v15 of the patch, addressing all known issues apart
from the trigger function argument passing style. Expect a new patch
with that taken care of early next week.

(The github branch too, should you be using that)

Thom Brown <thom(at)linux(dot)com> writes:
> CREATE OPERATOR CLASS now shows objectname as 'hash' instead of its
> name where it's declared as "USING hash". This isn't a problem with
> ALTER/DROP OPERATOR CLASS. Everything else above works as expected
> now.

Ah yes that needed a special case, it's properly handled now, and
tested.

>>> When dropping domains, the name of the domain includes the schema name:

Fixed.

> Could we change this to "REINDEX DATABASE triggers are not supported"?
> This way it would be consistent with the "AFTER CREATE INDEX
> CONCURRENTLY" warning.

Sure, done.

>>> REINDEX on a table seems to show no schema name but an object name for
>>> specific triggers:

Was a typo, fixed.

>>> When REINDEXing an index rather than a table, the table's details are
>>> shown in the trigger.  Is this expected?:

Fixed.

> ALTER CAST is still listed and needs removing, not just from the
> documentation but every place it's used your code too. I can
> currently create a trigger for it, but it's impossible for it to fire
> since there's no such command.

Removed.

> All these corrections I mentioned previously still needs to be made:

That's about the docs, I edited them accordingly to your comments.

>> Thom Brown <thom(at)linux(dot)com> writes:
> All other command triggers don't fire on read-only standbys, and the
> inconsistency doesn't seem right. On the one hand all
> CREATE/DROP/ALTER triggers aren't fired because of the "cannot execute
> <command> in a read-only transaction" error message, but triggers do
> occur before utility commands, which would otherwise display the same
> message, and might not display it at all if the trigger causes an
> error in its function call. So it seems like they should either all
> fire, or none of them should. What are you thoughts?

The others trigger don't fire because an ERROR case is detected before
they have a chance to run, much like on a primary in some ERROR cases.

Thom Brown <thom(at)linux(dot)com> writes:
> It was late last night and I forgot to get around to testing pg_dump,
> which isn't working correctly:

Fixed.

> Also I notice that CREATE/ALTER/DROP COMMAND TRIGGER appears just
> before CREATE/ALTER/DROP TRIGGER in the documentation. This breaks
> the alphabetical order and I wasn't expecting to find it there when
> scanning down the page. Could we move them into an alphabetic
> position?

I don't see that problem in the source files, could you be more specific?

Regards,
--
Dimitri Fontaine
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support