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

Commit5ec41e3

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

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
@@ -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,
@@ -69,9 +75,19 @@ static Query *fireRIRrules(Query *parsetree, List *activeRIRs,
6975
* These locks will ensure that the relation schemas don't change under us
7076
* while we are rewriting and planning the query.
7177
*
72-
* forUpdatePushedDown indicates that a pushed-down FOR UPDATE/SHARE applies
73-
* to the current subquery, requiring all rels to be opened with RowShareLock.
74-
* This should always be false at the start of the recursion.
78+
* forExecute indicates that the query is about to be executed.
79+
* If so, we'll acquire RowExclusiveLock on the query's resultRelation,
80+
* RowShareLock on any relation accessed FOR UPDATE/SHARE, and
81+
* AccessShareLock on all other relations mentioned.
82+
*
83+
* If forExecute is false, AccessShareLock is acquired on all relations.
84+
* This case is suitable for ruleutils.c, for example, where we only need
85+
* schema stability and we don't intend to actually modify any relations.
86+
*
87+
* forUpdatePushedDown indicates that a pushed-down FOR UPDATE/SHARE
88+
* applies to the current subquery, requiring all rels to be opened with at
89+
* least RowShareLock. This should always be false at the top of the
90+
* recursion. This flag is ignored if forExecute is false.
7591
*
7692
* A secondary purpose of this routine is to fix up JOIN RTE references to
7793
* dropped columns (see details below). Because the RTEs are modified in
@@ -99,10 +115,15 @@ static Query *fireRIRrules(Query *parsetree, List *activeRIRs,
99115
* construction of a nested join was O(N^2) in the nesting depth.)
100116
*/
101117
void
102-
AcquireRewriteLocks(Query*parsetree,boolforUpdatePushedDown)
118+
AcquireRewriteLocks(Query*parsetree,
119+
boolforExecute,
120+
boolforUpdatePushedDown)
103121
{
104122
ListCell*l;
105123
intrt_index;
124+
acquireLocksOnSubLinks_contextcontext;
125+
126+
context.for_execute=forExecute;
106127

107128
/*
108129
* First, process RTEs of the current query level.
@@ -128,14 +149,12 @@ AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown)
128149
* release it until end of transaction. This protects the
129150
* rewriter and planner against schema changes mid-query.
130151
*
131-
* If the relation is the query's result relation, then we
132-
* need RowExclusiveLock. Otherwise, check to see if the
133-
* relation is accessed FOR UPDATE/SHARE or not. We can't
134-
* just grab AccessShareLock because then the executor would
135-
* be trying to upgrade the lock, leading to possible
136-
* deadlocks.
152+
* Assuming forExecute is true, this logic must match what the
153+
* executor will do, else we risk lock-upgrade deadlocks.
137154
*/
138-
if (rt_index==parsetree->resultRelation)
155+
if (!forExecute)
156+
lockmode=AccessShareLock;
157+
elseif (rt_index==parsetree->resultRelation)
139158
lockmode=RowExclusiveLock;
140159
elseif (forUpdatePushedDown||
141160
get_parse_rowmark(parsetree,rt_index)!=NULL)
@@ -223,6 +242,7 @@ AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown)
223242
* recurse to process the represented subquery.
224243
*/
225244
AcquireRewriteLocks(rte->subquery,
245+
forExecute,
226246
(forUpdatePushedDown||
227247
get_parse_rowmark(parsetree,rt_index)!=NULL));
228248
break;
@@ -238,23 +258,23 @@ AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown)
238258
{
239259
CommonTableExpr*cte= (CommonTableExpr*)lfirst(l);
240260

241-
AcquireRewriteLocks((Query*)cte->ctequery, false);
261+
AcquireRewriteLocks((Query*)cte->ctequery,forExecute,false);
242262
}
243263

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

