On 30.10.24 13:31, jian he wrote:
On Tue, Oct 29, 2024 at 7:54 PM Peter Eisentraut <pe...@eisentraut.org> wrote:

I made a patch for this.  I have expanded the narrative discussion on
what commands are supported for event triggers, also made a few
corrections/additions there, based on inspecting the source code.  And
then removed the big matrix, which doesn't provide any additional
information, I think.

I think this is sufficient and covers everything.  The only hand-wavy
thing I can see is exactly which ALTER commands trigger the sql_drop
event.  But this was already quite imprecise before, and I think also
not quite correct.  This might need a separate investigation.

In any case, we can use this as a starting point to iterate on the right
wording etc.

hi. I have some minor issue.

    <para>
      An event trigger fires whenever the event with which it is associated
      occurs in the database in which it is defined.
</para>
is possible to rewrite this sentence, two "which" is kind of not easy
to understand?

I couldn't think of anything simpler that wouldn't be weirdly nested in some other way. This wasn't really related to this patch, so I didn't touch it. But suggestions are welcome.

create role alice;
create role bob;
grant alice to bob;
    <para>
      As an exception, this event does not occur for DDL commands targeting
      shared objects:
      <itemizedlist>
       <listitem><para>databases</para></listitem>
       <listitem><para>roles</para></listitem>
       <listitem><para>tablespaces</para></listitem>
       <listitem><para>parameter privileges</para></listitem>
       <listitem><para><command>ALTER SYSTEM</command></para></listitem>
      </itemizedlist>
      This event also does not occur for commands targeting event triggers
      themselves.
    </para>

not 100% sure this description
"      <listitem><para>roles</para></listitem>"
cover case like "grant alice to bob;"
Here "targeting  shared objects" is "role related meta information".
maybe a new item like
<listitem><para>roles privileges</para></listitem>.

Yeah, I added a clarification in the committed version.



Reply via email to