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

On 2013-10-23 20:51:27 +0100, Dean Rasheed wrote:
> On 23 October 2013 02:18, Andres Freund <andres@2ndquadrant.com> 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 really doubt it's the
> > correct approach, I don't really know enough about that area of the
> > code.
> > This looks like something that needs to be fixed.
> >
> 
> Hmm, my first thought is that rewriteTargetView() should be calling
> AcquireRewriteLocks() on viewquery, before doing too much with it.
> There may be sub-queries in viewquery's quals (and also now in its
> targetlist) and I don't think the relations referred to by those
> sub-queries are getting locked.

Well, that wouldn't follow the currently documented rule ontop
of QueryRewrite:* NOTE: the parsetree must either have come straight from the parser,* or have been scanned by
AcquireRewriteLocksto acquire suitable locks.
 

It might still be the right thing to do, but it seems suspicious that
the rules need to be tweaked like that.

> I think that any code that is doing anything significant with a rule
> action's query needs to think about locking the query's relations. I
> did a quick search and the only suspicious code I found was the
> matview and auto-updatable view code.

Yea, that were the locations the debugging patch cried on...

Greetings,

Andres Freund

-- Andres Freund                       http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training &
Services