From eeda7f6338095701cfe1ba3da37070508efe019e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Fri, 6 Nov 2020 16:17:56 -0500 Subject: [PATCH] Revert "Accept relations of any kind in LOCK TABLE". Revert 59ab4ac32, as well as the followup fix 33862cb9c, in all branches. We need to think a bit harder about what the behavior of LOCK TABLE on views should be, and there's no time for that before next week's releases. We'll take another crack at this later. Discussion: https://postgr.es/m/16703-e348f58aab3cf6cc@postgresql.org --- doc/src/sgml/ref/lock.sgml | 15 ++++++--------- src/backend/commands/lockcmds.c | 28 +++++++++++++++++++--------- src/test/regress/expected/lock.out | 11 ----------- src/test/regress/sql/lock.sql | 13 ------------- 4 files changed, 25 insertions(+), 42 deletions(-) diff --git a/doc/src/sgml/ref/lock.sgml b/doc/src/sgml/ref/lock.sgml index 37881f25ac..4cdfae2279 100644 --- a/doc/src/sgml/ref/lock.sgml +++ b/doc/src/sgml/ref/lock.sgml @@ -16,7 +16,7 @@ PostgreSQL documentation LOCK - lock a named relation (table, etc) + lock a table @@ -34,9 +34,7 @@ LOCK [ TABLE ] [ ONLY ] name [ * ] Description - LOCK TABLE obtains a table-level lock on a - relation (table, partitioned table, foreign table, view, - materialized view, index, composite type, sequence), waiting + LOCK TABLE obtains a table-level lock, waiting if necessary for any conflicting locks to be released. If NOWAIT is specified, LOCK TABLE does not wait to acquire the desired lock: if it @@ -117,18 +115,17 @@ LOCK [ TABLE ] [ ONLY ] name [ * ] name - The name (optionally schema-qualified) of an existing relation to - lock. If ONLY is specified before a table name, only that + The name (optionally schema-qualified) of an existing table to + lock. If ONLY is specified before the table name, only that table is locked. If ONLY is not specified, the table and all its descendant tables (if any) are locked. Optionally, * can be specified after the table name to explicitly indicate that - descendant tables are included. When locking a view, all relations appearing - in the view definition are locked, regardless of ONLY. + descendant tables are included. The command LOCK TABLE a, b; is equivalent to - LOCK TABLE a; LOCK TABLE b;. The relations are locked + LOCK TABLE a; LOCK TABLE b;. The tables are locked one-by-one in the order specified in the LOCK TABLE command. diff --git a/src/backend/commands/lockcmds.c b/src/backend/commands/lockcmds.c index 0cd762ea5b..2846ef3b7b 100644 --- a/src/backend/commands/lockcmds.c +++ b/src/backend/commands/lockcmds.c @@ -84,6 +84,14 @@ RangeVarCallbackForLockTable(const RangeVar *rv, Oid relid, Oid oldrelid, return; /* woops, concurrently dropped; no permissions * check */ + /* Currently, we only allow plain tables or views to be locked */ + if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE && + relkind != RELKIND_VIEW) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("\"%s\" is not a table or view", + rv->relname))); + /* * Make note if a temporary relation has been accessed in this * transaction. @@ -179,13 +187,11 @@ LockViewRecurse_walker(Node *node, LockViewRecurse_context *context) foreach(rtable, query->rtable) { RangeTblEntry *rte = lfirst(rtable); - Oid relid; AclResult aclresult; - /* ignore all non-relation RTEs */ - if (rte->rtekind != RTE_RELATION) - continue; - relid = rte->relid; + Oid relid = rte->relid; + char relkind = rte->relkind; + char *relname = get_rel_name(relid); /* * The OLD and NEW placeholder entries in the view's rtable are @@ -196,6 +202,11 @@ LockViewRecurse_walker(Node *node, LockViewRecurse_context *context) strcmp(rte->eref->aliasname, "new") == 0)) continue; + /* Currently, we only allow plain tables or views to be locked. */ + if (relkind != RELKIND_RELATION && relkind != RELKIND_PARTITIONED_TABLE && + relkind != RELKIND_VIEW) + continue; + /* * We might be dealing with a self-referential view. If so, we * can just stop recursing, since we already locked it. @@ -206,8 +217,7 @@ LockViewRecurse_walker(Node *node, LockViewRecurse_context *context) /* Check permissions with the view owner's privilege. */ aclresult = LockTableAclCheck(relid, context->lockmode, context->viewowner); if (aclresult != ACLCHECK_OK) - aclcheck_error(aclresult, get_relkind_objtype(rte->relkind), - get_rel_name(relid)); + aclcheck_error(aclresult, get_relkind_objtype(relkind), relname); /* We have enough rights to lock the relation; do so. */ if (!context->nowait) @@ -216,9 +226,9 @@ LockViewRecurse_walker(Node *node, LockViewRecurse_context *context) ereport(ERROR, (errcode(ERRCODE_LOCK_NOT_AVAILABLE), errmsg("could not obtain lock on relation \"%s\"", - get_rel_name(relid)))); + relname))); - if (rte->relkind == RELKIND_VIEW) + if (relkind == RELKIND_VIEW) LockViewRecurse(relid, context->lockmode, context->nowait, context->ancestor_views); else if (rte->inh) diff --git a/src/test/regress/expected/lock.out b/src/test/regress/expected/lock.out index f39280a4fa..d43bee0c56 100644 --- a/src/test/regress/expected/lock.out +++ b/src/test/regress/expected/lock.out @@ -12,9 +12,6 @@ CREATE VIEW lock_view3 AS SELECT * from lock_view2; CREATE VIEW lock_view4 AS SELECT (select a from lock_tbl1a limit 1) from lock_tbl1; CREATE VIEW lock_view5 AS SELECT * from lock_tbl1 where a in (select * from lock_tbl1a); CREATE VIEW lock_view6 AS SELECT * from (select * from lock_tbl1) sub; -CREATE MATERIALIZED VIEW lock_mv1 AS SELECT * FROM lock_view6; -CREATE INDEX lock_mvi1 ON lock_mv1 (a); -CREATE SEQUENCE lock_seq; CREATE ROLE regress_rol_lock1; ALTER ROLE regress_rol_lock1 SET search_path = lock_schema1; GRANT USAGE ON SCHEMA lock_schema1 TO regress_rol_lock1; @@ -155,16 +152,9 @@ BEGIN; LOCK TABLE ONLY lock_tbl1; ROLLBACK; RESET ROLE; --- Lock other relations -BEGIN TRANSACTION; -LOCK TABLE lock_mv1; -LOCK TABLE lock_mvi1; -LOCK TABLE lock_seq; -ROLLBACK; -- -- Clean up -- -DROP MATERIALIZED VIEW lock_mv1; DROP VIEW lock_view7; DROP VIEW lock_view6; DROP VIEW lock_view5; @@ -176,7 +166,6 @@ DROP TABLE lock_tbl3; DROP TABLE lock_tbl2; DROP TABLE lock_tbl1; DROP TABLE lock_tbl1a; -DROP SEQUENCE lock_seq; DROP SCHEMA lock_schema1 CASCADE; DROP ROLE regress_rol_lock1; -- atomic ops tests diff --git a/src/test/regress/sql/lock.sql b/src/test/regress/sql/lock.sql index 4f032f1c2e..05bdb8ad4c 100644 --- a/src/test/regress/sql/lock.sql +++ b/src/test/regress/sql/lock.sql @@ -13,9 +13,6 @@ CREATE VIEW lock_view3 AS SELECT * from lock_view2; CREATE VIEW lock_view4 AS SELECT (select a from lock_tbl1a limit 1) from lock_tbl1; CREATE VIEW lock_view5 AS SELECT * from lock_tbl1 where a in (select * from lock_tbl1a); CREATE VIEW lock_view6 AS SELECT * from (select * from lock_tbl1) sub; -CREATE MATERIALIZED VIEW lock_mv1 AS SELECT * FROM lock_view6; -CREATE INDEX lock_mvi1 ON lock_mv1 (a); -CREATE SEQUENCE lock_seq; CREATE ROLE regress_rol_lock1; ALTER ROLE regress_rol_lock1 SET search_path = lock_schema1; GRANT USAGE ON SCHEMA lock_schema1 TO regress_rol_lock1; @@ -120,18 +117,9 @@ LOCK TABLE ONLY lock_tbl1; ROLLBACK; RESET ROLE; --- Lock other relations -BEGIN TRANSACTION; -LOCK TABLE lock_mv1; -LOCK TABLE lock_mvi1; -LOCK TABLE lock_seq; -ROLLBACK; - - -- -- Clean up -- -DROP MATERIALIZED VIEW lock_mv1; DROP VIEW lock_view7; DROP VIEW lock_view6; DROP VIEW lock_view5; @@ -142,7 +130,6 @@ DROP TABLE lock_tbl3; DROP TABLE lock_tbl2; DROP TABLE lock_tbl1; DROP TABLE lock_tbl1a; -DROP SEQUENCE lock_seq; DROP SCHEMA lock_schema1 CASCADE; DROP ROLE regress_rol_lock1;