Re: [HACKERS] WIP: Triggers on VIEWs

2010-10-11 Thread Dean Rasheed
On 10 October 2010 19:06, Tom Lane wrote: > Applied with revisions. Brilliant! Thank you very much. > * I took out this change in planmain.c: > > +       /* > +        * If the query target is a VIEW, it won't be in the jointree, but we > +        * need a dummy RelOptInfo node for it. This need

Re: [HACKERS] WIP: Triggers on VIEWs

2010-10-10 Thread Tom Lane
Bernd Helmle writes: > --On 8. September 2010 09:00:33 +0100 Dean Rasheed > wrote: >> Here's an updated version with improved formatting and a few minor >> wording changes to the triggers chapter. > This version doesn't apply clean anymore due to some rejects in > plainmain.c. Corrected versio

Re: [HACKERS] WIP: Triggers on VIEWs

2010-10-08 Thread Tom Lane
BTW, while I'm looking at this: it seems like the "index" arrays in struct TrigDesc are really a lot more complication than they are worth. It'd be far easier to dispense with them and instead iterate through the main trigger array, skipping any triggers whose tgtype doesn't match what we need. If

Re: [HACKERS] WIP: Triggers on VIEWs

2010-10-08 Thread Tom Lane
Robert Haas writes: > On Fri, Oct 8, 2010 at 11:50 AM, Tom Lane wrote: >> Although we already have macros TRIGGER_FIRED_AFTER/TRIGGER_FIRED_BEFORE >> that seem to mask the details here, the changes you had to make in >> contrib illustrate that the macros' callers could still be embedding this >>

Re: [HACKERS] WIP: Triggers on VIEWs

2010-10-08 Thread Robert Haas
On Fri, Oct 8, 2010 at 11:50 AM, Tom Lane wrote: > I think the > right thing here is to replace "before" with a three-valued enum, > perhaps called "timing", so as to force people to take another look > at any code that touches the field directly. +1. That seems much nicer. > Although we alread

Re: [HACKERS] WIP: Triggers on VIEWs

2010-10-08 Thread Tom Lane
Dean Rasheed writes: > On 8 October 2010 16:50, Tom Lane wrote: >> I've started looking at this patch now.  I think it would have been best >> submitted as two patches: one to add the SQL-spec "INSTEAD OF" trigger >> functionality, and a follow-on to extend INSTEAD OF triggers to views. > SQL-sp

Re: [HACKERS] WIP: Triggers on VIEWs

2010-10-08 Thread Dean Rasheed
On 8 October 2010 16:50, Tom Lane wrote: > Bernd Helmle writes: >> I would like to do some more tests/review, but going to mark this patch as >> "Ready for Committer", so that someone more qualified on the executor part >> can have a look on it during this commitfest, if that's okay for us? > > I

Re: [HACKERS] WIP: Triggers on VIEWs

2010-10-08 Thread Tom Lane
Bernd Helmle writes: > I would like to do some more tests/review, but going to mark this patch as > "Ready for Committer", so that someone more qualified on the executor part > can have a look on it during this commitfest, if that's okay for us? I've started looking at this patch now. I think

Re: [HACKERS] WIP: Triggers on VIEWs

2010-10-06 Thread Dean Rasheed
On 5 October 2010 21:17, Bernd Helmle wrote: > Basic summary of this patch: > Thanks for the review. > * The patch includes a fairly complete discussion about INSTEAD OF triggers > and their usage on views. There are also additional enhancements to the RULE > documentation, which seems, given th

Re: [HACKERS] WIP: Triggers on VIEWs

2010-10-05 Thread Bernd Helmle
--On 30. September 2010 07:38:18 +0100 Dean Rasheed wrote: This version doesn't apply clean anymore due to some rejects in plainmain.c. Corrected version attached. Ah yes, those pesky bits have been rotting while I wasn't looking. Thanks for fixing them! Basic summary of this patch: *

Re: [HACKERS] WIP: Triggers on VIEWs

2010-09-29 Thread Dean Rasheed
On 29 September 2010 20:18, Bernd Helmle wrote: > > > --On 8. September 2010 09:00:33 +0100 Dean Rasheed > wrote: > >> Here's an updated version with improved formatting and a few minor >> wording changes to the triggers chapter. > > This version doesn't apply clean anymore due to some rejects in

Re: [HACKERS] WIP: Triggers on VIEWs

2010-09-23 Thread Bernd Helmle
--On 23. September 2010 08:59:32 +0100 Dean Rasheed wrote: Yes, I agree. To me this is the least surprising behaviour. I think a more common case would be where the trigger computed a value (such as the 'last updated' example). The executor doesn't have any kind of a handle on the row inser

Re: [HACKERS] WIP: Triggers on VIEWs

2010-09-23 Thread Dean Rasheed
On 23 September 2010 00:26, Marko Tiikkaja wrote: > On 2010-09-23 1:16 AM, Bernd Helmle wrote: >> >> INSERT INTO vfoo VALUES('helmle', 2) RETURNING *; >>   text  | id >> + >>  helmle |  2 >> (1 row) >> >> SELECT * FROM vfoo; >>  text  | id >> ---+ >>  bernd |  2 >> (1 row) >> >

Re: [HACKERS] WIP: Triggers on VIEWs

2010-09-22 Thread Marko Tiikkaja
On 2010-09-23 1:16 AM, Bernd Helmle wrote: INSERT INTO vfoo VALUES('helmle', 2) RETURNING *; text | id + helmle | 2 (1 row) SELECT * FROM vfoo; text | id ---+ bernd | 2 (1 row) This is solvable by a properly designed trigger function, but maybe we need to do som

Re: [HACKERS] WIP: Triggers on VIEWs

2010-09-22 Thread Bernd Helmle
--On 5. September 2010 09:09:55 +0100 Dean Rasheed wrote: I had a first look on your patch, great work! Attached is an updated patch with more tests and docs, and a few minor code tidy ups. I think that the INSTEAD OF triggers part of the patch is compliant with Feature T213 of the SQL 200

Re: [HACKERS] WIP: Triggers on VIEWs

2010-09-07 Thread Dean Rasheed
On 7 September 2010 02:03, David Christensen wrote: > > On Sep 5, 2010, at 3:09 AM, Dean Rasheed wrote: > >> On 15 August 2010 18:38, Dean Rasheed wrote: >>> Here is a WIP patch implementing triggers on VIEWs ... >>> >>> There are still a number of things left todo: >>>  - extend ALTER VIEW with

Re: [HACKERS] WIP: Triggers on VIEWs

2010-09-06 Thread David Christensen
On Sep 5, 2010, at 3:09 AM, Dean Rasheed wrote: > On 15 August 2010 18:38, Dean Rasheed wrote: >> Here is a WIP patch implementing triggers on VIEWs ... >> >> There are still a number of things left todo: >> - extend ALTER VIEW with enable/disable trigger commands >> - much more testing >>

Re: [HACKERS] WIP: Triggers on VIEWs

2010-08-16 Thread Dean Rasheed
On 16 August 2010 18:50, Tom Lane wrote: > Dean Rasheed writes: >> On 15 August 2010 18:38, Dean Rasheed wrote: >>> There are still a number of things left todo: >>>  - extend ALTER VIEW with enable/disable trigger commands > >> On further reflection, I wonder if the ability to disable VIEW >> t

Re: [HACKERS] WIP: Triggers on VIEWs

2010-08-16 Thread Tom Lane
Dean Rasheed writes: > On 15 August 2010 18:38, Dean Rasheed wrote: >> There are still a number of things left todo: >>  - extend ALTER VIEW with enable/disable trigger commands > On further reflection, I wonder if the ability to disable VIEW > triggers is needed/wanted at all. AFAIK the only r

Re: [HACKERS] WIP: Triggers on VIEWs

2010-08-16 Thread Dean Rasheed
On 15 August 2010 18:38, Dean Rasheed wrote: > There are still a number of things left todo: >  - extend ALTER VIEW with enable/disable trigger commands On further reflection, I wonder if the ability to disable VIEW triggers is needed/wanted at all. I just noticed that while it is possible to dis