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

Commitbed1259

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

File tree

3 files changed

+56
-23
lines changed

3 files changed

+56
-23
lines changed

‎src/backend/rewrite/rewriteHandler.c

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

39-
staticboolacquireLocksOnSubLinks(Node*node,void*context);
39+
typedefstructacquireLocksOnSubLinks_context
40+
{
41+
boolfor_execute;/* AcquireRewriteLocks' forExecute param */
42+
}acquireLocksOnSubLinks_context;
43+
44+
staticboolacquireLocksOnSubLinks(Node*node,
45+
acquireLocksOnSubLinks_context*context);
4046
staticQuery*rewriteRuleAction(Query*parsetree,
4147
Query*rule_action,
4248
Node*rule_qual,
@@ -66,9 +72,19 @@ static Query *fireRIRrules(Query *parsetree, List *activeRIRs,
6672
* These locks will ensure that the relation schemas don't change under us
6773
* while we are rewriting and planning the query.
6874
*
69-
* forUpdatePushedDown indicates that a pushed-down FOR UPDATE/SHARE applies
70-
* to the current subquery, requiring all rels to be opened with RowShareLock.
71-
* This should always be false at the start of the recursion.
75+
* forExecute indicates that the query is about to be executed.
76+
* If so, we'll acquire RowExclusiveLock on the query's resultRelation,
77+
* RowShareLock on any relation accessed FOR UPDATE/SHARE, and
78+
* AccessShareLock on all other relations mentioned.
79+
*
80+
* If forExecute is false, AccessShareLock is acquired on all relations.
81+
* This case is suitable for ruleutils.c, for example, where we only need
82+
* schema stability and we don't intend to actually modify any relations.
83+
*
84+
* forUpdatePushedDown indicates that a pushed-down FOR UPDATE/SHARE
85+
* applies to the current subquery, requiring all rels to be opened with at
86+
* least RowShareLock. This should always be false at the top of the
87+
* recursion. This flag is ignored if forExecute is false.
7288
*
7389
* A secondary purpose of this routine is to fix up JOIN RTE references to
7490
* dropped columns (see details below). Because the RTEs are modified in
@@ -96,10 +112,15 @@ static Query *fireRIRrules(Query *parsetree, List *activeRIRs,
96112
* construction of a nested join was O(N^2) in the nesting depth.)
97113
*/
98114
void
99-
AcquireRewriteLocks(Query*parsetree,boolforUpdatePushedDown)
115+
AcquireRewriteLocks(Query*parsetree,
116+
boolforExecute,
117+
boolforUpdatePushedDown)
100118
{
101119
ListCell*l;
102120
intrt_index;
121+
acquireLocksOnSubLinks_contextcontext;
122+
123+
context.for_execute=forExecute;
103124

104125
/*
105126
* First, process RTEs of the current query level.
@@ -125,14 +146,12 @@ AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown)
125146
* release it until end of transaction. This protects the
126147
* rewriter and planner against schema changes mid-query.
127148
*
128-
* If the relation is the query's result relation, then we
129-
* need RowExclusiveLock. Otherwise, check to see if the
130-
* relation is accessed FOR UPDATE/SHARE or not. We can't
131-
* just grab AccessShareLock because then the executor would
132-
* be trying to upgrade the lock, leading to possible
133-
* deadlocks.
149+
* Assuming forExecute is true, this logic must match what the
150+
* executor will do, else we risk lock-upgrade deadlocks.
134151
*/
135-
if (rt_index==parsetree->resultRelation)
152+
if (!forExecute)
153+
lockmode=AccessShareLock;
154+
elseif (rt_index==parsetree->resultRelation)
136155
lockmode=RowExclusiveLock;
137156
elseif (forUpdatePushedDown||
138157
get_parse_rowmark(parsetree,rt_index)!=NULL)
@@ -213,6 +232,7 @@ AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown)
213232
* recurse to process the represented subquery.
214233
*/
215234
AcquireRewriteLocks(rte->subquery,
235+
forExecute,
216236
(forUpdatePushedDown||
217237
get_parse_rowmark(parsetree,rt_index)!=NULL));
218238
break;
@@ -228,23 +248,23 @@ AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown)
228248
{
229249
CommonTableExpr*cte= (CommonTableExpr*)lfirst(l);
230250

231-
AcquireRewriteLocks((Query*)cte->ctequery, false);
251+
AcquireRewriteLocks((Query*)cte->ctequery,forExecute,false);
232252
}
233253

234254
/*
235255
* Recurse into sublink subqueries, too. But we already did the ones in
236256
* the rtable and cteList.
237257
*/
238258
if (parsetree->hasSubLinks)
239-
query_tree_walker(parsetree,acquireLocksOnSubLinks,NULL,
259+
query_tree_walker(parsetree,acquireLocksOnSubLinks,&context,
240260
QTW_IGNORE_RC_SUBQUERIES);
241261
}
242262

243263
/*
244264
* Walker to find sublink subqueries for AcquireRewriteLocks
245265
*/
246266
staticbool
247-
acquireLocksOnSubLinks(Node*node,void*context)
267+
acquireLocksOnSubLinks(Node*node,acquireLocksOnSubLinks_context*context)
248268
{
249269
if (node==NULL)
250270
return false;
@@ -253,7 +273,9 @@ acquireLocksOnSubLinks(Node *node, void *context)
253273
SubLink*sub= (SubLink*)node;
254274

255275
/* Do what we came for */
256-
AcquireRewriteLocks((Query*)sub->subselect, false);
276+
AcquireRewriteLocks((Query*)sub->subselect,
277+
context->for_execute,
278+
false);
257279
/* Fall through to process lefthand args of SubLink */
258280
}
259281

@@ -295,6 +317,9 @@ rewriteRuleAction(Query *parsetree,
295317
intrt_length;
296318
Query*sub_action;
297319
Query**sub_action_ptr;
320+
acquireLocksOnSubLinks_contextcontext;
321+
322+
context.for_execute= true;
298323

299324
/*
300325
* Make modifiable copies of rule action and qual (what we're passed are
@@ -306,8 +331,8 @@ rewriteRuleAction(Query *parsetree,
306331
/*
307332
* Acquire necessary locks and fix any deleted JOIN RTE entries.
308333
*/
309-
AcquireRewriteLocks(rule_action, false);
310-
(void)acquireLocksOnSubLinks(rule_qual,NULL);
334+
AcquireRewriteLocks(rule_action,true,false);
335+
(void)acquireLocksOnSubLinks(rule_qual,&context);
311336

312337
current_varno=rt_index;
313338
rt_length=list_length(parsetree->rtable);
@@ -1170,7 +1195,7 @@ ApplyRetrieveRule(Query *parsetree,
11701195
*/
11711196
rule_action=copyObject(linitial(rule->actions));
11721197

1173-
AcquireRewriteLocks(rule_action,forUpdatePushedDown);
1198+
AcquireRewriteLocks(rule_action,true,forUpdatePushedDown);
11741199

11751200
/*
11761201
* Recursively expand any view references inside the view.
@@ -1479,14 +1504,17 @@ CopyAndAddInvertedQual(Query *parsetree,
14791504
{
14801505
/* Don't scribble on the passed qual (it's in the relcache!) */
14811506
Node*new_qual= (Node*)copyObject(rule_qual);
1507+
acquireLocksOnSubLinks_contextcontext;
1508+
1509+
context.for_execute= true;
14821510

14831511
/*
14841512
* In case there are subqueries in the qual, acquire necessary locks and
14851513
* fix any deleted JOIN RTE entries. (This is somewhat redundant with
14861514
* rewriteRuleAction, but not entirely ... consider restructuring so that
14871515
* we only need to process the qual this way once.)
14881516
*/
1489-
(void)acquireLocksOnSubLinks(new_qual,NULL);
1517+
(void)acquireLocksOnSubLinks(new_qual,&context);
14901518

14911519
/* Fix references to OLD */
14921520
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
@@ -2277,7 +2277,7 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
22772277
query=getInsertSelectQuery(query,NULL);
22782278

22792279
/* Must acquire locks right away; see notes in get_query_def() */
2280-
AcquireRewriteLocks(query, false);
2280+
AcquireRewriteLocks(query, false, false);
22812281

22822282
context.buf=buf;
22832283
context.namespaces=list_make1(&dpns);
@@ -2421,8 +2421,11 @@ get_query_def(Query *query, StringInfo buf, List *parentnamespace,
24212421
* relations, and fix up deleted columns in JOIN RTEs.This ensures
24222422
* consistent results.Note we assume it's OK to scribble on the passed
24232423
* querytree!
2424+
*
2425+
* We are only deparsing the query (we are not about to execute it), so we
2426+
* only need AccessShareLock on the relations it mentions.
24242427
*/
2425-
AcquireRewriteLocks(query, false);
2428+
AcquireRewriteLocks(query, false, false);
24262429

24272430
context.buf=buf;
24282431
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
externNode*build_column_default(Relationrel,intattrno);
2325

2426
#endif/* REWRITEHANDLER_H */

0 commit comments

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