|From:||Dimitri Fontaine <dimitri(at)2ndQuadrant(dot)fr>|
|To:||Robert Haas <robertmhaas(at)gmail(dot)com>|
|Cc:||Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: Event Triggers reduced, v1|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> That's not quite what I was thinking. I actually can't imagine any
> situation where you want an event trigger that gets fired on EVERY
> command for which we can support command_start. If you're trying to
I don't have any solid use case in mind, just though it would make the
feature rather complete. Withdrawn.
> So my proposal for the present patch would be:
> 1. Rename command_start to ddl_command_start.
> 2. Remove support for everything other than CREATE, ALTER, and DROP.
> 3. Pass the operation and the SQL object type as separate magic variables.
All done in the attached. As usual, you get an incremental patch and a
complete one. It's also published on github for easy browsing:
> Yep, sure. Note that the proposal above constrains the list of
> commands we support in v1 in a very principled way: CREATE, ALTER,
> DROP. Everything else can be added later under a different (but
> similarly situated) firing point name. If we stick with command_start
> then I think we're going to be forever justifying our decisions as to
> what got included or excluded; which might be worth it if it seemed
> likely that there'd be much use for such a command trigger, but it
> doesn't (to me, anyway).
Yeah, done that way. I had to remove support for only 4 commands…
> I'm imagining that ddl_command_start triggers would get the
> information this way, but LOAD might be covered by something like
> admin_command_start that just gets the command tag.
My point was that I didn't want to replace the command tag by the object
type and operation combo, happy to see we're on the same line.
> EventTriggerCommandTagsEntry mapping the command tag to an ETC_*
> constant; I think the need for that hash goes away entirely if you
> just pass this information down from the ProcessUtility() switch. At
Well, no, because we use that hash mapping in BuildEventTriggerCache(),
when reading from the catalogs. I don't see a way to cut down on the
number of per-session hash-table here without involving a penalty.
> any rate having NameData involved seems like it's probably not too
> good an idea; if for some reason we need to keep that hash, use a
> NUL-terminated string and initialize the hash table with string_hash
> instead of tag_hash. That'll be simpler and also allows the length of
> the buffer to vary independently of NAMEDATALEN, which can only be to
> the good.
Oh, I just failed to realize that the hash table key mustn't be a static
length field. I'm done for tonight though, it's still something to fix.
Maybe you will beat me to it? :) (if not, I'll be happy to, with some
luck even tomorrow).
http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
|Next Message||Craig Ringer||2012-07-12 01:23:16||Re: DELETE vs TRUNCATE explanation|
|Previous Message||Alexander Korotkov||2012-07-11 23:11:19||Re: SP-GiST for ranges based on 2d-mapping and quad-tree|