I wrote:
>>>> I think the most appropriate solution may be to disallow SELECT FOR
>>>> UPDATE/SHARE on sequences ... so if you have a good reason why we
>>>> shouldn't do so, please explain it.
Attached is a proposed patch to close off this hole. I found that
somebody had already inserted code to forbid the case for foreign
tables, so I just extended that idea a bit (by copying-and-pasting
CheckValidResultRel). Questions:
* Does anyone want to bikeshed on the wording of the error messages?
* Does anyone want to argue for not forbidding SELECT FOR UPDATE on
toast tables?
regards, tom lane
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 620efda838421a94a9835a7e8db1d8fa4b50e1ea..2d491fe6c876e34bd757ab271fada1f959707447 100644
*** a/src/backend/executor/execMain.c
--- b/src/backend/executor/execMain.c
*************** ExecutorCheckPerms_hook_type ExecutorChe
*** 74,79 ****
--- 74,80 ----
/* decls for local routines only used within this module */
static void InitPlan(QueryDesc *queryDesc, int eflags);
+ static void CheckValidMarkRel(Relation rel, RowMarkType markType);
static void ExecPostprocessPlan(EState *estate);
static void ExecEndPlan(PlanState *planstate, EState *estate);
static void ExecutePlan(EState *estate, PlanState *planstate,
*************** InitPlan(QueryDesc *queryDesc, int eflag
*** 837,848 ****
break;
}
! /* if foreign table, tuples can't be locked */
! if (relation && relation->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
! ereport(ERROR,
! (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
! errmsg("SELECT FOR UPDATE/SHARE cannot be used with foreign table \"%s\"",
! RelationGetRelationName(relation))));
erm = (ExecRowMark *) palloc(sizeof(ExecRowMark));
erm->relation = relation;
--- 838,846 ----
break;
}
! /* Check that relation is a legal target for marking */
! if (relation)
! CheckValidMarkRel(relation, rc->markType);
erm = (ExecRowMark *) palloc(sizeof(ExecRowMark));
erm->relation = relation;
*************** InitPlan(QueryDesc *queryDesc, int eflag
*** 977,982 ****
--- 975,982 ----
* In most cases parser and/or planner should have noticed this already, but
* let's make sure. In the view case we do need a test here, because if the
* view wasn't rewritten by a rule, it had better have an INSTEAD trigger.
+ *
+ * Note: when changing this function, see also CheckValidMarkRel.
*/
void
CheckValidResultRel(Relation resultRel, CmdType operation)
*************** CheckValidResultRel(Relation resultRel,
*** 1048,1053 ****
--- 1048,1104 ----
}
/*
+ * Check that a proposed rowmark target relation is a legal target
+ *
+ * In most cases parser and/or planner should have noticed this already, but
+ * they don't cover all cases.
+ */
+ static void
+ CheckValidMarkRel(Relation rel, RowMarkType markType)
+ {
+ switch (rel->rd_rel->relkind)
+ {
+ case RELKIND_RELATION:
+ /* OK */
+ break;
+ case RELKIND_SEQUENCE:
+ /* Must disallow this because we don't vacuum sequences */
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot lock rows in sequence \"%s\"",
+ RelationGetRelationName(rel))));
+ break;
+ case RELKIND_TOASTVALUE:
+ /* We could allow this, but there seems no good reason to */
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot lock rows in TOAST relation \"%s\"",
+ RelationGetRelationName(rel))));
+ break;
+ case RELKIND_VIEW:
+ /* Should not get here */
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot lock rows in view \"%s\"",
+ RelationGetRelationName(rel))));
+ break;
+ case RELKIND_FOREIGN_TABLE:
+ /* Perhaps we can support this someday, but not today */
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot lock rows in foreign table \"%s\"",
+ RelationGetRelationName(rel))));
+ break;
+ default:
+ ereport(ERROR,
+ (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ errmsg("cannot lock rows in relation \"%s\"",
+ RelationGetRelationName(rel))));
+ break;
+ }
+ }
+
+ /*
* Initialize ResultRelInfo data for one result relation
*
* Caution: before Postgres 9.1, this function included the relkind checking
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
