Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-12-09 Thread Tom Lane
Simon Riggs writes: > On 9 December 2012 22:00, Tom Lane wrote: >> But in any case, those functions are expensive enough that I can't see >> running them against every view in the DB anytime somebody hits tab. >> I think just allowing tab-completion to include all views is probably >> the best co

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-12-09 Thread Simon Riggs
On 9 December 2012 22:00, Tom Lane wrote: > Dean Rasheed writes: >> It's a shame though that pg_view_is_updatable() and >> pg_view_is_insertable() are not really useful for identifying >> potentially updatable views (e.g., consider an auto-updatable view on >> top of a trigger-updatable view). I'

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-12-09 Thread Dean Rasheed
On 9 December 2012 22:00, Tom Lane wrote: > Dean Rasheed writes: >> It's a shame though that pg_view_is_updatable() and >> pg_view_is_insertable() are not really useful for identifying >> potentially updatable views (e.g., consider an auto-updatable view on >> top of a trigger-updatable view). I'

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-12-09 Thread Tom Lane
Dean Rasheed writes: > It's a shame though that pg_view_is_updatable() and > pg_view_is_insertable() are not really useful for identifying > potentially updatable views (e.g., consider an auto-updatable view on > top of a trigger-updatable view). I'm left wondering if I > misinterpreted the SQL st

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-12-09 Thread Dean Rasheed
On 9 December 2012 16:53, Tom Lane wrote: > Thom Brown writes: >> One observation: There doesn't appear to be any tab-completion for view >> names after DML statement keywords in psql. Might we want to add this? > > Well, there is, but it only knows about INSTEAD OF trigger cases. > I'm tempted

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-12-09 Thread Tom Lane
Thom Brown writes: > One observation: There doesn't appear to be any tab-completion for view > names after DML statement keywords in psql. Might we want to add this? Well, there is, but it only knows about INSTEAD OF trigger cases. I'm tempted to suggest that Query_for_list_of_insertables and f

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-12-09 Thread Thom Brown
On 8 December 2012 23:29, Tom Lane wrote: > Dean Rasheed writes: > > Attached is a rebased patch using new OIDs. > > Applied after a fair amount of hacking. > One observation: There doesn't appear to be any tab-completion for view names after DML statement keywords in psql. Might we want to a

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-12-09 Thread Dean Rasheed
On 8 December 2012 23:29, Tom Lane wrote: > Dean Rasheed writes: >> Attached is a rebased patch using new OIDs. > > Applied after a fair amount of hacking. > Awesome! Thank you very much. Regards, Dean -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to y

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-12-08 Thread Tom Lane
Dean Rasheed writes: > Attached is a rebased patch using new OIDs. Applied after a fair amount of hacking. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/p

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-12-08 Thread Dean Rasheed
On 8 December 2012 15:21, Tom Lane wrote: > Continuing to work on this ... I found multiple things I didn't like > about the permission-field update code. Attached are some heavily > commented extracts from the code as I've changed it. Does anybody > object to either the code or the objectives g

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-12-08 Thread Tom Lane
Continuing to work on this ... I found multiple things I didn't like about the permission-field update code. Attached are some heavily commented extracts from the code as I've changed it. Does anybody object to either the code or the objectives given in the comments? rega

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-12-07 Thread Tom Lane
[ finally getting back to this patch, after many distractions ] Dean Rasheed writes: > On 8 November 2012 21:13, Tom Lane wrote: >> I think the most reasonable backwards-compatibility argument is that we >> shouldn't change the behavior if there are either INSTEAD rules or >> INSTEAD triggers.

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-11-08 Thread Dean Rasheed
On 8 November 2012 21:13, Tom Lane wrote: > Dean Rasheed writes: >> create table bar(a int); >> create view bar_v as select * from bar; >> create rule bar_r as on insert to bar_v where new.a < 0 do instead nothing; >> insert into bar_v values(-1),(1); >> select * from bar_v; >> a >> --- >> 1 >>

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-11-08 Thread Tom Lane
Dean Rasheed writes: > create table bar(a int); > create view bar_v as select * from bar; > create rule bar_r as on insert to bar_v where new.a < 0 do instead nothing; > insert into bar_v values(-1),(1); > select * from bar_v; > a > --- > 1 > (1 row) > Having that put both -1 and 1 into bar see

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-11-08 Thread Dean Rasheed
On 8 November 2012 19:29, Tom Lane wrote: > Dean Rasheed writes: >> On 8 November 2012 17:37, Tom Lane wrote: >>> I believe that the right way to think about the auto-update >>> transformation is that it should act like a supplied-by-default >>> unconditional INSTEAD rule. > >> But if you treat

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-11-08 Thread Tom Lane
Dean Rasheed writes: > On 8 November 2012 17:37, Tom Lane wrote: >> I believe that the right way to think about the auto-update >> transformation is that it should act like a supplied-by-default >> unconditional INSTEAD rule. > But if you treat the auto-update transformation as a > supplied-by-d

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-11-08 Thread Dean Rasheed
On 8 November 2012 17:37, Tom Lane wrote: > Dean Rasheed writes: >> If we did nothing here then it would go on to either fire any INSTEAD >> OF triggers or raise an error if there aren't any. The problem with >> that is that it makes trigger-updatable views and auto-updatable views >> inconsisten

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-11-08 Thread Tom Lane
Dean Rasheed writes: > If we did nothing here then it would go on to either fire any INSTEAD > OF triggers or raise an error if there aren't any. The problem with > that is that it makes trigger-updatable views and auto-updatable views > inconsistent in their behaviour with qualified INSTEAD rules

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-11-08 Thread Tom Lane
Dean Rasheed writes: > On 8 November 2012 14:38, Dean Rasheed wrote: >> Oh wait, that's nonsense (not enough caffeine). The rewrite code needs >> to know whether there are INSTEAD OF triggers before it decides >> whether it's going to substitute the base relation. The fundamental >> problem is th

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-11-08 Thread David Fetter
On Thu, Nov 08, 2012 at 11:33:47AM -0500, Tom Lane wrote: > David Fetter writes: > > There are three different WITH CHECK OPTION options: > > > WITH CHECK OPTION > > WITH CASCADED CHECK OPTION > > WITH LOCAL CHECK OPTION > > No, there are four: the fourth case being if you leave off the phrase >

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-11-08 Thread Tom Lane
David Fetter writes: > There are three different WITH CHECK OPTION options: > WITH CHECK OPTION > WITH CASCADED CHECK OPTION > WITH LOCAL CHECK OPTION No, there are four: the fourth case being if you leave off the phrase altogether. That's the only case we accept, and it corresponds to the patc

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-11-08 Thread David Fetter
On Wed, Nov 07, 2012 at 05:55:32PM -0500, Tom Lane wrote: > David Fetter writes: > > On Wed, Nov 07, 2012 at 05:04:48PM -0500, Tom Lane wrote: > >> Should we be doing something > >> about such cases, or is playing dumb correct? > > > The SQL standard handles deciding the behavior based on whether

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-11-08 Thread Dean Rasheed
On 8 November 2012 14:38, Dean Rasheed wrote: > On 8 November 2012 08:33, Dean Rasheed wrote: >> OK, yes I think we do need to be throwing the error at runtime rather >> than at plan time. That's pretty easy if we just keep the current >> error message... > > Oh wait, that's nonsense (not enough

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-11-08 Thread Dean Rasheed
On 8 November 2012 08:33, Dean Rasheed wrote: > OK, yes I think we do need to be throwing the error at runtime rather > than at plan time. That's pretty easy if we just keep the current > error message... Oh wait, that's nonsense (not enough caffeine). The rewrite code needs to know whether there

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-11-08 Thread Dean Rasheed
On 8 November 2012 03:10, Tom Lane wrote: > Dean Rasheed writes: >> On 7 November 2012 22:04, Tom Lane wrote: >>> This seems to me to be dangerous and unintuitive, not to mention >>> underdocumented. I think it would be better to just not do anything if >>> there is any INSTEAD rule, period. >

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-11-07 Thread Tom Lane
Dean Rasheed writes: > On 7 November 2012 22:04, Tom Lane wrote: >> This seems to me to be dangerous and unintuitive, not to mention >> underdocumented. I think it would be better to just not do anything if >> there is any INSTEAD rule, period. > Is this any more dangerous than what already hap

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-11-07 Thread Dean Rasheed
On 7 November 2012 22:04, Tom Lane wrote: > Dean Rasheed writes: >> Thanks for looking at this. >> Attached is a rebased patch using new OIDs. > > I'm starting to look at this patch now. I don't understand the intended > interaction with qualified INSTEAD rules. The code looks like > > +

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-11-07 Thread Tom Lane
David Fetter writes: > On Wed, Nov 07, 2012 at 05:04:48PM -0500, Tom Lane wrote: >> Should we be doing something >> about such cases, or is playing dumb correct? > The SQL standard handles deciding the behavior based on whether WITH > CHECK OPTION is included in the view DDL. See the section 2 o

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-11-07 Thread David Fetter
On Wed, Nov 07, 2012 at 05:04:48PM -0500, Tom Lane wrote: > Dean Rasheed writes: > > Thanks for looking at this. > > Attached is a rebased patch using new OIDs. > > I'm starting to look at this patch now. I don't understand the intended > interaction with qualified INSTEAD rules. The code looks

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-11-07 Thread Tom Lane
Dean Rasheed writes: > Thanks for looking at this. > Attached is a rebased patch using new OIDs. I'm starting to look at this patch now. I don't understand the intended interaction with qualified INSTEAD rules. The code looks like + if (!instead && rt_entry_relation->rd_rel->relk

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-10-11 Thread Dean Rasheed
Thanks for looking at this. Attached is a rebased patch using new OIDs. On 11 October 2012 02:39, Peter Eisentraut wrote: > Compiler warning needs to be fixed: > > rewriteHandler.c: In function 'RewriteQuery': > rewriteHandler.c:2153:29: error: 'view_rte' may be used uninitialized in this > func

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-10-10 Thread Peter Eisentraut
Compiler warning needs to be fixed: rewriteHandler.c: In function 'RewriteQuery': rewriteHandler.c:2153:29: error: 'view_rte' may be used uninitialized in this function [-Werror=maybe-uninitialized] rewriteHandler.c:2015:17: note: 'view_rte' was declared here Duplicate OIDs need to be adjusted:

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-10-10 Thread Thom Brown
On 24 September 2012 12:10, Amit Kapila wrote: > On Sunday, September 23, 2012 12:33 AM Dean Rasheed wrote: >> On 18 September 2012 14:23, Amit kapila wrote: >> > Please find the review of the patch. >> > >> >> Thanks for the review. Attached is an updated patch, and I've include >> some respon

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-09-24 Thread Amit Kapila
On Sunday, September 23, 2012 12:33 AM Dean Rasheed wrote: > On 18 September 2012 14:23, Amit kapila wrote: > > Please find the review of the patch. > > > > Thanks for the review. Attached is an updated patch, and I've include > some responses to specific review comments below. I have verified

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-09-22 Thread Dean Rasheed
On 18 September 2012 14:23, Amit kapila wrote: > Please find the review of the patch. > Thanks for the review. Attached is an updated patch, and I've include some responses to specific review comments below. > Extra test cases that can be added to regression suite are as below: > > 1. where cla

Re: [HACKERS] Proof of concept: auto updatable views [Review of Patch]

2012-09-18 Thread Amit kapila
On 31 August 2012 07:59, Dean Rasheed wrote: > On 30 August 2012 20:05, Robert Haas wrote: >> On Sun, Aug 12, 2012 at 5:14 PM, Dean Rasheed >> wrote: > Here's an updated patch for the base feature (without support for > security barrier views) with updated docs and regression tests. Please

Re: [HACKERS] Proof of concept: auto updatable views

2012-09-02 Thread Dean Rasheed
On 31 August 2012 07:59, Dean Rasheed wrote: > On 30 August 2012 20:05, Robert Haas wrote: >> On Sun, Aug 12, 2012 at 5:14 PM, Dean Rasheed >> wrote: >>> None of this new code kicks in for non-security barrier views, so the >>> kinds of plans I posted upthread remain unchanged in that case. But

Re: [HACKERS] Proof of concept: auto updatable views

2012-08-31 Thread Dean Rasheed
On 30 August 2012 20:05, Robert Haas wrote: > On Sun, Aug 12, 2012 at 5:14 PM, Dean Rasheed > wrote: >> None of this new code kicks in for non-security barrier views, so the >> kinds of plans I posted upthread remain unchanged in that case. But >> now a significant fraction of the patch is code

Re: [HACKERS] Proof of concept: auto updatable views

2012-08-30 Thread Robert Haas
On Sun, Aug 12, 2012 at 5:14 PM, Dean Rasheed wrote: > None of this new code kicks in for non-security barrier views, so the > kinds of plans I posted upthread remain unchanged in that case. But > now a significant fraction of the patch is code added to handle > security barrier views. Of course w

Re: [HACKERS] Proof of concept: auto updatable views

2012-08-27 Thread Dean Rasheed
On 27 August 2012 20:26, Dean Rasheed wrote: > Here's an updated WIP patch which I'll add to the next commitfest. Re-sending gzipped (apparently the mail system corrupted it last time). Regards, Dean auto-update-views.patch.gz Description: GNU Zip compressed data -- Sent via pgsql-hackers ma

Re: [HACKERS] Proof of concept: auto updatable views

2012-08-13 Thread Dean Rasheed
I've been looking at this further and I have made some improvements, but also found a problem. On 1 July 2012 23:35, Dean Rasheed wrote: > I'm also aware that my new function ChangeVarAttnos() is almost > identical to the function map_variable_attnos() that Tom recently > added, but I couldn't im

Re: [HACKERS] Proof of concept: auto updatable views

2012-07-02 Thread Dean Rasheed
On 2 July 2012 21:34, Tom Lane wrote: > Robert Haas writes: >> On Sun, Jul 1, 2012 at 6:35 PM, Dean Rasheed >> wrote: >>> Basically what it does is this: in the first stage of query rewriting, >>> just after any non-SELECT rules are applied, the new code kicks in - >>> if the target relation is

Re: [HACKERS] Proof of concept: auto updatable views

2012-07-02 Thread Tom Lane
Robert Haas writes: > On Sun, Jul 1, 2012 at 6:35 PM, Dean Rasheed wrote: >> Basically what it does is this: in the first stage of query rewriting, >> just after any non-SELECT rules are applied, the new code kicks in - >> if the target relation is a view, and there were no unqualified >> INSTEAD

Re: [HACKERS] Proof of concept: auto updatable views

2012-07-02 Thread Robert Haas
On Sun, Jul 1, 2012 at 6:35 PM, Dean Rasheed wrote: > I've been playing around with the idea of supporting automatically > updatable views, and I have a working proof of concept. I've taken a > different approach than the previous attempts to implement this > feature (e.g., > http://archives.post

Re: [HACKERS] Proof of concept: auto updatable views

2012-07-01 Thread Darren Duncan
My thoughts on this is that it would be a very valuable feature to have, and would make Postgres views behave more like they always were intended to behave, which is indistinguishible to users from tables in behavior where all possible, and that the reverse mapping would be automatic with the DB

[HACKERS] Proof of concept: auto updatable views

2012-07-01 Thread Dean Rasheed
Hi, I've been playing around with the idea of supporting automatically updatable views, and I have a working proof of concept. I've taken a different approach than the previous attempts to implement this feature (e.g., http://archives.postgresql.org/pgsql-hackers/2009-01/msg01746.php), instead do