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

On 2013-11-02 17:05:24 -0700, Kevin Grittner wrote:
> Andres Freund <andres(at)2ndquadrant(dot)com> 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 created temporary table in the matview code which shouldn't
> > be required for correctness but gives warm and fuzzy feelings as
> > well as less debugging noise.
>
> Will think about this.  I agree is is probably worth doing
> something to reduce the noise when looking for cases that actually
> matter.

It's pretty much free, so I don't think there really is any reason to
deviate from other parts of the code. Note how e.g. copy_heap_data(),
DefineRelation() and ATRewriteTable() all lock the new relation, even if
it just has been created and is (and in the latter two cases will always
be) invisible.

> > Wouldn't it be a good idea to tack such WARNINGs (in a proper and
> > clean form) to index_open (checking the underlying relation is
> > locked), relation_open(..., NoLock) (checking the relation has
> > previously been locked) and maybe RelationIdGetRelation() when
> > cassert is enabled? ISTM we frequently had bugs around this.
>
> It would be nice to have such omissions pointed out during early
> testing.

Will try to come up with something.

Greetings,

Andres Freund

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