Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2015-09-08 Thread Stephen Frost
* Andres Freund (and...@anarazel.de) wrote: > On 2015-08-27 18:37:10 +0100, Dean Rasheed wrote: > > Yes, I concur. It now needs an acquireLocksOnSubLinks_context with > > for_execute = true, but otherwise it should work. > > Ok, I'll push that then, unless somebody else wants to do the honors. Do

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2015-08-27 Thread Dean Rasheed
On 27 August 2015 at 19:29, Andres Freund wrote: > On 2015-08-27 19:19:35 +0100, Dean Rasheed wrote: >> It also seems to me that this warning has proved its worth > > Same here - I plan to re-submit it. Perhaps the number of bugs it found > convinces Tom, after I address some of his points. > >> a

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2015-08-27 Thread Andres Freund
On 2015-08-27 19:19:35 +0100, Dean Rasheed wrote: > It also seems to me that this warning has proved its worth Same here - I plan to re-submit it. Perhaps the number of bugs it found convinces Tom, after I address some of his points. > although I don't think it's something a production build shou

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2015-08-27 Thread Dean Rasheed
On 27 August 2015 at 18:44, Andres Freund wrote: > On 2015-08-27 18:37:10 +0100, Dean Rasheed wrote: >> I have a feeling that RLS might suffer from the same issue, but I >> haven't looked yet. > > There's a similar issue there, yes: > http://archives.postgresql.org/message-id/20150827124931.GD159

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2015-08-27 Thread Andres Freund
On 2015-08-27 18:37:10 +0100, Dean Rasheed wrote: > On 27 August 2015 at 12:18, Andres Freund wrote: > >> > >> /* > >> + * If the view query contains any sublink subqueries, we should also > >> + * acquire locks on any relations they refer to. We know that there > >> won't > >> +

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2015-08-27 Thread Dean Rasheed
On 27 August 2015 at 12:18, Andres Freund wrote: > On 2013-10-24 20:58:55 +0100, Dean Rasheed wrote: >> So I >> think the only thing missing from rewriteTargetView() is to lock any >> relations from any sublink subqueries in viewquery using >> acquireLocksOnSubLinks() -- patch attached. > > This i

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2015-08-27 Thread Andres Freund
On 2013-10-24 20:58:55 +0100, Dean Rasheed wrote: > On 24 October 2013 18:28, Andres Freund wrote: > > On 2013-10-23 21:20:58 +0100, Dean Rasheed wrote: > >> On 23 October 2013 21:08, Andres Freund wrote: > >> > On 2013-10-23 20:51:27 +0100, Dean Rasheed wrote: > >> >> Hmm, my first thought is th

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2013-11-06 Thread Kevin Grittner
Andres Freund > Looks fine to me Any thoughts on whether this should be back-patched to 9.3?  I can see arguments both ways, and don't have a particularly strong feeling one way or the other. -- Kevin Grittner EDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgs

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2013-11-05 Thread Kevin Grittner
Andres Freund wrote: > On 2013-11-05 12:21:23 -0800, Kevin Grittner wrote: >> Andres Freund >> >>> Looks fine to me >> >> Any thoughts on whether this should be back-patched to 9.3?  I >> can see arguments both ways, and don't have a particularly >> strong feeling one way or the other. > > Hehe.

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2013-11-05 Thread Andres Freund
On 2013-11-05 12:21:23 -0800, Kevin Grittner wrote: > Andres Freund > > > Looks fine to me > > Any thoughts on whether this should be back-patched to 9.3?  I can > see arguments both ways, and don't have a particularly strong > feeling one way or the other. Hehe. I was wondering myself. I'd ten

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2013-11-05 Thread Andres Freund
On 2013-11-05 11:56:25 -0800, Kevin Grittner wrote: > Andres Freund wrote: > > On 2013-11-02 17:05:24 -0700, Kevin Grittner wrote: > >> Andres Freund wrote: > > >>> Also attached is 0004 which just adds a heap_lock() around a > >>> newly created temporary table in the matview code which > >>> sh

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2013-11-05 Thread Kevin Grittner
Andres Freund wrote: > On 2013-11-02 17:05:24 -0700, Kevin Grittner wrote: >> Andres Freund wrote: >>> Also attached is 0004 which just adds a heap_lock() around a >>> newly created temporary table in the matview code which >>> shouldn't be required for correctness but gives warm and fuzzy >>> f

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2013-11-04 Thread Andres Freund
On 2013-11-02 17:05:24 -0700, Kevin Grittner wrote: > Andres Freund wrote: > > > the matview patch (0002) > > This is definitely needed as a bug fix.  Will adjust comments and > commit, back-patched to 9.3. Thanks. > > Also attached is 0004 which just adds a heap_lock() around a > > newly crea

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2013-11-02 Thread Kevin Grittner
Andres Freund wrote: > the matview patch (0002) This is definitely needed as a bug fix.  Will adjust comments and commit, back-patched to 9.3. Thanks for spotting this, and thanks for the fix! > Also attached is 0004 which just adds a heap_lock() around a > newly created temporary table in the

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2013-10-24 Thread Dean Rasheed
On 24 October 2013 18:28, Andres Freund wrote: > On 2013-10-23 21:20:58 +0100, Dean Rasheed wrote: >> On 23 October 2013 21:08, Andres Freund wrote: >> > On 2013-10-23 20:51:27 +0100, Dean Rasheed wrote: >> >> Hmm, my first thought is that rewriteTargetView() should be calling >> >> AcquireRewrit

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2013-10-24 Thread Andres Freund
On 2013-10-23 21:20:58 +0100, Dean Rasheed wrote: > On 23 October 2013 21:08, Andres Freund wrote: > > On 2013-10-23 20:51:27 +0100, Dean Rasheed wrote: > >> Hmm, my first thought is that rewriteTargetView() should be calling > >> AcquireRewriteLocks() on viewquery, before doing too much with it.

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2013-10-23 Thread Dean Rasheed
On 23 October 2013 21:08, Andres Freund wrote: > On 2013-10-23 20:51:27 +0100, Dean Rasheed wrote: >> On 23 October 2013 02:18, Andres Freund wrote: >> > Hi, >> > >> > Using the same debugging hack^Wpatch (0001) as in the matview patch >> > (0002) an hour or so ago I noticed that INSERT INTO view

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2013-10-23 Thread Andres Freund
On 2013-10-23 20:51:27 +0100, Dean Rasheed wrote: > On 23 October 2013 02:18, Andres Freund wrote: > > Hi, > > > > Using the same debugging hack^Wpatch (0001) as in the matview patch > > (0002) an hour or so ago I noticed that INSERT INTO view WITH CHECK > > doesn't lock the underlying relations p

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2013-10-23 Thread Dean Rasheed
On 23 October 2013 02:18, Andres Freund wrote: > Hi, > > Using the same debugging hack^Wpatch (0001) as in the matview patch > (0002) an hour or so ago I noticed that INSERT INTO view WITH CHECK > doesn't lock the underlying relations properly. > > I've attached a sort-of-working (0003) hack but I

Re: [HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2013-10-22 Thread Andres Freund
On 2013-10-23 03:18:55 +0200, Andres Freund wrote: > (000[1-4]) Attached. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Training & Services >From bf329af4eb6d839ae2a75c4f8a2d6867877510f4 Mon Sep 17 00:00:00 200

[HACKERS] missing locking in at least INSERT INTO view WITH CHECK

2013-10-22 Thread Andres Freund
Hi, Using the same debugging hack^Wpatch (0001) as in the matview patch (0002) an hour or so ago I noticed that INSERT INTO view WITH CHECK doesn't lock the underlying relations properly. I've attached a sort-of-working (0003) hack but I really doubt it's the correct approach, I don't really know