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

Commitf557826

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

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
@@ -246,7 +246,7 @@ refresh_matview_datafill(DestReceiver *dest, Query *query,
246246

247247
/* Lock and rewrite, using a copy to preserve the original query. */
248248
copied_query=copyObject(query);
249-
AcquireRewriteLocks(copied_query, false);
249+
AcquireRewriteLocks(copied_query,true,false);
250250
rewritten=QueryRewrite(copied_query);
251251

252252
/* 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
@@ -38,7 +38,13 @@ typedef struct rewrite_event
3838
CmdTypeevent;/* type of rule being fired */
3939
}rewrite_event;
4040

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

108129
/*
109130
* First, process RTEs of the current query level.
@@ -129,14 +150,12 @@ AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown)
129150
* release it until end of transaction. This protects the
130151
* rewriter and planner against schema changes mid-query.
131152
*
132-
* If the relation is the query's result relation, then we
133-
* need RowExclusiveLock. Otherwise, check to see if the
134-
* relation is accessed FOR [KEY] UPDATE/SHARE or not.We
135-
* can't just grab AccessShareLock because then the executor
136-
* would be trying to upgrade the lock, leading to possible
137-
* deadlocks.
153+
* Assuming forExecute is true, this logic must match what the
154+
* executor will do, else we risk lock-upgrade deadlocks.
138155
*/
139-
if (rt_index==parsetree->resultRelation)
156+
if (!forExecute)
157+
lockmode=AccessShareLock;
158+
elseif (rt_index==parsetree->resultRelation)
140159
lockmode=RowExclusiveLock;
141160
elseif (forUpdatePushedDown||
142161
get_parse_rowmark(parsetree,rt_index)!=NULL)
@@ -224,6 +243,7 @@ AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown)
224243
* recurse to process the represented subquery.
225244
*/
226245
AcquireRewriteLocks(rte->subquery,
246+
forExecute,
227247
(forUpdatePushedDown||
228248
get_parse_rowmark(parsetree,rt_index)!=NULL));
229249
break;
@@ -239,23 +259,23 @@ AcquireRewriteLocks(Query *parsetree, bool forUpdatePushedDown)
239259
{
240260
CommonTableExpr*cte= (CommonTableExpr*)lfirst(l);
241261

242-
AcquireRewriteLocks((Query*)cte->ctequery, false);
262+
AcquireRewriteLocks((Query*)cte->ctequery,forExecute,false);
243263
}
244264

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

254274
/*
255275
* Walker to find sublink subqueries for AcquireRewriteLocks
256276
*/
257277
staticbool
258-
acquireLocksOnSubLinks(Node*node,void*context)
278+
acquireLocksOnSubLinks(Node*node,acquireLocksOnSubLinks_context*context)
259279
{
260280
if (node==NULL)
261281
return false;
@@ -264,7 +284,9 @@ acquireLocksOnSubLinks(Node *node, void *context)
264284
SubLink*sub= (SubLink*)node;
265285

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

@@ -306,6 +328,9 @@ rewriteRuleAction(Query *parsetree,
306328
intrt_length;
307329
Query*sub_action;
308330
Query**sub_action_ptr;
331+
acquireLocksOnSubLinks_contextcontext;
332+
333+
context.for_execute= true;
309334

310335
/*
311336
* Make modifiable copies of rule action and qual (what we're passed are
@@ -317,8 +342,8 @@ rewriteRuleAction(Query *parsetree,
317342
/*
318343
* Acquire necessary locks and fix any deleted JOIN RTE entries.
319344
*/
320-
AcquireRewriteLocks(rule_action, false);
321-
(void)acquireLocksOnSubLinks(rule_qual,NULL);
345+
AcquireRewriteLocks(rule_action,true,false);
346+
(void)acquireLocksOnSubLinks(rule_qual,&context);
322347

323348
current_varno=rt_index;
324349
rt_length=list_length(parsetree->rtable);
@@ -1387,7 +1412,7 @@ ApplyRetrieveRule(Query *parsetree,
13871412
*/
13881413
rule_action=copyObject(linitial(rule->actions));
13891414

1390-
AcquireRewriteLocks(rule_action,forUpdatePushedDown);
1415+
AcquireRewriteLocks(rule_action,true,forUpdatePushedDown);
13911416

13921417
/*
13931418
* Recursively expand any view references inside the view.
@@ -1719,14 +1744,17 @@ CopyAndAddInvertedQual(Query *parsetree,
17191744
{
17201745
/* Don't scribble on the passed qual (it's in the relcache!) */
17211746
Node*new_qual= (Node*)copyObject(rule_qual);
1747+
acquireLocksOnSubLinks_contextcontext;
1748+
1749+
context.for_execute= true;
17221750

17231751
/*
17241752
* In case there are subqueries in the qual, acquire necessary locks and
17251753
* fix any deleted JOIN RTE entries. (This is somewhat redundant with
17261754
* rewriteRuleAction, but not entirely ... consider restructuring so that
17271755
* we only need to process the qual this way once.)
17281756
*/
1729-
(void)acquireLocksOnSubLinks(new_qual,NULL);
1757+
(void)acquireLocksOnSubLinks(new_qual,&context);
17301758

17311759
/* Fix references to OLD */
17321760
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
@@ -3858,7 +3858,7 @@ make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc,
38583858
query=getInsertSelectQuery(query,NULL);
38593859

38603860
/* Must acquire locks right away; see notes in get_query_def() */
3861-
AcquireRewriteLocks(query, false);
3861+
AcquireRewriteLocks(query, false, false);
38623862

38633863
context.buf=buf;
38643864
context.namespaces=list_make1(&dpns);
@@ -4004,8 +4004,11 @@ get_query_def(Query *query, StringInfo buf, List *parentnamespace,
40044004
* relations, and fix up deleted columns in JOIN RTEs.This ensures
40054005
* consistent results.Note we assume it's OK to scribble on the passed
40064006
* querytree!
4007+
*
4008+
* We are only deparsing the query (we are not about to execute it), so we
4009+
* only need AccessShareLock on the relations it mentions.
40074010
*/
4008-
AcquireRewriteLocks(query, false);
4011+
AcquireRewriteLocks(query, false, false);
40094012

40104013
context.buf=buf;
40114014
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
externintrelation_is_updatable(Oidreloid,boolinclude_triggers);

0 commit comments

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