Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit7c31874

Browse files
committed
Avoid getting more than AccessShareLock when deparsing a query.
In make_ruledef and get_query_def, we have long used AcquireRewriteLocksto ensure that the querytree we are about to deparse is up-to-date andthe schemas of the underlying relations aren't changing. Howwever, thatfunction thinks the query is about to be executed, so it acquires locksthat are stronger than necessary for the purpose of deparsing. Thus forexample, if pg_dump asks to deparse a rule that includes "INSERT INTO t",we'd acquire RowExclusiveLock on t. That results in interference withconcurrent transactions that might for example ask for ShareLock on t.Since pg_dump is documented as being purely read-only, this is unexpected.(Worse, it used to actually be read-only; this behavior dates back onlyto 8.1, cf commitba42002.)Fix this by adding a parameter to AcquireRewriteLocks to tell it whetherwe want the "real" execution locks or only AccessShareLock.Report, diagnosis, and patch by Dean Rasheed. Back-patch to all supportedbranches.
1 parenta0c2fa9 commit7c31874

File tree

4 files changed

+57
-24
lines changed

4 files changed

+57
-24
lines changed

‎src/backend/commands/matview.c‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -303,7 +303,7 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
303303

304304
/* Lock and rewrite, using a copy to preserve the original query. */
305305
copied_query=copyObject(query);
306-
AcquireRewriteLocks(copied_query, false);
306+
AcquireRewriteLocks(copied_query,true,false);
307307
rewritten=QueryRewrite(copied_query);
308308

309309
/* SELECT should never rewrite to more or less than one SELECT query */

‎src/backend/rewrite/rewriteHandler.c‎

Lines changed: 48 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,13 @@ typedef struct rewrite_event
3737
CmdTypeevent;/* type of rule being fired */
3838
}rewrite_event;
3939

40-
staticboolacquireLocksOnSubLinks(Node*node,void*context);
40+
typedefstructacquireLocksOnSubLinks_context
41+
{
42+
boolfor_execute;/* AcquireRewriteLocks' forExecute param */
43+
}acquireLocksOnSubLinks_context;
44+
45+
staticboolacquireLocksOnSubLinks(Node*node,
46+
acquireLocksOnSubLinks_context*context);
4147
staticQuery*rewriteRuleAction(Query*parsetree,
4248
Query*rule_action,
4349
Node*rule_qual,
@@ -71,9 +77,19 @@ static Bitmapset *adjust_view_column_set(Bitmapset *cols, List *targetlist);
7177
* These locks will ensure that the relation schemas don't change under us
7278
* while we are rewriting and planning the query.
7379
*
74-
* forUpdatePushedDown indicates that a pushed-down FOR [KEY] UPDATE/SHARE applies
75-
* to the current subquery, requiring all rels to be opened with RowShareLock.
76-
* This should always be false at the start of the recursion.
80+
* forExecute indicates that the query is about to be executed.
81+
* If so, we'll acquire RowExclusiveLock on the query's resultRelation,
82+
* RowShareLock on any relation accessed FOR [KEY] UPDATE/SHARE, and
83+
* AccessShareLock on all other relations mentioned.
84+
*
85+
* If forExecute is false, AccessShareLock is acquired on all relations.
86+
* This case is suitable for ruleutils.c, for example, where we only need
87+
* schema stability and we don't intend to actually modify any relations.
88+
*
89+
* forUpdatePushedDown indicates that a pushed-down FOR [KEY] UPDATE/SHARE
90+
* applies to the current subquery, requiring all rels to be opened with at
91+
* least RowShareLock. This should always be false at the top of the
92+
* recursion. This flag is ignored if forExecute is false.
7793
*
7894
* A secondary purpose of this routine is to fix up JOIN RTE references to
7995
* dropped columns (see details below). Because the RTEs are modified in
@@ -101,10 +117,15 @@ static Bitmapset *adjust_view_column_set(Bitmapset *cols, List *targetlist);
101117
* construction of a nested join was O(N^2) in the nesting depth.)
102118
*/
103119
void
104-
AcquireRewriteLocks(Query*parsetree,boolforUpdatePushedDown)
120+
AcquireRewriteLocks(Query*parsetree,
121+
boolforExecute,
122+
boolforUpdatePushedDown)
105123
{
106124
ListCell*l;
107125
intrt_index;
126+
acquireLocksOnSubLinks_contextcontext;
127+
128+
context.for_execute=forExecute;
108129

109130
/*
110131
* First, process RTEs of the current query level.
@@ -130,14 +151,12 @@ AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown)
130151
* release it until end of transaction. This protects the
131152
* rewriter and planner against schema changes mid-query.
132153
*
133-
* If the relation is the query's result relation, then we
134-
* need RowExclusiveLock. Otherwise, check to see if the
135-
* relation is accessed FOR [KEY] UPDATE/SHARE or not.We
136-
* can't just grab AccessShareLock because then the executor
137-
* would be trying to upgrade the lock, leading to possible
138-
* deadlocks.
154+
* Assuming forExecute is true, this logic must match what the
155+
* executor will do, else we risk lock-upgrade deadlocks.
139156
*/
140-
if (rt_index==parsetree->resultRelation)
157+
if (!forExecute)
158+
lockmode=AccessShareLock;
159+
elseif (rt_index==parsetree->resultRelation)
141160
lockmode=RowExclusiveLock;
142161
elseif (forUpdatePushedDown||
143162
get_parse_rowmark(parsetree,rt_index)!=NULL)
@@ -225,6 +244,7 @@ AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown)
225244
* recurse to process the represented subquery.
226245
*/
227246
AcquireRewriteLocks(rte->subquery,
247+
forExecute,
228248
(forUpdatePushedDown||
229249
get_parse_rowmark(parsetree,rt_index)!=NULL));
230250
break;
@@ -240,23 +260,23 @@ AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown)
240260
{
241261
CommonTableExpr*cte= (CommonTableExpr*)lfirst(l);
242262

243-
AcquireRewriteLocks((Query*)cte->ctequery, false);
263+
AcquireRewriteLocks((Query*)cte->ctequery,forExecute,false);
244264
}
245265

