Re: [HACKERS] Command Triggers patch v18

2012-04-03 Thread Robert Haas
On Sun, Apr 1, 2012 at 7:22 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: See above example: I am pretty sure you need a stack. In next version, certainly. As of now I'm willing to start a new stack in each command executed in a command trigger. That means 9.2 will only expose the first

Re: [HACKERS] Command Triggers patch v18

2012-04-03 Thread Dimitri Fontaine
Robert Haas robertmh...@gmail.com writes: I think we're talking past each other. If someone executes DDL command A and the command trigger executes DDL command B which fires another command trigger, then the command trigger for A needs to see the information relevant to A both before and

Re: [HACKERS] Command Triggers patch v18

2012-04-01 Thread Dimitri Fontaine
Robert Haas robertmh...@gmail.com writes: How about calling it command tag? I think both context and toplevel are inconsistent with other uses of those terms: context is an error-reporting field, among other things; and we don't care about the toplevel command, just the command-tag of the one

Re: [HACKERS] Command Triggers patch v18

2012-03-31 Thread Robert Haas
On Fri, Mar 30, 2012 at 11:19 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Yeah context is not explicit, we could call that toplevel: the command tag of the command that the user typed. When toplevel is null, the event trigger is fired on a command the user sent, when it's not null, the

Re: [HACKERS] Command Triggers patch v18

2012-03-30 Thread Dimitri Fontaine
Robert Haas robertmh...@gmail.com writes: I'm thinking of things like extension whitelisting. When some unprivileged user says CREATE EXTENSION harmless, and harmless is marked as superuser-only, we might like to have a hook that gets called *at permissions-checking time* and gets to say, oh,

Re: [HACKERS] Command Triggers patch v18

2012-03-30 Thread Robert Haas
On Fri, Mar 30, 2012 at 4:32 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: I did that another way in previous incarnations of the patch, which was to allow for INSTEAD OF event trigger backed by a SECURITY DEFINER function. When the extension is whitelisted, prevent against recursion then

Re: [HACKERS] Command Triggers patch v18

2012-03-30 Thread Dimitri Fontaine
Robert Haas robertmh...@gmail.com writes: Oh, right: I remember that now. I still think it's a bad way to do it, because the trigger potentially has a lot of work to do to reconstruct a working command string, and it still ends up getting executed by the wrong user. For CREATE EXTENSION it's

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Robert Haas
On Wed, Mar 28, 2012 at 3:41 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: What about :  create command trigger foo before prepare alter table …  create command trigger foo before start of alter table …  create command trigger foo before execute alter table …  create command trigger foo

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Dimitri Fontaine
Robert Haas robertmh...@gmail.com writes: I am sure that we could find a way to beat this with a stick until it behaves, but I don't really like that idea. It seems to me to be a [...] we should learn from that lesson: when you may want to have a bunch of I first wanted to ensure that reusing

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Thom Brown
On 29 March 2012 13:30, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: I'll go make that happen, and still need input here. We first want to have command triggers on specific commands or ANY command, and we want to implement 3 places from where to fire them. Here's a new syntax proposal to

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Robert Haas
On Thu, Mar 29, 2012 at 9:01 AM, Thom Brown t...@linux.com wrote: On 29 March 2012 13:30, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: I'll go make that happen, and still need input here. We first want to have command triggers on specific commands or ANY command, and we want to implement 3

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Robert Haas
On Thu, Mar 29, 2012 at 8:30 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: I'll go make that happen, and still need input here. We first want to have command triggers on specific commands or ANY command, and we want to implement 3 places from where to fire them. Here's a new syntax

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Dimitri Fontaine
Robert Haas robertmh...@gmail.com writes: On Thu, Mar 29, 2012 at 9:01 AM, Thom Brown t...@linux.com wrote: On 29 March 2012 13:30, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: I'll go make that happen, and still need input here. We first want to have command triggers on specific commands or

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Thom Brown
On 29 March 2012 16:30, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Mar 29, 2012 at 9:01 AM, Thom Brown t...@linux.com wrote: On 29 March 2012 13:30, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: I'll go make that happen, and still need

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Dimitri Fontaine
Robert Haas robertmh...@gmail.com writes:     create command trigger before COMMAND_STEP of alter table          execute procedure snitch(); One thought is that it might be better to say AT or ON or WHEN rather than BEFORE, since BEFORE END is just a little strange; and also because a future

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Robert Haas
On Thu, Mar 29, 2012 at 11:49 AM, Thom Brown t...@linux.com wrote: On 29 March 2012 16:30, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Robert Haas robertmh...@gmail.com writes: On Thu, Mar 29, 2012 at 9:01 AM, Thom Brown t...@linux.com wrote: On 29 March 2012 13:30, Dimitri Fontaine

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Dimitri Fontaine
Robert Haas robertmh...@gmail.com writes: I've said repeatedly and over a long period of time that development of this feature wasn't started early enough in the cycle to get it finished in time for 9.2. I think that I've identified some pretty That could well be, yeah. serious issues in

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Robert Haas
On Thu, Mar 29, 2012 at 12:17 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Robert Haas robertmh...@gmail.com writes:     create command trigger before COMMAND_STEP of alter table          execute procedure snitch(); One thought is that it might be better to say AT or ON or WHEN rather

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Robert Haas
Apropos of nothing and since I haven't found a particularly good time to say this in amidst all the technical discussion, I appreciate very much all the work you've been putting into this. On Thu, Mar 29, 2012 at 12:42 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: serious issues in the

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Dimitri Fontaine
Robert Haas robertmh...@gmail.com writes: Well, preceding and before are synonyms, so I don't see any advantage in that change. But I really did mean AT permissions_checking time, not before or after it. That is, we'd have a hook where instead of doing something like this: aclresult =

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Dimitri Fontaine
Robert Haas robertmh...@gmail.com writes: Apropos of nothing and since I haven't found a particularly good time to say this in amidst all the technical discussion, I appreciate very much all the work you've been putting into this. Hey, thanks, I very much appreciate your support here! (1) is

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Robert Haas
On Thu, Mar 29, 2012 at 4:21 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Robert Haas robertmh...@gmail.com writes: Well, preceding and before are synonyms, so I don't see any advantage in that change.  But I really did mean AT permissions_checking time, not before or after it.  That is,

