Re: [HACKERS] Command Triggers patch v18

2012-04-03 Thread Dimitri Fontaine
Robert Haas 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 after the command > tr

Re: [HACKERS] Command Triggers patch v18

2012-04-03 Thread Robert Haas
On Sun, Apr 1, 2012 at 7:22 AM, Dimitri Fontaine 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 level of the stac

Re: [HACKERS] Command Triggers patch v18

2012-04-01 Thread Dimitri Fontaine
Robert Haas 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 we're executing

Re: [HACKERS] Command Triggers patch v18

2012-03-31 Thread Robert Haas
On Fri, Mar 30, 2012 at 11:19 AM, Dimitri Fontaine 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 > trigger is fired

Re: [HACKERS] Command Triggers patch v18

2012-03-30 Thread Dimitri Fontaine
Robert Haas 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 not that bad, Th

Re: [HACKERS] Command Triggers patch v18

2012-03-30 Thread Robert Haas
On Fri, Mar 30, 2012 at 4:32 AM, Dimitri Fontaine 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 CREATE EXTENSION i

Re: [HACKERS] Command Triggers patch v18

2012-03-30 Thread Dimitri Fontaine
Robert Haas 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, well, that > ex

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Robert Haas
On Thu, Mar 29, 2012 at 4:21 PM, Dimitri Fontaine wrote: > Robert Haas 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 >>

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Dimitri Fontaine
Robert Haas 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 not hard to fix

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Dimitri Fontaine
Robert Haas 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 = pg_class_aclcheck(re

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 wrote: >> serious issues in the discussion we've had so f

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Robert Haas
On Thu, Mar 29, 2012 at 12:17 PM, Dimitri Fontaine wrote: > Robert Haas 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 i

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Dimitri Fontaine
Robert Haas 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 the discussion we've

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Robert Haas
On Thu, Mar 29, 2012 at 11:49 AM, Thom Brown wrote: > On 29 March 2012 16:30, Dimitri Fontaine wrote: >> Robert Haas writes: >>> On Thu, Mar 29, 2012 at 9:01 AM, Thom Brown wrote: On 29 March 2012 13:30, Dimitri Fontaine wrote: > I'll go make that happen, and still need input here. We

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Dimitri Fontaine
Robert Haas 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 hook point mi

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Thom Brown
On 29 March 2012 16:30, Dimitri Fontaine wrote: > Robert Haas writes: >> On Thu, Mar 29, 2012 at 9:01 AM, Thom Brown wrote: >>> On 29 March 2012 13:30, Dimitri Fontaine wrote: I'll go make that happen, and still need input here. We first want to have command triggers on specific comma

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Dimitri Fontaine
Robert Haas writes: > On Thu, Mar 29, 2012 at 9:01 AM, Thom Brown wrote: >> On 29 March 2012 13:30, Dimitri Fontaine 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 plac

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Robert Haas
On Thu, Mar 29, 2012 at 8:30 AM, Dimitri Fontaine 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 cope with t

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Robert Haas
On Thu, Mar 29, 2012 at 9:01 AM, Thom Brown wrote: > On 29 March 2012 13:30, Dimitri Fontaine 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.

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Thom Brown
On 29 March 2012 13:30, Dimitri Fontaine 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 cope with that: > >

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Dimitri Fontaine
Robert Haas 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 existing parser ke

Re: [HACKERS] Command Triggers patch v18

2012-03-29 Thread Robert Haas
On Wed, Mar 28, 2012 at 3:41 PM, Dimitri Fontaine 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 before end of al

Re: [HACKERS] Command Triggers patch v18

2012-03-28 Thread Dimitri Fontaine
Robert Haas 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 (create-schema-subcommand-start? > alter-

Re: [HACKERS] Command Triggers patch v18

2012-03-28 Thread Robert Haas
On Wed, Mar 28, 2012 at 7:16 AM, Robert Haas 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 >> that early and w

Re: [HACKERS] Command Triggers patch v18

2012-03-28 Thread Robert Haas
On Wed, Mar 28, 2012 at 4:12 AM, Dimitri Fontaine wrote: > Robert Haas 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

Re: [HACKERS] Command Triggers patch v18

2012-03-28 Thread Dimitri Fontaine
Robert Haas 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 invasive to impl

Re: [HACKERS] Command Triggers patch v18

2012-03-27 Thread Robert Haas
On Tue, Mar 27, 2012 at 4:05 PM, Andres Freund 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 I think we

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 wrote: > > I still think the likeliest way towards that is defining some behaviour > > we want regarding > > * naming conflict handling > > * locking > > > > And then religiously make

Re: [HACKERS] Command Triggers patch v18

2012-03-27 Thread Robert Haas
On Tue, Mar 27, 2012 at 11:58 AM, Andres Freund 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 > refactoring of exist

Re: [HACKERS] Command Triggers patch v18

2012-03-27 Thread Robert Haas
On Tue, Mar 27, 2012 at 11:05 AM, Dimitri Fontaine 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 > cases. Could we get

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 wrote: > > Not necessarily. One use-case is doing something *before* something > > happens like enforcing naming conventions, data types et al during > > development/schema migration. >

Re: [HACKERS] Command Triggers patch v18

2012-03-27 Thread Dimitri Fontaine
Robert Haas 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 Dan Farina would lik

Re: [HACKERS] Command Triggers patch v18

2012-03-27 Thread Dimitri Fontaine
Robert Haas 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 is an EVENT TRIGGE

Re: [HACKERS] Command Triggers patch v18

2012-03-27 Thread Robert Haas
On Tue, Mar 27, 2012 at 9:37 AM, Andres Freund 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 reason why we don't 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 > > 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

Re: [HACKERS] Command Triggers patch v18

2012-03-27 Thread Robert Haas
On Tue, Mar 27, 2012 at 4:27 AM, Dimitri Fontaine 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 > specialized code spre

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 c

Re: [HACKERS] Command Triggers patch v18

2012-03-26 Thread Robert Haas
On Mon, Mar 26, 2012 at 9:11 PM, Robert Haas 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 all over the place, and

Re: [HACKERS] Command Triggers patch v18

2012-03-26 Thread Robert Haas
On Tue, Mar 20, 2012 at 1:49 PM, Dimitri Fontaine 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 in v17 t

Re: [HACKERS] Command Triggers, v16

2012-03-26 Thread Robert Haas
On Mar 26, 2012, at 5:36 PM, Tom Lane wrote: > Robert Haas writes: >> 2. I'm not sure which patches Tom is planning to look at or in what >> order, so I've been avoiding the ones he seems to be taking an >> interest in. > > Well, I think I'm definitely on the hook for the pg_stat_statements, > p

Re: [HACKERS] Command Triggers, v16

2012-03-26 Thread Tom Lane
Robert Haas writes: > 2. I'm not sure which patches Tom is planning to look at or in what > order, so I've been avoiding the ones he seems to be taking an > interest in. Well, I think I'm definitely on the hook for the pg_stat_statements, pgsql_fdw, foreign table stats, and caching-stable-subexpr

Re: [HACKERS] Command Triggers, v16

2012-03-26 Thread Christopher Browne
On Mon, Mar 26, 2012 at 4:45 PM, Robert Haas wrote: > As soon as we're done here, the CommitFest will end, and there won't > be any other people's patches to review. Hurray? :-) -- When confronted by a difficult problem, solve it by reducing it to the question, "How would the Lone Ranger handle

Re: [HACKERS] Command Triggers, v16

2012-03-26 Thread Robert Haas
On Mon, Mar 26, 2012 at 4:36 PM, Dimitri Fontaine wrote: > Well, wait a minute. There's a difference between half-baked and > reacting to a review that changes the goal of a patch. My idea of the > code I wanted to write here is extremely different from what we as a > community decided to be doing

Re: [HACKERS] Command Triggers, v16

2012-03-26 Thread Thom Brown
On 26 March 2012 21:36, Dimitri Fontaine wrote: > Robert Haas writes: >> Personally, I am about at the point where I'd like to punt everything >> and move on.  As nice as it would be to squeeze a few more things into >> 9.2, there WILL be a 9.3.  If a few less people had submitted >> half-baked c

Re: [HACKERS] Command Triggers, v16

2012-03-26 Thread Dimitri Fontaine
Robert Haas writes: > 1. It sure seems like there is an awful lot of code churn and design > work going on. There has only been minor adjustments for a while now, and they have been visible because Thom was doing lots of testing for me and it was way easier for me to publish a new version and hav

Re: [HACKERS] Command Triggers, v16

2012-03-26 Thread Andres Freund
On Monday, March 26, 2012 10:18:59 PM Dimitri Fontaine wrote: > don't know how to fix the plpython specifics he's talking > about. Just copy what is done in the normal trigger handling facility (the decref both in the CATCH and in the normal path). Ping me some other way if you need help... And

Re: [HACKERS] Command Triggers, v16

2012-03-26 Thread Dimitri Fontaine
Tom Lane writes: > So I don't think that the mere fact of being an ANY trigger rather than > a command-specific trigger should be taken to mean that a particular > ordering is desirable. Trigger name order isn't the greatest solution > by any means, but it's more flexible than hard-wiring accordi

Re: [HACKERS] Command Triggers, v16

2012-03-26 Thread Robert Haas
On Mon, Mar 26, 2012 at 3:36 PM, Thom Brown wrote: > Can someone clarify whether this will be reviewed by a committer? > Will there be time to get this reviewed before the commitfest closes? > I get the impression the commitfest closure is fairly imminent. Well, I have been holding off for two re

Re: [HACKERS] Command Triggers, v16

2012-03-26 Thread Tom Lane
Thom Brown writes: > Can someone clarify whether this will be reviewed by a committer? > Will there be time to get this reviewed before the commitfest closes? > I get the impression the commitfest closure is fairly imminent. I don't have that impression. (I wish I did.)

Re: [HACKERS] Command Triggers, v16

2012-03-26 Thread Tom Lane
Dimitri Fontaine writes: > Robert Haas writes: >> Dimitri's proposed behavior would be advantageous if you have an ANY >> trigger that wants to "take over the world" and make sure that nobody >> else can run before it. I think, though, that's not a case we want to >> cater to - all of this stuff

Re: [HACKERS] Command Triggers, v16

2012-03-26 Thread Robert Haas
On Mon, Mar 26, 2012 at 3:24 PM, Dimitri Fontaine wrote: > One use case would be for londiste/slony/bucardo to rewrite the command > and queue its text, that will be done in C and should probably be done > first. Using an ANY command trigger means that code will run before user > specific code (or

Re: [HACKERS] Command Triggers, v16

2012-03-26 Thread Thom Brown
Can someone clarify whether this will be reviewed by a committer? Will there be time to get this reviewed before the commitfest closes? I get the impression the commitfest closure is fairly imminent. Thom -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to yo

Re: [HACKERS] Command Triggers, v16

2012-03-26 Thread Dimitri Fontaine
Robert Haas writes: > Dimitri's proposed behavior would be advantageous if you have an ANY > trigger that wants to "take over the world" and make sure that nobody > else can run before it. I think, though, that's not a case we want to > cater to - all of this stuff requires superuser privileges a

Re: [HACKERS] Command Triggers, v16

2012-03-26 Thread Robert Haas
On Sun, Mar 25, 2012 at 12:15 PM, Andres Freund wrote: > On Friday, March 16, 2012 10:40:46 AM Dimitri Fontaine wrote: >> > This will have the effect of calling triggers outside of alphabetic >> > order. I don't think thats a good idea even if one part is ANY and the >> > other a specific command.

Re: [HACKERS] Command Triggers, v16

2012-03-25 Thread Andres Freund
On Friday, March 16, 2012 10:40:46 AM Dimitri Fontaine wrote: > > This will have the effect of calling triggers outside of alphabetic > > order. I don't think thats a good idea even if one part is ANY and the > > other a specific command. > > I don't think there is any reason anymore to separate th

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 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 tests I

Re: [HACKERS] Command Triggers

2012-03-21 Thread Robert Haas
On Wed, Mar 21, 2012 at 11:58 AM, Andres Freund wrote: > On Wednesday, March 21, 2012 04:54:00 PM Robert Haas wrote: >> On Tue, Mar 20, 2012 at 12:02 PM, Andres Freund wrote: >> > The attached patch applies from 8.3 to 9.1 (8.2 has conflicts but >> > thankfully...). >> >> It seems it doesn't appl

Re: [HACKERS] Command Triggers

2012-03-21 Thread Andres Freund
On Wednesday, March 21, 2012 04:54:00 PM Robert Haas wrote: > On Tue, Mar 20, 2012 at 12:02 PM, Andres Freund wrote: > > The attached patch applies from 8.3 to 9.1 (8.2 has conflicts but > > thankfully...). > > It seems it doesn't apply to master (any more?). Its not required there because of the

Re: [HACKERS] Command Triggers

2012-03-21 Thread Robert Haas
On Tue, Mar 20, 2012 at 12:02 PM, Andres Freund wrote: > The attached patch applies from 8.3 to 9.1 (8.2 has conflicts but > thankfully...). It seems it doesn't apply to master (any more?). -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via

Re: [HACKERS] Command Triggers patch v18

2012-03-20 Thread Thom Brown
On 20 March 2012 17:49, Dimitri Fontaine 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 in v17 that I did

Re: [HACKERS] Command Triggers, patch v11

2012-03-20 Thread Dimitri Fontaine
Tom Lane writes: > I've applied the CTAS patch after rather heavy editorialization. Don't > know what consequences that will have for Dimitri's patch. It allows my patch to add support for CREATE TABLE AS and SELECT INTO, I've been doing that and am on my way to sending a v18 now. The way you wo

Re: [HACKERS] Command Triggers

2012-03-20 Thread Andres Freund
On Tuesday, February 28, 2012 12:43:02 AM Andres Freund wrote: > On Tuesday, February 28, 2012 12:30:36 AM Tom Lane wrote: > > Andres Freund writes: > > > Sorry for letting this slide. > > > > > > Is it worth adding this bit to OpenIntoRel? Not sure if there is danger > > > in allowing anyone to

Re: [HACKERS] Command Triggers, patch v11

2012-03-20 Thread Andres Freund
On Tuesday, March 20, 2012 02:39:56 AM Tom Lane wrote: > I've applied the CTAS patch after rather heavy editorialization. Don't > know what consequences that will have for Dimitri's patch. Thanks for all the work you put into this! Looks cleaner now... Thanks, Andres -- Sent via pgsql-hackers

Re: [HACKERS] Command Triggers, patch v11

2012-03-19 Thread Tom Lane
I've applied the CTAS patch after rather heavy editorialization. Don't know what consequences that will have for Dimitri's patch. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.post

Re: [HACKERS] Command Triggers, patch v11

2012-03-19 Thread Tom Lane
I wrote: > One thing I soon found is that it lacks support for EXPLAIN SELECT INTO. While I'm not particularly excited about fixing PREPARE ... SELECT INTO or CREATE RULE ... SELECT INTO, I've come to the conclusion that the EXPLAIN case is a must-fix. Because not only is EXPLAIN SELECT INTO brok

Re: [HACKERS] Command Triggers, patch v11

2012-03-19 Thread Tom Lane
Robert Haas writes: > On Mon, Mar 19, 2012 at 12:53 PM, Peter Eisentraut wrote: >> Doesn't seem worth it to me. At least, "SELECT " makes some sense: >> rows were selected. "CREATE TABLE " means what? tables >> were created? >> >> What might make sense is to delegate this ad

Re: [HACKERS] Command Triggers, patch v11

2012-03-19 Thread Robert Haas
On Mon, Mar 19, 2012 at 12:53 PM, Peter Eisentraut wrote: > On sön, 2012-03-18 at 21:16 -0400, Tom Lane wrote: >> If we were going to change the output at all, I would vote for "CREATE >> TABLE " so as to preserve the rowcount functionality.  Keep in >> mind though that this would force client

Re: [HACKERS] Command Triggers, patch v11

2012-03-19 Thread Peter Eisentraut
On sön, 2012-03-18 at 21:16 -0400, Tom Lane wrote: > If we were going to change the output at all, I would vote for "CREATE > TABLE " so as to preserve the rowcount functionality. Keep in > mind though that this would force client-side changes, for instance in > libpq's PQcmdTuples(). Fixing

Re: [HACKERS] Command Triggers, patch v11

2012-03-19 Thread Robert Haas
On Mon, Mar 19, 2012 at 12:45 PM, Tom Lane wrote: > Robert Haas writes: >> On Sun, Mar 18, 2012 at 2:29 PM, Tom Lane wrote: >>> 1. ExecuteQuery still has to know that's a CREATE TABLE AS operation so >>> that it can enforce that the prepared query is a SELECT.  (BTW, maybe >>> this should be wea

Re: [HACKERS] Command Triggers, patch v11

2012-03-19 Thread Tom Lane
Robert Haas writes: > On Sun, Mar 18, 2012 at 2:29 PM, Tom Lane wrote: >> 1. ExecuteQuery still has to know that's a CREATE TABLE AS operation so >> that it can enforce that the prepared query is a SELECT. (BTW, maybe >> this should be weakened to "something that returns tuples", in view of >> R

Re: [HACKERS] Command Triggers, patch v11

2012-03-19 Thread Robert Haas
On Sun, Mar 18, 2012 at 2:29 PM, Tom Lane wrote: > 1. ExecuteQuery still has to know that's a CREATE TABLE AS operation so > that it can enforce that the prepared query is a SELECT.  (BTW, maybe > this should be weakened to "something that returns tuples", in view of > RETURNING?) +1 for "somethi

Re: [HACKERS] Command Triggers, patch v11

2012-03-19 Thread Andres Freund
On Sunday, March 18, 2012 07:29:30 PM Tom Lane wrote: > BTW, I've been looking through how to do what I suggested earlier to get > rid of the coziness and code duplication between CreateTableAs and the > prepare.c code; namely, let CreateTableAs create a DestReceiver and then > call ExecuteQuery wi

Re: [HACKERS] Command Triggers, patch v11

2012-03-19 Thread Dimitri Fontaine
Thom Brown writes: > Will you also be committing the trigger function variable changes > shortly? I don't wish to test anything prior to this as that will > involve a complete re-test from my side anyway. It's on its way, I had to spend time elsewhere, sorry about that. With some luck I can post

Re: [HACKERS] Command Triggers, patch v11

2012-03-18 Thread Tom Lane
Peter Eisentraut writes: > On lör, 2012-03-17 at 18:04 -0400, Tom Lane wrote: >> I'm not sure what we should do instead. We have gotten push-back before >> anytime we changed the command tag for an existing command (and in fact >> it seems that we intentionally added the rowcount display in 9.0,

Re: [HACKERS] Command Triggers, patch v11

2012-03-18 Thread Tom Lane
Dimitri Fontaine writes: > That lights a bulb: what about rewriting CREATE TABLE AS as two > commands, internally: Given the compatibility constraints on issues like what command tag to return, I think that would probably make our jobs harder not easier. regards, tom lane

Re: [HACKERS] Command Triggers, patch v11

2012-03-18 Thread Peter Eisentraut
On lör, 2012-03-17 at 18:04 -0400, Tom Lane wrote: > I'm not sure what we should do instead. We have gotten push-back before > anytime we changed the command tag for an existing command (and in fact > it seems that we intentionally added the rowcount display in 9.0, which > means there are people

Re: [HACKERS] Command Triggers, patch v11

2012-03-18 Thread Dimitri Fontaine
Tom Lane writes: > 1. ExecuteQuery still has to know that's a CREATE TABLE AS operation so > that it can enforce that the prepared query is a SELECT. (BTW, maybe > this should be weakened to "something that returns tuples", in view of > RETURNING?) That lights a bulb: what about rewriting CREATE

Re: [HACKERS] Command Triggers, patch v11

2012-03-18 Thread Tom Lane
BTW, I've been looking through how to do what I suggested earlier to get rid of the coziness and code duplication between CreateTableAs and the prepare.c code; namely, let CreateTableAs create a DestReceiver and then call ExecuteQuery with that receiver. It appears that we still need at least two

Re: [HACKERS] Command Triggers, patch v11

2012-03-17 Thread Andres Freund
On Saturday, March 17, 2012 11:04:30 PM Tom Lane wrote: > I've found a couple more issues in the CTAS patch: > > 1. Previous versions delivered a "SELECT n" command tag for either > spelling of the command: > > regression=# select * into t1 from int8_tbl; > SELECT 6 > regression=# create table t2

Re: [HACKERS] Command Triggers, patch v11

2012-03-17 Thread Tom Lane
I've found a couple more issues in the CTAS patch: 1. Previous versions delivered a "SELECT n" command tag for either spelling of the command: regression=# select * into t1 from int8_tbl; SELECT 6 regression=# create table t2 as select * from int8_tbl; SELECT 6 With the patch I get regression=#

Re: [HACKERS] Command Triggers, patch v11

2012-03-17 Thread Andres Freund
On Saturday, March 17, 2012 06:45:27 PM Tom Lane wrote: > I'm not sure that anybody cares about being able to fire command > triggers on DECLARE CURSOR I actually think it would make sense to explicitly not fire command triggers there given that DECLARE CURSOR actually potentially is somewhat perf

Re: [HACKERS] Command Triggers, patch v11

2012-03-17 Thread Tom Lane
While looking at this I also noticed that DECLARE CURSOR uses a structure that's randomly different in yet a third way: we start with a utility statement containing a query, and then flip that upside down so that the SELECT Query contains a utility statement! I have a vague feeling that I'm the on

Re: [HACKERS] Command Triggers, patch v11

2012-03-17 Thread Tom Lane
Andres Freund writes: > On Friday, March 16, 2012 10:52:55 PM Tom Lane wrote: >> Something else I just came across is that there are assorted places that >> are aware that ExplainStmt contains a Query, eg setrefs.c, plancache.c, >> and those have got to treat CreateTableAsStmt similarly. > Hm. Is

Re: [HACKERS] Command Triggers, patch v11

2012-03-16 Thread Andres Freund
On Friday, March 16, 2012 10:52:55 PM Tom Lane wrote: > Andres Freund writes: > > On Friday, March 16, 2012 10:31:57 PM Tom Lane wrote: > >> I'm thinking that if the table creation > >> were to be moved into the tuple receiver's startup routine, we could > >> avoid needing to get control back betw

Re: [HACKERS] Command Triggers, patch v11

2012-03-16 Thread Tom Lane
Andres Freund writes: > On Friday, March 16, 2012 10:31:57 PM Tom Lane wrote: >> I'm thinking that if the table creation >> were to be moved into the tuple receiver's startup routine, we could >> avoid needing to get control back between ExecutorStartup and >> ExecutorRun, and then all that would

Re: [HACKERS] Command Triggers, patch v11

2012-03-16 Thread Andres Freund
On Friday, March 16, 2012 10:31:57 PM Tom Lane wrote: > Andres Freund writes: > > One more thing I disliked quite a bit was the duplication of the EXECUTE > > handling. Do you see a way to deduplicate that? > Yeah, that's what's bugging me, too. I think a chunk of the problem is > that you're ins

Re: [HACKERS] Command Triggers, patch v11

2012-03-16 Thread Tom Lane
Andres Freund writes: > One more thing I disliked quite a bit was the duplication of the EXECUTE > handling. Do you see a way to deduplicate that? Yeah, that's what's bugging me, too. I think a chunk of the problem is that you're insisting on having control come back to CreateTableAs() to perfo

Re: [HACKERS] Command Triggers, patch v11

2012-03-16 Thread Andres Freund
On Friday, March 16, 2012 09:54:47 PM Tom Lane wrote: > Andres Freund writes: > > [ ctas-01.patch ] > > I'm starting to look at this now. Great! > For a patch that's supposed to de-complicate things, it seems pretty messy :-( Yea. It started out simple but never stopped getting more complicated

Re: [HACKERS] Command Triggers, patch v11

2012-03-16 Thread Tom Lane
Andres Freund writes: > [ ctas-01.patch ] I'm starting to look at this now. For a patch that's supposed to de-complicate things, it seems pretty messy :-( One thing I soon found is that it lacks support for EXPLAIN SELECT INTO. That used to work, but now you get regression=# explain select * i

Re: [HACKERS] Command Triggers, v16

2012-03-16 Thread Thom Brown
On 16 March 2012 16:26, Tom Lane wrote: > Dimitri Fontaine writes: >> Tom Lane writes: >>> If you think "cmdtrigger" isn't a good name maybe you should have >>> picked a different one to start with. > >> Well, I think it's a good internal name. I'm not too sure about exposing >> it, the only rea

Re: [HACKERS] Command Triggers, v16

2012-03-16 Thread Tom Lane
Dimitri Fontaine writes: > Tom Lane writes: >> If you think "cmdtrigger" isn't a good name maybe you should have >> picked a different one to start with. > Well, I think it's a good internal name. I'm not too sure about exposing > it, the only reason why it's a good name is because it's a single

Re: [HACKERS] Command Triggers, v16

2012-03-16 Thread Dimitri Fontaine
Robert Haas writes: > there has to be some way to do that without breaking command triggers. Sure, special case the switch branch in utility.c so as to return a different command tag for ALTER TABLE and ALTER FOREIGN TABLE. For precedents, see AlterObjectTypeCommandTag(ObjectType objtype) and

Re: [HACKERS] Command Triggers, v16

2012-03-16 Thread Robert Haas
On Fri, Mar 16, 2012 at 7:42 AM, Dimitri Fontaine wrote: >> 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:

Re: [HACKERS] Command Triggers, v16

2012-03-16 Thread Dimitri Fontaine
Tom Lane writes: > Multi-word type names are a serious pain in the ass; they require > hackery in a lot of places. We support the ones that the SQL spec > requires us to, but I will object in the strongest terms to inventing > any that are not required by spec. I object in even stronger terms to

Re: [HACKERS] Command Triggers, v16

2012-03-16 Thread Tom Lane
Andres Freund writes: > On Friday, March 16, 2012 02:50:55 PM Tom Lane wrote: >> While I'm looking at the grammar ... it also seems like a serious >> PITA from a maintenance standpoint that we're now going to have to >> adjust the CREATE COMMAND TRIGGER productions every time somebody >> thinks of

Re: [HACKERS] Command Triggers, v16

2012-03-16 Thread Andres Freund
On Friday, March 16, 2012 02:50:55 PM Tom Lane wrote: > While I'm looking at the grammar ... it also seems like a serious > PITA from a maintenance standpoint that we're now going to have to > adjust the CREATE COMMAND TRIGGER productions every time somebody > thinks of a new SQL command. I don't

Re: [HACKERS] Command Triggers, v16

2012-03-16 Thread Tom Lane
Andres Freund writes: > On Thursday, March 15, 2012 10:58:49 PM Dimitri Fontaine wrote: >> I tricked that in the grammar, the type is called cmdtrigger but I >> though it wouldn't be a good choice for the SQL statement. > Hm. I am decidedly unhappy with that grammar hackery... But then maybe I am

Re: [HACKERS] Command Triggers, v16

2012-03-16 Thread Thom Brown
On 16 March 2012 12:07, Dimitri Fontaine wrote: > Thom Brown writes: >> Okay, I shalln't do any more testing until the next patch.  I should >> probably have worked on automating my tests more, but never got round >> to it. > >  make installcheck :) > > That said, your test allow to spot OID prob

Re: [HACKERS] Command Triggers, v16

2012-03-16 Thread Dimitri Fontaine
Thom Brown writes: > Okay, I shalln't do any more testing until the next patch. I should > probably have worked on automating my tests more, but never got round > to it. make installcheck :) That said, your test allow to spot OID problems that we can't add in the regression tests (OID being t

  1   2   3   4   >