Hi,

In an postgres build that has added instrumentation to detect cases
where indexes are index_open()ed without any locks on the underlying
relation, the matview code started to cry during the regression tests.

The problem seems to be that refresh_matview_datafill() uses
QueryRewrite() without previously using AcquireRewriteLocks() but as
QueryRewrite states:
 * ...
 * NOTE: the parsetree must either have come straight from the parser,
 * or have been scanned by AcquireRewriteLocks to acquire suitable locks.
 */

I think the missed locks could lead to postgres crashing under
concurrent DDL.

I've attached a quick patch that fixes the issue for me in HEAD, but I
am not 100% sure I understand the details of refresh_matview_datafill().

This probably needs to go into 9.3 in an adapted form.

Greetings,

Andres Freund

-- 
 Andres Freund                     http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services
>From ae34a1b4d59627360131bb795da75326deea40b3 Mon Sep 17 00:00:00 2001
From: Andres Freund <and...@anarazel.de>
Date: Wed, 23 Oct 2013 01:25:49 +0200
Subject: [PATCH] Acquire appropriate locks when rewriting during REFRESH
 MATERIALIZED VIEW

---
 src/backend/commands/matview.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/backend/commands/matview.c b/src/backend/commands/matview.c
index fcfc678..1c02eab 100644
--- a/src/backend/commands/matview.c
+++ b/src/backend/commands/matview.c
@@ -283,6 +283,7 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 	Oid			save_userid;
 	int			save_sec_context;
 	int			save_nestlevel;
+	Query	   *copied_query;
 
 	/*
 	 * Switch to the owner's userid, so that any functions are run as that
@@ -294,8 +295,14 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
 						   save_sec_context | SECURITY_RESTRICTED_OPERATION);
 	save_nestlevel = NewGUCNestLevel();
 
-	/* Rewrite, copying the given Query to make sure it's not changed */
-	rewritten = QueryRewrite((Query *) copyObject(query));
+	/* copy the given Query to make sure it's not changed */
+	copied_query = copyObject(query);
+
+	/* acquire locks for rewriting */
+	AcquireRewriteLocks(copied_query, false);
+
+	/* Rewrite */
+	rewritten = QueryRewrite(copied_query);
 
 	/* SELECT should never rewrite to more or less than one SELECT query */
 	if (list_length(rewritten) != 1)
-- 
1.8.3.251.g1462b67

-- 
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