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

Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> On 2013-11-02 17:05:24 -0700, Kevin Grittner wrote:
>> Andres Freund <andres(at)2ndquadrant(dot)com> 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
>>> 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.

None of those locations are using heap_open() as the first
parameter to heap_close().  That looks kinda iffy, and the fact
that it is not yet done anywhere in the code gives me pause.  You
probably had a reason for preferring that to a simple call to
LockRelationOid(), but I'm not seeing what that reason is.  I also
don't understand the use of the lockmode variable here.

I'm thinking of something like the attached instead.

--
Kevin Grittner
EDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company