From: Dimitri Fontaine <dimitri@2ndQuadrant.fr>
To: Thom Brown <thom@linux.com>
Cc: Dimitri Fontaine <dimitri@2ndquadrant.fr>
Cc: pgsql-hackers@postgresql.org, Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>
Subject: Re: Command Triggers, patch v11
Date: Fri, 09 Mar 2012 22:38:53 +0100
Message-ID: <m2fwdhjvyq.fsf@2ndQuadrant.fr> (raw)
In-Reply-To: <CAA-aLv5M5wJtMcCsfj-44vxHC4iYw=itkVFTpQz0W_76TdtV+Q@mail.gmail.com>
References: <m24nufq466.fsf@2ndQuadrant.fr>
<CAA-aLv4kv3DXuwXdDyU2qcWLjvYCsVo5=qTZq+t8uk5S9sAkqQ@mail.gmail.com>
<CAA-aLv5paBN4P-cXUsu=KMQva-SCSjo9MpzM6szYFt5wqeFkeg@mail.gmail.com>
<201203041650.39826.andres@anarazel.de>
<CAA-aLv66UQ3mqsk96AUhc9zqw8hwEPdCrGHv81+F3=puSiw5Jg@mail.gmail.com>
<m2mx7uye3r.fsf@2ndQuadrant.fr>
<CAA-aLv6SioRE8MAED2z=0c2NZpf1q5jztwjzE6+gvGLbniV7aw@mail.gmail.com>
<m2ty21fnkw.fsf@2ndQuadrant.fr>
<CAA-aLv5-KAMD_PMEUK=SiG_OBdM4jiQ3O=kpw8fZM+-Uq4PJ1g@mail.gmail.com>
<CAA-aLv46eeyi_ecPLOaY61OL5e+BWkba4_qrJ7V+=u2afbwCMg@mail.gmail.com>
<m21up2wx2h.fsf@2ndQuadrant.fr>
<CAA-aLv5M5wJtMcCsfj-44vxHC4iYw=itkVFTpQz0W_76TdtV+Q@mail.gmail.com>
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@linux.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@linux.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@linux.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
Attachments:
command-trigger.v15.patch.gz (application/octet-stream, 67.4K) download
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: dimitri@2ndQuadrant.fr, thom@linux.com, andres@anarazel.de, tgl@sss.pgh.pa.us
Subject: Re: Command Triggers, patch v11
In-Reply-To: <m2fwdhjvyq.fsf@2ndQuadrant.fr>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox