database/postgres
1
0
Fork 0
mirror of https://github.com/postgres/postgres.git synced 2025-11-13 16:22:44 +03:00
Code Activity

Improve representation of PlanRowMark.

Browse Source 
This patch fixes two inadequacies of the PlanRowMark representation.

First, that the original LockingClauseStrength isn't stored (and cannot be
inferred for foreign tables, which always get ROW_MARK_COPY).  Since some
PlanRowMarks are created out of whole cloth and don't actually have an
ancestral RowMarkClause, this requires adding a dummy LCS_NONE value to
enum LockingClauseStrength, which is fairly annoying but the alternatives
seem worse.  This fix allows getting rid of the use of get_parse_rowmark()
in FDWs (as per the discussion around commits 462bd95705 and
8ec8760fc8), and it simplifies some things elsewhere.

Second, that the representation assumed that all child tables in an
inheritance hierarchy would use the same RowMarkType.  That's true today
but will soon not be true.  We add an "allMarkTypes" field that identifies
the union of mark types used in all a parent table's children, and use
that where appropriate (currently, only in preprocess_targetlist()).

In passing fix a couple of minor infelicities left over from the SKIP
LOCKED patch, notably that _outPlanRowMark still thought waitPolicy
is a bool.

Catversion bump is required because the numeric values of enum
LockingClauseStrength 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
This commit is contained in:
Tom Lane
2015-03-15 18:41:47 -04:00
parent 9fac5fd741
commit 7b8b8a4331
14 changed files with 110 additions and 87 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
src/backend/nodes/copyfuncs.c
View File

@@ -991,6 +991,8 @@ _copyPlanRowMark(const PlanRowMark *from)
COPY_SCALAR_FIELD(prti);
COPY_SCALAR_FIELD(rowmarkId);
COPY_SCALAR_FIELD(markType);
COPY_SCALAR_FIELD(allMarkTypes);
COPY_SCALAR_FIELD(strength);
COPY_SCALAR_FIELD(waitPolicy);
COPY_SCALAR_FIELD(isParent);
@@ -2510,7 +2512,7 @@ _copyXmlSerialize(const XmlSerialize *from)
static RoleSpec *
_copyRoleSpec(const RoleSpec *from)
{
RoleSpec *newnode = makeNode(RoleSpec);
RoleSpec *newnode = makeNode(RoleSpec);
COPY_SCALAR_FIELD(roletype);
COPY_STRING_FIELD(rolename);

4
src/backend/nodes/outfuncs.c
View File

@@ -852,7 +852,9 @@ _outPlanRowMark(StringInfo str, const PlanRowMark *node)
WRITE_UINT_FIELD(prti);
WRITE_UINT_FIELD(rowmarkId);
WRITE_ENUM_FIELD(markType, RowMarkType);
WRITE_BOOL_FIELD(waitPolicy);
WRITE_INT_FIELD(allMarkTypes);
WRITE_ENUM_FIELD(strength, LockClauseStrength);
WRITE_ENUM_FIELD(waitPolicy, LockWaitPolicy);
WRITE_BOOL_FIELD(isParent);
}

53
src/backend/optimizer/plan/planner.c
View File

@@ -2219,35 +2219,42 @@ preprocess_rowmarks(PlannerInfo *root)
if (rte->rtekind != RTE_RELATION)
continue;
/*
* Similarly, ignore RowMarkClauses for foreign tables; foreign tables
* will instead get ROW_MARK_COPY items in the next loop. (FDWs might
* choose to do something special while fetching their rows, but that
* is of no concern here.)
*/
if (rte->relkind == RELKIND_FOREIGN_TABLE)
continue;
rels = bms_del_member(rels, rc->rti);
newrc = makeNode(PlanRowMark);
newrc->rti = newrc->prti = rc->rti;
newrc->rowmarkId = ++(root->glob->lastRowMarkId);
switch (rc->strength)
if (rte->relkind == RELKIND_FOREIGN_TABLE)
{
case LCS_FORUPDATE:
newrc->markType = ROW_MARK_EXCLUSIVE;
break;
case LCS_FORNOKEYUPDATE:
newrc->markType = ROW_MARK_NOKEYEXCLUSIVE;
break;
case LCS_FORSHARE:
newrc->markType = ROW_MARK_SHARE;
break;
case LCS_FORKEYSHARE:
newrc->markType = ROW_MARK_KEYSHARE;
break;
/* For now, we force all foreign tables to use ROW_MARK_COPY */
newrc->markType = ROW_MARK_COPY;
}
else
{
/* regular table, apply the appropriate lock type */
switch (rc->strength)
{
case LCS_NONE:
/* we intentionally throw an error for LCS_NONE */
elog(ERROR, "unrecognized LockClauseStrength %d",
(int) rc->strength);
break;
case LCS_FORKEYSHARE:
newrc->markType = ROW_MARK_KEYSHARE;
break;
case LCS_FORSHARE:
newrc->markType = ROW_MARK_SHARE;
break;
case LCS_FORNOKEYUPDATE:
newrc->markType = ROW_MARK_NOKEYEXCLUSIVE;
break;
case LCS_FORUPDATE:
newrc->markType = ROW_MARK_EXCLUSIVE;
break;
}
}
newrc->allMarkTypes = (1 << newrc->markType);
newrc->strength = rc->strength;
newrc->waitPolicy = rc->waitPolicy;
newrc->isParent = false;
@@ -2276,6 +2283,8 @@ preprocess_rowmarks(PlannerInfo *root)
newrc->markType = ROW_MARK_REFERENCE;
else
newrc->markType = ROW_MARK_COPY;
newrc->allMarkTypes = (1 << newrc->markType);
newrc->strength = LCS_NONE;
newrc->waitPolicy = LockWaitBlock; /* doesn't matter */
newrc->isParent = false;