Re: [HACKERS] Command Triggers patch v18

2012-03-28 Thread Dimitri Fontaine
Robert Haas robertmh...@gmail.com writes: I think BEFORE command triggers ideally should run * before permission checks * before locking * before internal checks are done (nameing conflicts, type checks and such) It is possible to do this, and it would actually be much easier and less

Re: [HACKERS] Command Triggers patch v18

2012-03-28 Thread Robert Haas
On Wed, Mar 28, 2012 at 4:12 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Robert Haas robertmh...@gmail.com writes: I think BEFORE command triggers ideally should run * before permission checks * before locking * before internal checks are done (nameing conflicts, type checks and such)

Re: [HACKERS] Command Triggers patch v18

2012-03-28 Thread Robert Haas
On Wed, Mar 28, 2012 at 7:16 AM, Robert Haas robertmh...@gmail.com wrote: Now I can see why implementing that on top of the ANY command feature is surprising enough for wanting to not do it this way. Maybe the answer is to use another keyword to be able to register command triggers that run

Re: [HACKERS] Command Triggers patch v18

2012-03-28 Thread Dimitri Fontaine
Robert Haas robertmh...@gmail.com writes: Further thought: I think maybe we shouldn't use keywords at all for this, and instead use descriptive strings like post-parse or pre-execution or command-start, because I bet in the end we're going to end up with a bunch of them

Re: [HACKERS] Command Triggers patch v18

2012-03-27 Thread Dimitri Fontaine
Hi, First things first, thanks for the review! I'm working on a new version of the patch to fix all the specific comments you called nitpicking here and in your previous email. This new patch will also implement a single list of triggers to run in alphabetical order, not split by ANY/specific

Re: [HACKERS] Command Triggers patch v18

2012-03-27 Thread Robert Haas
On Tue, Mar 27, 2012 at 4:27 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: In the first versions of the patch I did try to have a single point where to integrate the feature and that didn't work out. If you want to publish information such as object id, name and schema you need to have

Re: [HACKERS] Command Triggers patch v18

2012-03-27 Thread Andres Freund
On Tuesday, March 27, 2012 02:55:47 PM Robert Haas wrote: On Tue, Mar 27, 2012 at 4:27 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: In the first versions of the patch I did try to have a single point where to integrate the feature and that didn't work out. If you want to publish

Re: [HACKERS] Command Triggers patch v18

2012-03-27 Thread Robert Haas
On Tue, Mar 27, 2012 at 9:37 AM, Andres Freund and...@2ndquadrant.com wrote: Not necessarily. One use-case is doing something *before* something happens like enforcing naming conventions, data types et al during development/schema migration. That is definitely a valid use case. The only

