Re: Command Triggers, v16

Thom Brown <thombrown(at)gmail(dot)com> writes:
> Note: incremental patch attached for the following section...

Applied, thanks!

> I don’t know if this was a problem before that I didn’t spot
> (probably), but triggers for both ANY COMMAND and ALTER FOREIGN TABLE
> show a command tag of ALTER TABLE for ALTER FOREIGN TABLE statements
> where the column is renamed:
>
> I don’t think this is the fault of the trigger code because it
> actually says ALTER TABLE at the bottom, suggesting it’s something
> already present. This isn’t the case when adding or dropping columns.
> Any comments?

We're building command trigger on top of the existing code. From the
grammar we can see that an alter table and an alter foreign table are
processed as the same command.

AlterForeignTableStmt:
ALTER FOREIGN TABLE relation_expr alter_table_cmds
{
AlterTableStmt *n = makeNode(AlterTableStmt);

So while I think we could want to revisit that choice down the road, I
don't think that's for the command triggers patch to do it.

> Altering the properties of a function (such as cost, security definer,
> whether it’s stable etc) doesn’t report the function’s OID:

That's now fixed. I've checked that all the other places where we're
saying objectId = InvalidOid are related to before create or after drop
commands.

> I get a garbage objectname for AFTER ANY COMMAND triggers on ALTER
> TEXT SEARCH DICTIONARY when changing its options. It doesn’t show it
> in the below example because I can’t get it displaying in plain text,
> but where the objectname is blank is where I’m seeing unicode a square
> containing “0074” 63 times in a row:

I couldn't reproduce, but I've spotted the problem in the source, and
fixed it. I could find a couple other places that should have been using
pstrdup(NameStr(…)) too, and fixed them. I added a test.

> Specific command triggers on ALTER VIEW don’t work at all:

Can't reproduce, and that's already part of the regression tests.

> Command triggers that fire for CREATE RULE show a schema, but DROP
> RULE doesn’t. Which is it?:

Oh, both RULE and TRIGGER are not qualified, and I was filling the
schemaname with the schema of the relation they refer to in the CREATE
command, and had DROP correctly handling the TRIGGER case.

That's now fixed, schemaname is NULL in all cases here.

You can read the changes here:

https://github.com/dimitri/postgres/compare/5e8e37922d...144d870162

And I've been attaching an incremental patch too. Next patch revision is
expected later today with support for plpython, plperl and pltcl.

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