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

Commit7b8b8a4

Browse files
committed
Improve representation of PlanRowMark.
This patch fixes two inadequacies of the PlanRowMark representation.First, that the original LockingClauseStrength isn't stored (and cannot beinferred for foreign tables, which always get ROW_MARK_COPY). Since somePlanRowMarks are created out of whole cloth and don't actually have anancestral RowMarkClause, this requires adding a dummy LCS_NONE value toenum LockingClauseStrength, which is fairly annoying but the alternativesseem worse. This fix allows getting rid of the use of get_parse_rowmark()in FDWs (as per the discussion around commits462bd95 and8ec8760), and it simplifies some things elsewhere.Second, that the representation assumed that all child tables in aninheritance hierarchy would use the same RowMarkType. That's true todaybut will soon not be true. We add an "allMarkTypes" field that identifiesthe union of mark types used in all a parent table's children, and usethat where appropriate (currently, only in preprocess_targetlist()).In passing fix a couple of minor infelicities left over from the SKIPLOCKED patch, notably that _outPlanRowMark still thought waitPolicyis a bool.Catversion bump is required because the numeric values of enumLockingClauseStrength can appear in on-disk rules.Extracted from a much larger patch to support foreign table inheritance;it seemed worth breaking this out, since it's a separable concern.Shigeru Hanada and Etsuro Fujita, somewhat modified by me
1 parent9fac5fd commit7b8b8a4

File tree

14 files changed

+108
-85
lines changed

14 files changed

+108
-85
lines changed