33
src/backend/optimizer/prep/prepsecurity.c
View File

@@ -73,8 +73,8 @@ expand_security_quals(PlannerInfo *root, List *tlist)
rt_index = 0;
foreach(cell, parse->rtable)
{
bool targetRelation = false;
RangeTblEntry *rte = (RangeTblEntry *) lfirst(cell);
bool targetRelation = false;
RangeTblEntry *rte = (RangeTblEntry *) lfirst(cell);
rt_index++;
@@ -241,30 +241,10 @@ expand_security_qual(PlannerInfo *root, List *tlist, int rt_index,
rc = get_plan_rowmark(root->rowMarks, rt_index);
if (rc != NULL)
{
switch (rc->markType)
{
case ROW_MARK_EXCLUSIVE:
applyLockingClause(subquery, 1, LCS_FORUPDATE,
rc->waitPolicy, false);
break;
case ROW_MARK_NOKEYEXCLUSIVE:
applyLockingClause(subquery, 1, LCS_FORNOKEYUPDATE,
rc->waitPolicy, false);
break;
case ROW_MARK_SHARE:
applyLockingClause(subquery, 1, LCS_FORSHARE,
rc->waitPolicy, false);
break;
case ROW_MARK_KEYSHARE:
applyLockingClause(subquery, 1, LCS_FORKEYSHARE,
rc->waitPolicy, false);
break;
case ROW_MARK_REFERENCE:
case ROW_MARK_COPY:
/* No locking needed */
break;
}
root->rowMarks = list_delete(root->rowMarks, rc);
if (rc->strength != LCS_NONE)
applyLockingClause(subquery, 1, rc->strength,
rc->waitPolicy, false);
root->rowMarks = list_delete_ptr(root->rowMarks, rc);
}
/*
@@ -276,6 +256,7 @@ expand_security_qual(PlannerInfo *root, List *tlist, int rt_index,
if (targetRelation)
applyLockingClause(subquery, 1, LCS_FORUPDATE,
LockWaitBlock, false);
/*
* Replace any variables in the outer query that refer to the
* original relation RTE with references to columns that we will

8
src/backend/optimizer/prep/preptlist.c
View File

@@ -92,9 +92,9 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
if (rc->rti != rc->prti)
continue;
if (rc->markType != ROW_MARK_COPY)
if (rc->allMarkTypes & ~(1 << ROW_MARK_COPY))
{
/* It's a regular table, so fetch its TID */
/* Need to fetch TID */
var = makeVar(rc->rti,
SelfItemPointerAttributeNumber,
TIDOID,
@@ -125,9 +125,9 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
tlist = lappend(tlist, tle);
}
}
else
if (rc->allMarkTypes & (1 << ROW_MARK_COPY))
{
/* Not a table, so we need the whole row as a junk var */
/* Need the whole row as a junk var */
var = makeWholeRowVar(rt_fetch(rc->rti, range_table),
rc->rti,
0,

5
src/backend/optimizer/prep/prepunion.c
View File

@@ -1389,9 +1389,14 @@ expand_inherited_rtentry(PlannerInfo *root, RangeTblEntry *rte, Index rti)
newrc->prti = rti;
newrc->rowmarkId = oldrc->rowmarkId;
newrc->markType = oldrc->markType;
newrc->allMarkTypes = (1 << newrc->markType);
newrc->strength = oldrc->strength;
newrc->waitPolicy = oldrc->waitPolicy;
newrc->isParent = false;
/* Include child's rowmark type in parent's allMarkTypes */
oldrc->allMarkTypes |= newrc->allMarkTypes;
root->rowMarks = lappend(root->rowMarks, newrc);
}

36
src/backend/parser/analyze.c
View File

@@ -2254,11 +2254,18 @@ transformCreateTableAsStmt(ParseState *pstate, CreateTableAsStmt *stmt)
}
char *
/*
* Produce a string representation of a LockClauseStrength value.
* This should only be applied to valid values (not LCS_NONE).
*/
const char *
LCS_asString(LockClauseStrength strength)
{
switch (strength)
{
case LCS_NONE:
Assert(false);
break;
case LCS_FORKEYSHARE:
return "FOR KEY SHARE";
case LCS_FORSHARE:
@@ -2279,6 +2286,8 @@ LCS_asString(LockClauseStrength strength)
void
CheckSelectLocking(Query *qry, LockClauseStrength strength)
{
Assert(strength != LCS_NONE); /* else caller error */
if (qry->setOperations)
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
@@ -2498,6 +2507,8 @@ applyLockingClause(Query *qry, Index rtindex,
{
RowMarkClause *rc;
Assert(strength != LCS_NONE); /* else caller error */
/* If it's an explicit clause, make sure hasForUpdate gets set */
if (!pushedDown)
qry->hasForUpdate = true;
@@ -2506,20 +2517,21 @@ applyLockingClause(Query *qry, Index rtindex,
if ((rc = get_parse_rowmark(qry, rtindex)) != NULL)
{
/*
* If the same RTE is specified for more than one locking strength,
* treat is as the strongest. (Reasonable, since you can't take both
* a shared and exclusive lock at the same time; it'll end up being
* exclusive anyway.)
* If the same RTE is specified with more than one locking strength,
* use the strongest. (Reasonable, since you can't take both a shared
* and exclusive lock at the same time; it'll end up being exclusive
* anyway.)
*
* Similarly, if the same RTE is specified with more than one lock wait
* policy, consider that NOWAIT wins over SKIP LOCKED, which in turn
* wins over waiting for the lock (the default). This is a bit more
* debatable but raising an error doesn't seem helpful. (Consider for
* instance SELECT FOR UPDATE NOWAIT from a view that internally
* Similarly, if the same RTE is specified with more than one lock
* wait policy, consider that NOWAIT wins over SKIP LOCKED, which in
* turn wins over waiting for the lock (the default). This is a bit
* more debatable but raising an error doesn't seem helpful. (Consider
* for instance SELECT FOR UPDATE NOWAIT from a view that internally
* contains a plain FOR UPDATE spec.) Having NOWAIT win over SKIP
* LOCKED is reasonable since the former throws an error in case of
* coming across a locked tuple, which may be undesirable in some cases
* but it seems better than silently returning inconsistent results.
* coming across a locked tuple, which may be undesirable in some
* cases but it seems better than silently returning inconsistent
* results.
*
* And of course pushedDown becomes false if any clause is explicit.
*/

26
src/backend/tcop/utility.c
View File

@@ -2395,26 +2395,22 @@ CreateCommandTag(Node *parsetree)
else if (stmt->rowMarks != NIL)
{
/* not 100% but probably close enough */
switch (((PlanRowMark *) linitial(stmt->rowMarks))->markType)
switch (((PlanRowMark *) linitial(stmt->rowMarks))->strength)
{
case ROW_MARK_EXCLUSIVE:
tag = "SELECT FOR UPDATE";
break;
case ROW_MARK_NOKEYEXCLUSIVE:
tag = "SELECT FOR NO KEY UPDATE";
break;
case ROW_MARK_SHARE:
tag = "SELECT FOR SHARE";
break;
case ROW_MARK_KEYSHARE:
case LCS_FORKEYSHARE:
tag = "SELECT FOR KEY SHARE";
break;
case ROW_MARK_REFERENCE:
case ROW_MARK_COPY:
tag = "SELECT";
case LCS_FORSHARE:
tag = "SELECT FOR SHARE";
break;
case LCS_FORNOKEYUPDATE:
tag = "SELECT FOR NO KEY UPDATE";
break;
case LCS_FORUPDATE:
tag = "SELECT FOR UPDATE";
break;
default:
tag = "???";
tag = "SELECT";
break;
}
}

5
src/backend/utils/adt/ruleutils.c
View File

@@ -4453,6 +4453,11 @@ get_select_query_def(Query *query, deparse_context *context,
switch (rc->strength)
{
case LCS_NONE:
/* we intentionally throw an error for LCS_NONE */
elog(ERROR, "unrecognized LockClauseStrength %d",
(int) rc->strength);
break;
case LCS_FORKEYSHARE:
appendContextKeyword(context, " FOR KEY SHARE",
-PRETTYINDENT_STD, PRETTYINDENT_STD, 0);