Re: Command Triggers, patch v11 - Mailing list pgsql-hackers

Hi,

Thom Brown <thom@linux.com> writes:
> The message returned by creating a command trigger after create index
> is still problematic:

Fixed.  I'm attaching an incremental patch here, the github branch is
updated too.

> CREATE VIEW doesn't return schema:

Fixed, and as an added bonus I fixed the CREATE SEQUENCE oddity about
that too.

> No specific triggers fire when altering a conversion:

Couldn't reproduce, works here, added tests.

> No specific triggers fire when altering the properties of a function:

Fixed.

> No specific triggers fire when altering a sequence:

Couldn't reproduce, added tests.

> No specific triggers when altering a view:

Same again.

> The object name shown in specific triggers when dropping aggregates
> shows the schema name:
> Same for collations:
> Dropping functions shows the object name as the schema name:
> Same with dropping operators:
> ...and operator family:
> … and the same for dropping text search
> configuration/dictionary/parser/template.

Fixed in the attached (all of those where located at exactly the same
place, by the way, that's one fix).

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

I'm using format_type_be(objectId) so that int4 is integer and int8
bigint etc, but that's adding the schemaname. I didn't have time to look
for another API that wouldn't add the schemaname nor to add one myself,
will do that soon.

> I hadn't previously tested triggers against CLUSTER, REINDEX, VACUUM
> and LOAD, but have tested them now.

Cool :)

> When creating a trigger on REINDEX, I get the following message:

Fixed.

> Creating AFTER CLUSTER command triggers produce an error (as expected
> since it's not supported), but AFTER REINDEX only produces a warning.
> These should be the same, probably both an error.

Fixed.

> VACUUM doesn't fire a specific command trigger:

I though it was better this way, I changed my mind and completed the code.

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

Still on the TODO.

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

Yeah well.  Will see about how much damage needs to be done in the
current APIs, running out of steam for tonight's batch.

> REINDEXing the whole database doesn't fire specific command triggers:

We don't want to because REINDEX DATABASE is managing transactions on
its own, same limitation as with AFTER VACUUM and all.  Will have a look
at what it takes to document that properly.

> Documentation:

Fixed.

Thom Brown <thom@linux.com> writes:
> I've also since found that if I issue a VACUUM, CLUSTER or REINDEX on
> a read-only standby, the BEFORE ANY COMMAND trigger fires.  I don't
> think any trigger should fire on a read-only standby.

Well I'm not sold on that myself (think pl/untrusted that would reach
out to the OS and do whatever is needed there).  You can even set the
session_replication_role GUC to replica and only have the replica
command triggers fired.

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