Re: [HACKERS] Command Triggers patch v18

2012-03-27 Thread Dimitri Fontaine
Robert Haas robertmh...@gmail.com writes: I am coming more and more strongly to the conclusion that we're going in the wrong direction here. It seems to me that you're spending an enormous amount of energy implementing something that goes by the name COMMAND TRIGGER when what you really want

Re: [HACKERS] Command Triggers patch v18

2012-03-27 Thread Dimitri Fontaine
Robert Haas robertmh...@gmail.com writes: I actually think that, to really meet all needs here, we may need the ability to get control in more than one place. For example, one thing that KaiGai has wanted to do (and I bet Dimitri would live to be able to do it too, and I'm almost sure that

Re: [HACKERS] Command Triggers patch v18

2012-03-27 Thread Andres Freund
Hi, On Tuesday, March 27, 2012 04:29:58 PM Robert Haas wrote: On Tue, Mar 27, 2012 at 9:37 AM, Andres Freund and...@2ndquadrant.com wrote: Not necessarily. One use-case is doing something *before* something happens like enforcing naming conventions, data types et al during

Re: [HACKERS] Command Triggers patch v18

2012-03-27 Thread Robert Haas
On Tue, Mar 27, 2012 at 11:05 AM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: I agree that it's not a very helpful decision, but I'm not the one who said we wanted command triggers rather than event triggers.  :-) Color me unconvinced about event triggers. That's not answering my use

Re: [HACKERS] Command Triggers patch v18

2012-03-27 Thread Robert Haas
On Tue, Mar 27, 2012 at 11:58 AM, Andres Freund and...@2ndquadrant.com wrote: I still think the likeliest way towards that is defining some behaviour we want regarding * naming conflict handling * locking And then religiously make sure the patch adheres to that. That might need some

Re: [HACKERS] Command Triggers patch v18

2012-03-27 Thread Andres Freund
Hi, On Tuesday, March 27, 2012 07:34:46 PM Robert Haas wrote: On Tue, Mar 27, 2012 at 11:58 AM, Andres Freund and...@2ndquadrant.com wrote: I still think the likeliest way towards that is defining some behaviour we want regarding * naming conflict handling * locking And then

Re: [HACKERS] Command Triggers patch v18

2012-03-27 Thread Robert Haas
On Tue, Mar 27, 2012 at 4:05 PM, Andres Freund and...@2ndquadrant.com wrote: Looking up oids and such before calling the trigger doesn't seem to be problematic from my pov. Command triggers are a superuser only facility, additional information they gain are no problem. Thinking about that

Re: [HACKERS] Command Triggers patch v18

2012-03-26 Thread Robert Haas
On Tue, Mar 20, 2012 at 1:49 PM, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Hi, I guess I sent v17 a little early considering that we now already have v18 including support for CREATE TABLE AS and SELECT INTO, thanks to the work of Andres and Tom. There was some spurious tags in the

Re: [HACKERS] Command Triggers patch v18

2012-03-26 Thread Robert Haas
On Mon, Mar 26, 2012 at 9:11 PM, Robert Haas robertmh...@gmail.com wrote: [ various trivial issues ] OK, now I got that out of my system. Now on to bigger topics. I am extremely concerned about the way in which this patch arranges to invoke command triggers. You've got call sites spattered

Re: [HACKERS] Command Triggers patch v18

2012-03-25 Thread Andres Freund
On Friday, March 23, 2012 04:32:02 PM Dimitri Fontaine wrote: I would like to get back on code level review now if at all possible, and I would integrate your suggestions here into the next patch revision if another one is needed. Ok, I will give it another go. Btw I just wanted to alert you

Re: [HACKERS] Command Triggers patch v18

2012-03-23 Thread Dimitri Fontaine
Thom Brown t...@linux.com writes: The new command triggers work correctly. Thanks for your continued testing :) Having looked at your regression tests, you don't seem to have enough before triggers in the tests. There's no test for before CREATE TABLE, CREATE TABLE AS or SELECT INTO. In my

Re: [HACKERS] Command Triggers patch v18

2012-03-20 Thread Thom Brown
On 20 March 2012 17:49, Dimitri Fontaine dimi...@2ndquadrant.fr wrote: Hi, I guess I sent v17 a little early considering that we now already have v18 including support for CREATE TABLE AS and SELECT INTO, thanks to the work of Andres and Tom. There was some spurious tags in the sgml files