From 04e17bae50a73af524731fa11210d5c3f7d8e1f9 Mon Sep 17 00:00:00 2001 From: Simon Riggs Date: Thu, 29 Jul 2010 11:06:34 +0000 Subject: [PATCH] Add explicit regression tests for ALTER TABLE lock levels. Use this to catch a couple of lock level assignments that slipped through manual testing, per Peter Eisentraut. --- src/backend/commands/cluster.c | 8 +- src/backend/commands/tablecmds.c | 12 ++- src/include/commands/cluster.h | 5 +- src/test/regress/expected/alter_table.out | 121 ++++++++++++++++++++++ src/test/regress/sql/alter_table.sql | 77 ++++++++++++++ 5 files changed, 212 insertions(+), 11 deletions(-) diff --git a/src/backend/commands/cluster.c b/src/backend/commands/cluster.c index ce99b55a63..48132b43e5 100644 --- a/src/backend/commands/cluster.c +++ b/src/backend/commands/cluster.c @@ -11,7 +11,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.204 2010/07/25 23:21:21 rhaas Exp $ + * $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.205 2010/07/29 11:06:34 sriggs Exp $ * *------------------------------------------------------------------------- */ @@ -376,7 +376,7 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose, /* Check heap and index are valid to cluster on */ if (OidIsValid(indexOid)) - check_index_is_clusterable(OldHeap, indexOid, recheck); + check_index_is_clusterable(OldHeap, indexOid, recheck, AccessExclusiveLock); /* Log what we're doing (this could use more effort) */ if (OidIsValid(indexOid)) @@ -405,11 +405,11 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose, * definition can't change under us. */ void -check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck) +check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck, LOCKMODE lockmode) { Relation OldIndex; - OldIndex = index_open(indexOid, AccessExclusiveLock); + OldIndex = index_open(indexOid, lockmode); /* * Check that index is in fact an index on the given relation diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 1496b6eb15..431b06568b 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.335 2010/07/28 05:22:24 sriggs Exp $ + * $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.336 2010/07/29 11:06:34 sriggs Exp $ * *------------------------------------------------------------------------- */ @@ -2550,6 +2550,8 @@ AlterTableGetLockLevel(List *cmds) case AT_DropCluster: case AT_SetRelOptions: case AT_ResetRelOptions: + case AT_SetOptions: + case AT_ResetOptions: case AT_SetStorage: cmd_lockmode = ShareUpdateExclusiveLock; break; @@ -2669,19 +2671,19 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd, ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode); /* Performs own permission checks */ ATPrepSetStatistics(rel, cmd->name, cmd->def, lockmode); - pass = AT_PASS_COL_ATTRS; + pass = AT_PASS_MISC; break; case AT_SetOptions: /* ALTER COLUMN SET ( options ) */ case AT_ResetOptions: /* ALTER COLUMN RESET ( options ) */ ATSimplePermissionsRelationOrIndex(rel); /* This command never recurses */ - pass = AT_PASS_COL_ATTRS; + pass = AT_PASS_MISC; break; case AT_SetStorage: /* ALTER COLUMN SET STORAGE */ ATSimplePermissions(rel, false); ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode); /* No command-specific prep needed */ - pass = AT_PASS_COL_ATTRS; + pass = AT_PASS_MISC; break; case AT_DropColumn: /* DROP COLUMN */ ATSimplePermissions(rel, false); @@ -6906,7 +6908,7 @@ ATExecClusterOn(Relation rel, const char *indexName, LOCKMODE lockmode) indexName, RelationGetRelationName(rel)))); /* Check index is valid to cluster on */ - check_index_is_clusterable(rel, indexOid, false); + check_index_is_clusterable(rel, indexOid, false, lockmode); /* And do the work */ mark_index_clustered(rel, indexOid); diff --git a/src/include/commands/cluster.h b/src/include/commands/cluster.h index ed3853af24..74b951f118 100644 --- a/src/include/commands/cluster.h +++ b/src/include/commands/cluster.h @@ -6,7 +6,7 @@ * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group * Portions Copyright (c) 1994-5, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/commands/cluster.h,v 1.41 2010/02/26 02:01:24 momjian Exp $ + * $PostgreSQL: pgsql/src/include/commands/cluster.h,v 1.42 2010/07/29 11:06:34 sriggs Exp $ * *------------------------------------------------------------------------- */ @@ -14,6 +14,7 @@ #define CLUSTER_H #include "nodes/parsenodes.h" +#include "storage/lock.h" #include "utils/relcache.h" @@ -21,7 +22,7 @@ extern void cluster(ClusterStmt *stmt, bool isTopLevel); extern void cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose, int freeze_min_age, int freeze_table_age); extern void check_index_is_clusterable(Relation OldHeap, Oid indexOid, - bool recheck); + bool recheck, LOCKMODE lockmode); extern void mark_index_clustered(Relation rel, Oid indexOid); extern Oid make_new_heap(Oid OIDOldHeap, Oid NewTableSpace); diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 5aff44f23a..83e24fd8c9 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -1473,6 +1473,127 @@ select * from another; drop table another; -- +-- lock levels +-- +drop type lockmodes; +ERROR: type "lockmodes" does not exist +create type lockmodes as enum ( + 'AccessShareLock' +,'RowShareLock' +,'RowExclusiveLock' +,'ShareUpdateExclusiveLock' +,'ShareLock' +,'ShareRowExclusiveLock' +,'ExclusiveLock' +,'AccessExclusiveLock' +); +drop view my_locks; +ERROR: view "my_locks" does not exist +create or replace view my_locks as +select case when c.relname like 'pg_toast%' then 'pg_toast' else c.relname end, max(mode::lockmodes) as max_lockmode +from pg_locks l join pg_class c on l.relation = c.oid +where virtualtransaction = ( + select virtualtransaction + from pg_locks + where transactionid = txid_current()::integer) +and locktype = 'relation' +and relnamespace != (select oid from pg_namespace where nspname = 'pg_catalog') +and c.relname != 'my_locks' +group by c.relname; +create table alterlock (f1 int primary key, f2 text); +NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "alterlock_pkey" for table "alterlock" +-- share update exclusive +begin; alter table alterlock alter column f2 set statistics 150; +select * from my_locks order by 1; + relname | max_lockmode +-----------+-------------------------- + alterlock | ShareUpdateExclusiveLock +(1 row) + +rollback; +begin; alter table alterlock cluster on alterlock_pkey; +select * from my_locks order by 1; + relname | max_lockmode +----------------+-------------------------- + alterlock | ShareUpdateExclusiveLock + alterlock_pkey | ShareUpdateExclusiveLock +(2 rows) + +commit; +begin; alter table alterlock set without cluster; +select * from my_locks order by 1; + relname | max_lockmode +-----------+-------------------------- + alterlock | ShareUpdateExclusiveLock +(1 row) + +commit; +begin; alter table alterlock set (fillfactor = 100); +select * from my_locks order by 1; + relname | max_lockmode +-----------+-------------------------- + alterlock | ShareUpdateExclusiveLock + pg_toast | ShareUpdateExclusiveLock +(2 rows) + +commit; +begin; alter table alterlock reset (fillfactor); +select * from my_locks order by 1; + relname | max_lockmode +-----------+-------------------------- + alterlock | ShareUpdateExclusiveLock + pg_toast | ShareUpdateExclusiveLock +(2 rows) + +commit; +begin; alter table alterlock set (toast.autovacuum_enabled = off); +select * from my_locks order by 1; + relname | max_lockmode +-----------+-------------------------- + alterlock | ShareUpdateExclusiveLock + pg_toast | ShareUpdateExclusiveLock +(2 rows) + +commit; +begin; alter table alterlock set (autovacuum_enabled = off); +select * from my_locks order by 1; + relname | max_lockmode +-----------+-------------------------- + alterlock | ShareUpdateExclusiveLock + pg_toast | ShareUpdateExclusiveLock +(2 rows) + +commit; +begin; alter table alterlock alter column f2 set (n_distinct = 1); +select * from my_locks order by 1; + relname | max_lockmode +-----------+-------------------------- + alterlock | ShareUpdateExclusiveLock +(1 row) + +rollback; +begin; alter table alterlock alter column f2 set storage extended; +select * from my_locks order by 1; + relname | max_lockmode +-----------+-------------------------- + alterlock | ShareUpdateExclusiveLock +(1 row) + +rollback; +-- share row exclusive +begin; alter table alterlock alter column f2 set default 'x'; +select * from my_locks order by 1; + relname | max_lockmode +-----------+----------------------- + alterlock | ShareRowExclusiveLock +(1 row) + +rollback; +-- cleanup +drop table alterlock; +drop view my_locks; +drop type lockmodes; +-- -- alter function -- create function test_strict(text) returns text as diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 82c2e4ee01..760670cd06 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1090,6 +1090,83 @@ select * from another; drop table another; +-- +-- lock levels +-- +drop type lockmodes; +create type lockmodes as enum ( + 'AccessShareLock' +,'RowShareLock' +,'RowExclusiveLock' +,'ShareUpdateExclusiveLock' +,'ShareLock' +,'ShareRowExclusiveLock' +,'ExclusiveLock' +,'AccessExclusiveLock' +); + +drop view my_locks; +create or replace view my_locks as +select case when c.relname like 'pg_toast%' then 'pg_toast' else c.relname end, max(mode::lockmodes) as max_lockmode +from pg_locks l join pg_class c on l.relation = c.oid +where virtualtransaction = ( + select virtualtransaction + from pg_locks + where transactionid = txid_current()::integer) +and locktype = 'relation' +and relnamespace != (select oid from pg_namespace where nspname = 'pg_catalog') +and c.relname != 'my_locks' +group by c.relname; + +create table alterlock (f1 int primary key, f2 text); + +-- share update exclusive +begin; alter table alterlock alter column f2 set statistics 150; +select * from my_locks order by 1; +rollback; + +begin; alter table alterlock cluster on alterlock_pkey; +select * from my_locks order by 1; +commit; + +begin; alter table alterlock set without cluster; +select * from my_locks order by 1; +commit; + +begin; alter table alterlock set (fillfactor = 100); +select * from my_locks order by 1; +commit; + +begin; alter table alterlock reset (fillfactor); +select * from my_locks order by 1; +commit; + +begin; alter table alterlock set (toast.autovacuum_enabled = off); +select * from my_locks order by 1; +commit; + +begin; alter table alterlock set (autovacuum_enabled = off); +select * from my_locks order by 1; +commit; + +begin; alter table alterlock alter column f2 set (n_distinct = 1); +select * from my_locks order by 1; +rollback; + +begin; alter table alterlock alter column f2 set storage extended; +select * from my_locks order by 1; +rollback; + +-- share row exclusive +begin; alter table alterlock alter column f2 set default 'x'; +select * from my_locks order by 1; +rollback; + +-- cleanup +drop table alterlock; +drop view my_locks; +drop type lockmodes; + -- -- alter function -- -- 2.40.0