Andres Freund <and...@2ndquadrant.com> wrote:
> On 2013-11-02 17:05:24 -0700, Kevin Grittner wrote:
>> Andres Freund <and...@2ndquadrant.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
diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index d5a10ad..d5e58ae 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -30,6 +30,7 @@
 #include "miscadmin.h"
 #include "parser/parse_relation.h"
 #include "rewrite/rewriteHandler.h"
+#include "storage/lmgr.h"
 #include "storage/smgr.h"
 #include "tcop/tcopprot.h"
 #include "utils/builtins.h"
@@ -243,6 +244,7 @@ ExecRefreshMatView(RefreshMatViewStmt *stmt, const char *queryString,
 	/* Create the transient table that will receive the regenerated data. */
 	OIDNewHeap = make_new_heap(matviewOid, tableSpace, concurrent,
 							   ExclusiveLock);
+	LockRelationOid(OIDNewHeap, AccessExclusiveLock);
 	dest = CreateTransientRelDestReceiver(OIDNewHeap);
 
 	/* Generate the data, if wanted. */
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to