‎contrib/postgres_fdw/postgres_fdw.c‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -822,13 +822,14 @@ postgresGetForeignPlan(PlannerInfo *root,
822822
}
823823
else
824824
{
825-
RowMarkClause*rc=get_parse_rowmark(root->parse,baserel->relid);
825+
PlanRowMark*rc=get_plan_rowmark(root->rowMarks,baserel->relid);
826826

827827
if (rc)
828828
{
829829
/*
830830
* Relation is specified as a FOR UPDATE/SHARE target, so handle
831-
* that.
831+
* that. (But we could also see LCS_NONE, meaning this isn't a
832+
* target relation after all.)
832833
*
833834
* For now, just ignore any [NO] KEY specification, since (a) it's
834835
* not clear what that means for a remote table that we don't have
@@ -837,6 +838,9 @@ postgresGetForeignPlan(PlannerInfo *root,
837838
*/
838839
switch (rc->strength)
839840
{
841+
caseLCS_NONE:
842+
/* No locking needed */
843+
break;
840844
caseLCS_FORKEYSHARE:
841845
caseLCS_FORSHARE:
842846
appendStringInfoString(&sql," FOR SHARE");

‎src/backend/nodes/copyfuncs.c‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -991,6 +991,8 @@ _copyPlanRowMark(const PlanRowMark *from)
991991
COPY_SCALAR_FIELD(prti);
992992
COPY_SCALAR_FIELD(rowmarkId);
993993
COPY_SCALAR_FIELD(markType);
994+
COPY_SCALAR_FIELD(allMarkTypes);
995+
COPY_SCALAR_FIELD(strength);
994996
COPY_SCALAR_FIELD(waitPolicy);
995997
COPY_SCALAR_FIELD(isParent);
996998

@@ -2510,7 +2512,7 @@ _copyXmlSerialize(const XmlSerialize *from)
25102512
staticRoleSpec*
25112513
_copyRoleSpec(constRoleSpec*from)
25122514
{
2513-
RoleSpec*newnode=makeNode(RoleSpec);
2515+
RoleSpec*newnode=makeNode(RoleSpec);
25142516

25152517
COPY_SCALAR_FIELD(roletype);
25162518
COPY_STRING_FIELD(rolename);

‎src/backend/nodes/outfuncs.c‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -852,7 +852,9 @@ _outPlanRowMark(StringInfo str, const PlanRowMark *node)
852852
WRITE_UINT_FIELD(prti);
853853
WRITE_UINT_FIELD(rowmarkId);
854854
WRITE_ENUM_FIELD(markType,RowMarkType);
855-
WRITE_BOOL_FIELD(waitPolicy);
855+
WRITE_INT_FIELD(allMarkTypes);
856+
WRITE_ENUM_FIELD(strength,LockClauseStrength);
857+
WRITE_ENUM_FIELD(waitPolicy,LockWaitPolicy);
856858
WRITE_BOOL_FIELD(isParent);
857859
}
858860

‎src/backend/optimizer/plan/planner.c‎

Lines changed: 31 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -2219,35 +2219,42 @@ preprocess_rowmarks(PlannerInfo *root)
22192219
if (rte->rtekind!=RTE_RELATION)
22202220
continue;
22212221

2222-
/*
2223-
* Similarly, ignore RowMarkClauses for foreign tables; foreign tables
2224-
* will instead get ROW_MARK_COPY items in the next loop. (FDWs might
2225-
* choose to do something special while fetching their rows, but that
2226-
* is of no concern here.)
2227-
*/
2228-
if (rte->relkind==RELKIND_FOREIGN_TABLE)
2229-
continue;
2230-
22312222
rels=bms_del_member(rels,rc->rti);
22322223

22332224
newrc=makeNode(PlanRowMark);
22342225
newrc->rti=newrc->prti=rc->rti;
22352226
newrc->rowmarkId=++(root->glob->lastRowMarkId);
2236-
switch (rc->strength)
2227+
if (rte->relkind==RELKIND_FOREIGN_TABLE)
22372228
{
2238-
caseLCS_FORUPDATE:
2239-
newrc->markType=ROW_MARK_EXCLUSIVE;
2240-
break;
2241-
caseLCS_FORNOKEYUPDATE:
2242-
newrc->markType=ROW_MARK_NOKEYEXCLUSIVE;
2243-
break;
2244-
caseLCS_FORSHARE:
2245-
newrc->markType=ROW_MARK_SHARE;
2246-
break;
2247-
caseLCS_FORKEYSHARE:
2248-
newrc->markType=ROW_MARK_KEYSHARE;
2249-
break;
2229+
/* For now, we force all foreign tables to use ROW_MARK_COPY */
2230+
newrc->markType=ROW_MARK_COPY;
2231+
}
2232+
else
2233+
{
2234+
/* regular table, apply the appropriate lock type */
2235+
switch (rc->strength)
2236+
{
2237+
caseLCS_NONE:
2238+
/* we intentionally throw an error for LCS_NONE */
2239+
elog(ERROR,"unrecognized LockClauseStrength %d",
2240+
(int)rc->strength);
2241+
break;
2242+
caseLCS_FORKEYSHARE:
2243+
newrc->markType=ROW_MARK_KEYSHARE;
2244+
break;
2245+
caseLCS_FORSHARE:
2246+
newrc->markType=ROW_MARK_SHARE;
2247+
break;
2248+
caseLCS_FORNOKEYUPDATE:
2249+
newrc->markType=ROW_MARK_NOKEYEXCLUSIVE;
2250+
break;
2251+
caseLCS_FORUPDATE:
2252+
newrc->markType=ROW_MARK_EXCLUSIVE;
2253+
break;
2254+
}
22502255
}
2256+
newrc->allMarkTypes= (1 <<newrc->markType);
2257+
newrc->strength=rc->strength;
22512258
newrc->waitPolicy=rc->waitPolicy;
22522259
newrc->isParent= false;
22532260

@@ -2276,6 +2283,8 @@ preprocess_rowmarks(PlannerInfo *root)
22762283
newrc->markType=ROW_MARK_REFERENCE;
22772284
else
22782285
newrc->markType=ROW_MARK_COPY;
2286+
newrc->allMarkTypes= (1 <<newrc->markType);
2287+
newrc->strength=LCS_NONE;
22792288
newrc->waitPolicy=LockWaitBlock;/* doesn't matter */
22802289
newrc->isParent= false;
22812290

‎src/backend/optimizer/prep/prepsecurity.c‎

Lines changed: 7 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -73,8 +73,8 @@ expand_security_quals(PlannerInfo *root, List *tlist)
7373
rt_index=0;
7474
foreach(cell,parse->rtable)
7575
{
76-
booltargetRelation= false;
77-
RangeTblEntry*rte= (RangeTblEntry*)lfirst(cell);
76+
booltargetRelation= false;
77+
RangeTblEntry*rte= (RangeTblEntry*)lfirst(cell);
7878

7979
rt_index++;
8080

@@ -241,30 +241,10 @@ expand_security_qual(PlannerInfo *root, List *tlist, int rt_index,
241241
rc=get_plan_rowmark(root->rowMarks,rt_index);
242242
if (rc!=NULL)
243243
{
244-
switch (rc->markType)
245-
{
246-
caseROW_MARK_EXCLUSIVE:
247-
applyLockingClause(subquery,1,LCS_FORUPDATE,
248-
rc->waitPolicy, false);
249-
break;
250-
caseROW_MARK_NOKEYEXCLUSIVE:
251-
applyLockingClause(subquery,1,LCS_FORNOKEYUPDATE,
252-
rc->waitPolicy, false);
253-
break;
254-
caseROW_MARK_SHARE:
255-
applyLockingClause(subquery,1,LCS_FORSHARE,
256-
rc->waitPolicy, false);
257-
break;
258-
caseROW_MARK_KEYSHARE:
259-
applyLockingClause(subquery,1,LCS_FORKEYSHARE,
260-
rc->waitPolicy, false);
261-
break;
262-
caseROW_MARK_REFERENCE:
263-
caseROW_MARK_COPY:
264-
/* No locking needed */
265-
break;
266-
}
267-
root->rowMarks=list_delete(root->rowMarks,rc);
244+
if (rc->strength!=LCS_NONE)
245+
applyLockingClause(subquery,1,rc->strength,
246+
rc->waitPolicy, false);
247+
root->rowMarks=list_delete_ptr(root->rowMarks,rc);
268248
}
269249

270250
/*
@@ -276,6 +256,7 @@ expand_security_qual(PlannerInfo *root, List *tlist, int rt_index,
276256
if (targetRelation)
277257
applyLockingClause(subquery,1,LCS_FORUPDATE,
278258
LockWaitBlock, false);
259+
279260
/*
280261
* Replace any variables in the outer query that refer to the
281262
* original relation RTE with references to columns that we will

‎src/backend/optimizer/prep/preptlist.c‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -92,9 +92,9 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
9292
if (rc->rti!=rc->prti)
9393
continue;
9494

95-
if (rc->markType!=ROW_MARK_COPY)
95+
if (rc->allMarkTypes& ~(1 <<ROW_MARK_COPY))
9696
{
97-
/*It's a regular table, sofetch its TID */
97+
/*Need tofetch TID */
9898
var=makeVar(rc->rti,
9999
SelfItemPointerAttributeNumber,
100100
TIDOID,
@@ -125,9 +125,9 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
125125
tlist=lappend(tlist,tle);
126126
}
127127
}
128-
else
128+
if (rc->allMarkTypes& (1 <<ROW_MARK_COPY))
129129
{
130-
/*Not a table, so we need the whole row as a junk var */
130+
/*Need the whole row as a junk var */
131131
var=makeWholeRowVar(rt_fetch(rc->rti,range_table),
132132
rc->rti,
133133
0,

‎src/backend/optimizer/prep/prepunion.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1389,9 +1389,14 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
13891389
newrc->prti=rti;
13901390
newrc->rowmarkId=oldrc->rowmarkId;
13911391
newrc->markType=oldrc->markType;
1392+
newrc->allMarkTypes= (1 <<newrc->markType);
1393+
newrc->strength=oldrc->strength;
13921394
newrc->waitPolicy=oldrc->waitPolicy;
13931395
newrc->isParent= false;
13941396

1397+
/* Include child's rowmark type in parent's allMarkTypes */
1398+
oldrc->allMarkTypes |=newrc->allMarkTypes;
1399+
13951400
root->rowMarks=lappend(root->rowMarks,newrc);
13961401
}
13971402

‎src/backend/parser/analyze.c‎

Lines changed: 24 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2254,11 +2254,18 @@ transformCreateTableAsStmt(ParseState *pstate, CreateTableAsStmt *stmt)
22542254
}
22552255

22562256

2257-
char*
2257+
/*
2258+
* Produce a string representation of a LockClauseStrength value.
2259+
* This should only be applied to valid values (not LCS_NONE).
2260+
*/
2261+
constchar*
22582262
LCS_asString(LockClauseStrengthstrength)
22592263
{
22602264
switch (strength)
22612265
{
2266+
caseLCS_NONE:
2267+
Assert(false);
2268+
break;
22622269
caseLCS_FORKEYSHARE:
22632270
return"FOR KEY SHARE";
22642271
caseLCS_FORSHARE:
@@ -2279,6 +2286,8 @@ LCS_asString(LockClauseStrength strength)
22792286
void
22802287
CheckSelectLocking(Query*qry,LockClauseStrengthstrength)
22812288
{
2289+
Assert(strength!=LCS_NONE);/* else caller error */
2290+
22822291
if (qry->setOperations)
22832292
ereport(ERROR,
22842293
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -2498,6 +2507,8 @@ applyLockingClause(Query *qry, Index rtindex,
24982507
{
24992508
RowMarkClause*rc;
25002509

2510+
Assert(strength!=LCS_NONE);/* else caller error */
2511+
25012512
/* If it's an explicit clause, make sure hasForUpdate gets set */
25022513
if (!pushedDown)
25032514
qry->hasForUpdate= true;
@@ -2506,20 +2517,21 @@ applyLockingClause(Query *qry, Index rtindex,
25062517
if ((rc=get_parse_rowmark(qry,rtindex))!=NULL)
25072518
{
25082519
/*
2509-
* If the same RTE is specifiedfor more than one locking strength,
2510-
*treat is asthe strongest. (Reasonable, since you can't take both
2511-
*a sharedand exclusive lock at the same time; it'll end up being
2512-
*exclusiveanyway.)
2520+
* If the same RTE is specifiedwith more than one locking strength,
2521+
*usethe strongest. (Reasonable, since you can't take both a shared
2522+
* and exclusive lock at the same time; it'll end up being exclusive
2523+
* anyway.)
25132524
*
2514-
* Similarly, if the same RTE is specified with more than one lock wait
2515-
* policy, consider that NOWAIT wins over SKIP LOCKED, which in turn
2516-
* wins over waiting for the lock (the default). This is a bit more
2517-
* debatable but raising an error doesn't seem helpful.(Consider for
2518-
* instance SELECT FOR UPDATE NOWAIT from a view that internally
2525+
* Similarly, if the same RTE is specified with more than one lock
2526+
*waitpolicy, consider that NOWAIT wins over SKIP LOCKED, which in
2527+
*turnwins over waiting for the lock (the default). This is a bit
2528+
*moredebatable but raising an error doesn't seem helpful. (Consider
2529+
*forinstance SELECT FOR UPDATE NOWAIT from a view that internally
25192530
* contains a plain FOR UPDATE spec.) Having NOWAIT win over SKIP
25202531
* LOCKED is reasonable since the former throws an error in case of
2521-
* coming across a locked tuple, which may be undesirable in some cases
2522-
* but it seems better than silently returning inconsistent results.
2532+
* coming across a locked tuple, which may be undesirable in some
2533+
* cases but it seems better than silently returning inconsistent
2534+
* results.
25232535
*
25242536
* And of course pushedDown becomes false if any clause is explicit.
25252537
*/

‎src/backend/tcop/utility.c‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -2395,26 +2395,22 @@ CreateCommandTag(Node *parsetree)
23952395
elseif (stmt->rowMarks!=NIL)
23962396
{
23972397
/* not 100% but probably close enough */
2398-
switch (((PlanRowMark*)linitial(stmt->rowMarks))->markType)
2398+
switch (((PlanRowMark*)linitial(stmt->rowMarks))->strength)
23992399
{
2400-
caseROW_MARK_EXCLUSIVE:
2401-
tag="SELECT FOR UPDATE";
2402-
break;
2403-
caseROW_MARK_NOKEYEXCLUSIVE:
2404-
tag="SELECT FOR NO KEY UPDATE";
2400+
caseLCS_FORKEYSHARE:
2401+
tag="SELECT FOR KEY SHARE";
24052402
break;
2406-
caseROW_MARK_SHARE:
2403+
caseLCS_FORSHARE:
24072404
tag="SELECT FOR SHARE";
24082405
break;
2409-
caseROW_MARK_KEYSHARE:
2410-
tag="SELECT FOR KEYSHARE";
2406+
caseLCS_FORNOKEYUPDATE:
2407+
tag="SELECT FORNOKEYUPDATE";
24112408
break;
2412-
caseROW_MARK_REFERENCE:
2413-
caseROW_MARK_COPY:
2414-
tag="SELECT";
2409+
caseLCS_FORUPDATE:
2410+
tag="SELECT FOR UPDATE";
24152411
break;
24162412
default:
2417-
tag="???";
2413+
tag="SELECT";
24182414
break;
24192415
}
24202416
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4453,6 +4453,11 @@ get_select_query_def(Query *query, deparse_context *context,
44534453

44544454
switch (rc->strength)
44554455
{
4456+
caseLCS_NONE:
4457+
/* we intentionally throw an error for LCS_NONE */
4458+
elog(ERROR,"unrecognized LockClauseStrength %d",
4459+
(int)rc->strength);
4460+
break;
44564461
caseLCS_FORKEYSHARE:
44574462
appendContextKeyword(context," FOR KEY SHARE",
44584463
-PRETTYINDENT_STD,PRETTYINDENT_STD,0);

0 commit comments

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