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(at)2ndquadrant(dot)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 AcquireRewriteLocks to 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