253273
/*
254274
* Walker to find sublink subqueries for AcquireRewriteLocks
255275
*/
256276
staticbool
257-
acquireLocksOnSubLinks(Node*node,void*context)
277+
acquireLocksOnSubLinks(Node*node,acquireLocksOnSubLinks_context*context)
258278
{
259279
if (node==NULL)
260280
return false;
@@ -263,7 +283,9 @@ acquireLocksOnSubLinks(Node *node, void *context)
263283
SubLink*sub= (SubLink*)node;
264284

265285
/* Do what we came for */
266-
AcquireRewriteLocks((Query*)sub->subselect, false);
286+
AcquireRewriteLocks((Query*)sub->subselect,
287+
context->for_execute,
288+
false);
267289
/* Fall through to process lefthand args of SubLink */
268290
}
269291

@@ -305,6 +327,9 @@ rewriteRuleAction(Query *parsetree,
305327
intrt_length;
306328
Query*sub_action;
307329
Query**sub_action_ptr;
330+
acquireLocksOnSubLinks_contextcontext;
331+
332+
context.for_execute= true;
308333

309334
/*
310335
* Make modifiable copies of rule action and qual (what we're passed are
@@ -316,8 +341,8 @@ rewriteRuleAction(Query *parsetree,
316341
/*
317342
* Acquire necessary locks and fix any deleted JOIN RTE entries.
318343
*/
319-
AcquireRewriteLocks(rule_action, false);
320-
(void)acquireLocksOnSubLinks(rule_qual,NULL);
344+
AcquireRewriteLocks(rule_action,true,false);
345+
(void)acquireLocksOnSubLinks(rule_qual,&context);
321346

322347
current_varno=rt_index;
323348
rt_length=list_length(parsetree->rtable);
@@ -1367,7 +1392,7 @@ ApplyRetrieveRule(Query *parsetree,
13671392
*/
13681393
rule_action=copyObject(linitial(rule->actions));
13691394

1370-
AcquireRewriteLocks(rule_action,forUpdatePushedDown);
1395+
AcquireRewriteLocks(rule_action,true,forUpdatePushedDown);
13711396

13721397
/*
13731398
* Recursively expand any view references inside the view.
@@ -1690,14 +1715,17 @@ CopyAndAddInvertedQual(Query *parsetree,
16901715
{
16911716
/* Don't scribble on the passed qual (it's in the relcache!) */
16921717
Node*new_qual= (Node*)copyObject(rule_qual);
1718+
acquireLocksOnSubLinks_contextcontext;
1719+
1720+
context.for_execute= true;
16931721

16941722
/*
16951723
* In case there are subqueries in the qual, acquire necessary locks and
16961724
* fix any deleted JOIN RTE entries. (This is somewhat redundant with
16971725
* rewriteRuleAction, but not entirely ... consider restructuring so that
16981726
* we only need to process the qual this way once.)
16991727
*/
1700-
(void)acquireLocksOnSubLinks(new_qual,NULL);
1728+
(void)acquireLocksOnSubLinks(new_qual,&context);
17011729

17021730
/* Fix references to OLD */
17031731
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
@@ -2518,7 +2518,7 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
25182518
query=getInsertSelectQuery(query,NULL);
25192519

25202520
/* Must acquire locks right away; see notes in get_query_def() */
2521-
AcquireRewriteLocks(query, false);
2521+
AcquireRewriteLocks(query, false, false);
25222522

25232523
context.buf=buf;
25242524
context.namespaces=list_make1(&dpns);
@@ -2666,8 +2666,11 @@ get_query_def(Query *query, StringInfo buf, List *parentnamespace,
26662666
* relations, and fix up deleted columns in JOIN RTEs.This ensures
26672667
* consistent results.Note we assume it's OK to scribble on the passed
26682668
* querytree!
2669+
*
2670+
* We are only deparsing the query (we are not about to execute it), so we
2671+
* only need AccessShareLock on the relations it mentions.
26692672
*/
2670-
AcquireRewriteLocks(query, false);
2673+
AcquireRewriteLocks(query, false, false);
26712674

26722675
context.buf=buf;
26732676
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