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 t...@sss.pgh.pa.us wrote: Dean Rasheed dean.a.rash...@gmail.com 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

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 t...@sss.pgh.pa.us wrote: Dean Rasheed dean.a.rash...@gmail.com 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

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

2012-12-09 Thread Tom Lane
Thom Brown t...@linux.com 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

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 t...@sss.pgh.pa.us wrote: Thom Brown t...@linux.com 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

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

2012-12-09 Thread Tom Lane
Dean Rasheed dean.a.rash...@gmail.com 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

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 t...@sss.pgh.pa.us wrote: Dean Rasheed dean.a.rash...@gmail.com 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

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 t...@sss.pgh.pa.us wrote: Dean Rasheed dean.a.rash...@gmail.com 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

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

2012-12-09 Thread Tom Lane
Simon Riggs si...@2ndquadrant.com writes: On 9 December 2012 22:00, Tom Lane t...@sss.pgh.pa.us 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

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?

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 t...@sss.pgh.pa.us 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

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

2012-12-08 Thread Tom Lane
Dean Rasheed dean.a.rash...@gmail.com 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:

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 dean.a.rash...@gmail.com writes: On 8 November 2012 21:13, Tom Lane t...@sss.pgh.pa.us wrote: I think the most reasonable backwards-compatibility argument is that we shouldn't change the behavior if there are either

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 t...@sss.pgh.pa.us wrote: Dean Rasheed dean.a.rash...@gmail.com writes: On 7 November 2012 22:04, Tom Lane t...@sss.pgh.pa.us wrote: This seems to me to be dangerous and unintuitive, not to mention underdocumented. I think it would be better to just not do

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 dean.a.rash...@gmail.com 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

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 dean.a.rash...@gmail.com wrote: On 8 November 2012 08:33, Dean Rasheed dean.a.rash...@gmail.com 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...

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 da...@fetter.org 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

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

2012-11-08 Thread Tom Lane
David Fetter da...@fetter.org 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

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 da...@fetter.org 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

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

2012-11-08 Thread Tom Lane
Dean Rasheed dean.a.rash...@gmail.com writes: On 8 November 2012 14:38, Dean Rasheed dean.a.rash...@gmail.com 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

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

2012-11-08 Thread Tom Lane
Dean Rasheed dean.a.rash...@gmail.com 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

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 t...@sss.pgh.pa.us wrote: Dean Rasheed dean.a.rash...@gmail.com 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

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

2012-11-08 Thread Tom Lane
Dean Rasheed dean.a.rash...@gmail.com writes: On 8 November 2012 17:37, Tom Lane t...@sss.pgh.pa.us 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

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 t...@sss.pgh.pa.us wrote: Dean Rasheed dean.a.rash...@gmail.com writes: On 8 November 2012 17:37, Tom Lane t...@sss.pgh.pa.us wrote: I believe that the right way to think about the auto-update transformation is that it should act like a supplied-by-default

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

2012-11-08 Thread Tom Lane
Dean Rasheed dean.a.rash...@gmail.com 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

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 t...@sss.pgh.pa.us wrote: Dean Rasheed dean.a.rash...@gmail.com 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 *

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

2012-11-07 Thread Tom Lane
Dean Rasheed dean.a.rash...@gmail.com 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

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 dean.a.rash...@gmail.com 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

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

2012-11-07 Thread Tom Lane
David Fetter da...@fetter.org 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

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 t...@sss.pgh.pa.us wrote: Dean Rasheed dean.a.rash...@gmail.com 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

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

2012-11-07 Thread Tom Lane
Dean Rasheed dean.a.rash...@gmail.com writes: On 7 November 2012 22:04, Tom Lane t...@sss.pgh.pa.us 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

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 pete...@gmx.net wrote: Compiler warning needs to be fixed: rewriteHandler.c: In function 'RewriteQuery': rewriteHandler.c:2153:29: error: 'view_rte' may be used uninitialized in

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 amit.kap...@huawei.com wrote: On Sunday, September 23, 2012 12:33 AM Dean Rasheed wrote: On 18 September 2012 14:23, Amit kapila amit.kap...@huawei.com wrote: Please find the review of the patch. Thanks for the review. Attached is an updated patch,

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-09-24 Thread Amit Kapila
On Sunday, September 23, 2012 12:33 AM Dean Rasheed wrote: On 18 September 2012 14:23, Amit kapila amit.kap...@huawei.com 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.

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 amit.kap...@huawei.com 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

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 dean(dot)a(dot)rasheed(at)gmail(dot)com wrote: On 30 August 2012 20:05, Robert Haas robertmhaas(at)gmail(dot)com wrote: On Sun, Aug 12, 2012 at 5:14 PM, Dean Rasheed dean(dot)a(dot)rasheed(at)gmail(dot)com wrote: Here's an updated patch for the base