246266
/*
247267
* Recurse into sublink subqueries, too. But we already did the ones in
248268
* the rtable and cteList.
249269
*/
250270
if (parsetree->hasSubLinks)
251-
query_tree_walker(parsetree,acquireLocksOnSubLinks,NULL,
271+
query_tree_walker(parsetree,acquireLocksOnSubLinks,&context,
252272
QTW_IGNORE_RC_SUBQUERIES);
253273
}
254274

255275
/*
256276
* Walker to find sublink subqueries for AcquireRewriteLocks
257277
*/
258278
staticbool
259-
acquireLocksOnSubLinks(Node*node,void*context)
279+
acquireLocksOnSubLinks(Node*node,acquireLocksOnSubLinks_context*context)
260280
{
261281
if (node==NULL)
262282
return false;
@@ -265,7 +285,9 @@ acquireLocksOnSubLinks(Node *node, void *context)
265285
SubLink*sub= (SubLink*)node;
266286

267287
/* Do what we came for */
268-
AcquireRewriteLocks((Query*)sub->subselect, false);
288+
AcquireRewriteLocks((Query*)sub->subselect,
289+
context->for_execute,
290+
false);
269291
/* Fall through to process lefthand args of SubLink */
270292
}
271293

@@ -307,6 +329,9 @@ rewriteRuleAction(Query *parsetree,
307329
intrt_length;
308330
Query*sub_action;
309331
Query**sub_action_ptr;
332+
acquireLocksOnSubLinks_contextcontext;
333+
334+
context.for_execute= true;
310335

311336
/*
312337
* Make modifiable copies of rule action and qual (what we're passed are
@@ -318,8 +343,8 @@ rewriteRuleAction(Query *parsetree,
318343
/*
319344
* Acquire necessary locks and fix any deleted JOIN RTE entries.
320345
*/
321-
AcquireRewriteLocks(rule_action, false);
322-
(void)acquireLocksOnSubLinks(rule_qual,NULL);
346+
AcquireRewriteLocks(rule_action,true,false);
347+
(void)acquireLocksOnSubLinks(rule_qual,&context);
323348

324349
current_varno=rt_index;
325350
rt_length=list_length(parsetree->rtable);
@@ -1389,7 +1414,7 @@ ApplyRetrieveRule(Query *parsetree,
13891414
*/
13901415
rule_action=copyObject(linitial(rule->actions));
13911416

1392-
AcquireRewriteLocks(rule_action,forUpdatePushedDown);
1417+
AcquireRewriteLocks(rule_action,true,forUpdatePushedDown);
13931418

13941419
/*
13951420
* Recursively expand any view references inside the view.
@@ -1712,14 +1737,17 @@ CopyAndAddInvertedQual(Query *parsetree,
17121737
{
17131738
/* Don't scribble on the passed qual (it's in the relcache!) */
17141739
Node*new_qual= (Node*)copyObject(rule_qual);
1740+
acquireLocksOnSubLinks_contextcontext;
1741+
1742+
context.for_execute= true;
17151743

17161744
/*
17171745
* In case there are subqueries in the qual, acquire necessary locks and
17181746
* fix any deleted JOIN RTE entries. (This is somewhat redundant with
17191747
* rewriteRuleAction, but not entirely ... consider restructuring so that
17201748
* we only need to process the qual this way once.)
17211749
*/
1722-
(void)acquireLocksOnSubLinks(new_qual,NULL);
1750+
(void)acquireLocksOnSubLinks(new_qual,&context);
17231751

17241752
/* Fix references to OLD */
17251753
ChangeVarNodes(new_qual,PRS2_OLD_VARNO,rt_index,0);

‎src/backend/utils/adt/ruleutils.c‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3966,7 +3966,7 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
39663966
query=getInsertSelectQuery(query,NULL);
39673967

39683968
/* Must acquire locks right away; see notes in get_query_def() */
3969-
AcquireRewriteLocks(query, false);
3969+
AcquireRewriteLocks(query, false, false);
39703970

39713971
context.buf=buf;
39723972
context.namespaces=list_make1(&dpns);
@@ -4108,8 +4108,11 @@ get_query_def(Query *query, StringInfo buf, List *parentnamespace,
41084108
* relations, and fix up deleted columns in JOIN RTEs.This ensures
41094109
* consistent results.Note we assume it's OK to scribble on the passed
41104110
* querytree!
4111+
*
4112+
* We are only deparsing the query (we are not about to execute it), so we
4113+
* only need AccessShareLock on the relations it mentions.
41114114
*/
4112-
AcquireRewriteLocks(query, false);
4115+
AcquireRewriteLocks(query, false, false);
41134116

41144117
context.buf=buf;
41154118
context.namespaces=lcons(&dpns,list_copy(parentnamespace));

‎src/include/rewrite/rewriteHandler.h‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818
#include"nodes/parsenodes.h"
1919

2020
externList*QueryRewrite(Query*parsetree);
21-
externvoidAcquireRewriteLocks(Query*parsetree,boolforUpdatePushedDown);
21+
externvoidAcquireRewriteLocks(Query*parsetree,
22+
boolforExecute,
23+
boolforUpdatePushedDown);
2224

2325
externNode*build_column_default(Relationrel,intattrno);
2426
externQuery*get_view_query(Relationview);

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp