Skip to content

Commit 04e17ba

Browse files
simonat2ndQuadrantsimonat2ndQuadrant
committed
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.
1 parent f0790a6 commit 04e17ba

File tree

5 files changed

+212
-11
lines changed

5 files changed

+212
-11
lines changed

‎src/backend/commands/cluster.c

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
*
1212
*
1313
* IDENTIFICATION
14-
* $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.204 2010/07/25 23:21:21 rhaas Exp $
14+
* $PostgreSQL: pgsql/src/backend/commands/cluster.c,v 1.205 2010/07/29 11:06:34 sriggs Exp $
1515
*
1616
*-------------------------------------------------------------------------
1717
*/
@@ -376,7 +376,7 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose,
376376

377377
/* Check heap and index are valid to cluster on */
378378
if (OidIsValid(indexOid))
379-
check_index_is_clusterable(OldHeap, indexOid, recheck);
379+
check_index_is_clusterable(OldHeap, indexOid, recheck, AccessExclusiveLock);
380380

381381
/* Log what we're doing (this could use more effort) */
382382
if (OidIsValid(indexOid))
@@ -405,11 +405,11 @@ cluster_rel(Oid tableOid, Oid indexOid, bool recheck, bool verbose,
405405
* definition can't change under us.
406406
*/
407407
void
408-
check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck)
408+
check_index_is_clusterable(Relation OldHeap, Oid indexOid, bool recheck, LOCKMODE lockmode)
409409
{
410410
Relation OldIndex;
411411

412-
OldIndex = index_open(indexOid, AccessExclusiveLock);
412+
OldIndex = index_open(indexOid, lockmode);
413413

414414
/*
415415
* Check that index is in fact an index on the given relation

‎src/backend/commands/tablecmds.c

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.335 2010/07/28 05:22:24 sriggs Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.336 2010/07/29 11:06:34 sriggs Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -2550,6 +2550,8 @@ AlterTableGetLockLevel(List *cmds)
25502550
case AT_DropCluster:
25512551
case AT_SetRelOptions:
25522552
case AT_ResetRelOptions:
2553+
case AT_SetOptions:
2554+
case AT_ResetOptions:
25532555
case AT_SetStorage:
25542556
cmd_lockmode = ShareUpdateExclusiveLock;
25552557
break;
@@ -2669,19 +2671,19 @@ ATPrepCmd(List **wqueue, Relation rel, AlterTableCmd *cmd,
26692671
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
26702672
/* Performs own permission checks */
26712673
ATPrepSetStatistics(rel, cmd->name, cmd->def, lockmode);
2672-
pass = AT_PASS_COL_ATTRS;
2674+
pass = AT_PASS_MISC;
26732675
break;
26742676
case AT_SetOptions: /* ALTER COLUMN SET ( options ) */
26752677
case AT_ResetOptions: /* ALTER COLUMN RESET ( options ) */
26762678
ATSimplePermissionsRelationOrIndex(rel);
26772679
/* This command never recurses */
2678-
pass = AT_PASS_COL_ATTRS;
2680+
pass = AT_PASS_MISC;
26792681
break;
26802682
case AT_SetStorage: /* ALTER COLUMN SET STORAGE */
26812683
ATSimplePermissions(rel, false);
26822684
ATSimpleRecursion(wqueue, rel, cmd, recurse, lockmode);
26832685
/* No command-specific prep needed */
2684-
pass = AT_PASS_COL_ATTRS;
2686+
pass = AT_PASS_MISC;
26852687
break;
26862688
case AT_DropColumn: /* DROP COLUMN */
26872689
ATSimplePermissions(rel, false);
@@ -6906,7 +6908,7 @@ ATExecClusterOn(Relation rel, const char *indexName, LOCKMODE lockmode)
69066908
indexName, RelationGetRelationName(rel))));
69076909

69086910
/* Check index is valid to cluster on */
6909-
check_index_is_clusterable(rel, indexOid, false);
6911+
check_index_is_clusterable(rel, indexOid, false, lockmode);
69106912

69116913
/* And do the work */
69126914
mark_index_clustered(rel, indexOid);

‎src/include/commands/cluster.h

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,22 +6,23 @@
66
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
77
* Portions Copyright (c) 1994-5, Regents of the University of California
88
*
9-
* $PostgreSQL: pgsql/src/include/commands/cluster.h,v 1.41 2010/02/26 02:01:24 momjian Exp $
9+
* $PostgreSQL: pgsql/src/include/commands/cluster.h,v 1.42 2010/07/29 11:06:34 sriggs Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
1313
#ifndef CLUSTER_H
1414
#define CLUSTER_H
1515

1616
#include "nodes/parsenodes.h"
17+
#include "storage/lock.h"
1718
#include "utils/relcache.h"
1819

1920

2021
extern void cluster(ClusterStmt *stmt, bool isTopLevel);
2122
extern void cluster_rel(Oid tableOid, Oid indexOid, bool recheck,
2223
bool verbose, int freeze_min_age, int freeze_table_age);
2324
extern void check_index_is_clusterable(Relation OldHeap, Oid indexOid,
24-
bool recheck);
25+
bool recheck, LOCKMODE lockmode);
2526
extern void mark_index_clustered(Relation rel, Oid indexOid);
2627

2728
extern Oid make_new_heap(Oid OIDOldHeap, Oid NewTableSpace);

‎src/test/regress/expected/alter_table.out

Lines changed: 121 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1473,6 +1473,127 @@ select * from another;
14731473

14741474
drop table another;
14751475
--
1476+
-- lock levels
1477+
--
1478+
drop type lockmodes;
1479+
ERROR: type "lockmodes" does not exist
1480+
create type lockmodes as enum (
1481+
'AccessShareLock'
1482+
,'RowShareLock'
1483+
,'RowExclusiveLock'
1484+
,'ShareUpdateExclusiveLock'
1485+
,'ShareLock'
1486+
,'ShareRowExclusiveLock'
1487+
,'ExclusiveLock'
1488+
,'AccessExclusiveLock'
1489+
);
1490+
drop view my_locks;
1491+
ERROR: view "my_locks" does not exist
1492+
create or replace view my_locks as
1493+
select case when c.relname like 'pg_toast%' then 'pg_toast' else c.relname end, max(mode::lockmodes) as max_lockmode
1494+
from pg_locks l join pg_class c on l.relation = c.oid
1495+
where virtualtransaction = (
1496+
select virtualtransaction
1497+
from pg_locks
1498+
where transactionid = txid_current()::integer)
1499+
and locktype = 'relation'
1500+
and relnamespace != (select oid from pg_namespace where nspname = 'pg_catalog')
1501+
and c.relname != 'my_locks'
1502+
group by c.relname;
1503+
create table alterlock (f1 int primary key, f2 text);
1504+
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "alterlock_pkey" for table "alterlock"
1505+
-- share update exclusive
1506+
begin; alter table alterlock alter column f2 set statistics 150;
1507+
select * from my_locks order by 1;
1508+
relname | max_lockmode
1509+
-----------+--------------------------
1510+
alterlock | ShareUpdateExclusiveLock
1511+
(1 row)
1512+
1513+
rollback;
1514+
begin; alter table alterlock cluster on alterlock_pkey;
1515+
select * from my_locks order by 1;
1516+
relname | max_lockmode
1517+
----------------+--------------------------
1518+
alterlock | ShareUpdateExclusiveLock
1519+
alterlock_pkey | ShareUpdateExclusiveLock
1520+
(2 rows)
1521+
1522+
commit;
1523+
begin; alter table alterlock set without cluster;
1524+
select * from my_locks order by 1;
1525+
relname | max_lockmode
1526+
-----------+--------------------------
1527+
alterlock | ShareUpdateExclusiveLock
1528+
(1 row)
1529+
1530+
commit;
1531+
begin; alter table alterlock set (fillfactor = 100);
1532+
select * from my_locks order by 1;
1533+
relname | max_lockmode
1534+
-----------+--------------------------
1535+
alterlock | ShareUpdateExclusiveLock
1536+
pg_toast | ShareUpdateExclusiveLock
1537+
(2 rows)
1538+
1539+
commit;
1540+
begin; alter table alterlock reset (fillfactor);
1541+
select * from my_locks order by 1;
1542+
relname | max_lockmode
1543+
-----------+--------------------------
1544+
alterlock | ShareUpdateExclusiveLock
1545+
pg_toast | ShareUpdateExclusiveLock
1546+
(2 rows)
1547+
1548+
commit;
1549+
begin; alter table alterlock set (toast.autovacuum_enabled = off);
1550+
select * from my_locks order by 1;
1551+
relname | max_lockmode
1552+
-----------+--------------------------
1553+
alterlock | ShareUpdateExclusiveLock
1554+
pg_toast | ShareUpdateExclusiveLock
1555+
(2 rows)
1556+
1557+
commit;
1558+
begin; alter table alterlock set (autovacuum_enabled = off);
1559+
select * from my_locks order by 1;
1560+
relname | max_lockmode
1561+
-----------+--------------------------
1562+
alterlock | ShareUpdateExclusiveLock
1563+
pg_toast | ShareUpdateExclusiveLock
1564+
(2 rows)
1565+
1566+
commit;
1567+
begin; alter table alterlock alter column f2 set (n_distinct = 1);
1568+
select * from my_locks order by 1;
1569+
relname | max_lockmode
1570+
-----------+--------------------------
1571+
alterlock | ShareUpdateExclusiveLock
1572+
(1 row)
1573+
1574+
rollback;
1575+
begin; alter table alterlock alter column f2 set storage extended;
1576+
select * from my_locks order by 1;
1577+
relname | max_lockmode
1578+
-----------+--------------------------
1579+
alterlock | ShareUpdateExclusiveLock
1580+
(1 row)
1581+
1582+
rollback;
1583+
-- share row exclusive
1584+
begin; alter table alterlock alter column f2 set default 'x';
1585+
select * from my_locks order by 1;
1586+
relname | max_lockmode
1587+
-----------+-----------------------
1588+
alterlock | ShareRowExclusiveLock
1589+
(1 row)
1590+
1591+
rollback;
1592+
-- cleanup
1593+
drop table alterlock;
1594+
drop view my_locks;
1595+
drop type lockmodes;
1596+
--
14761597
-- alter function
14771598
--
14781599
create function test_strict(text) returns text as

‎src/test/regress/sql/alter_table.sql

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1090,6 +1090,83 @@ select * from another;
10901090

10911091
drop table another;
10921092

1093+
--
1094+
-- lock levels
1095+
--
1096+
drop type lockmodes;
1097+
create type lockmodes as enum (
1098+
'AccessShareLock'
1099+
,'RowShareLock'
1100+
,'RowExclusiveLock'
1101+
,'ShareUpdateExclusiveLock'
1102+
,'ShareLock'
1103+
,'ShareRowExclusiveLock'
1104+
,'ExclusiveLock'
1105+
,'AccessExclusiveLock'
1106+
);
1107+
1108+
drop view my_locks;
1109+
create or replace view my_locks as
1110+
select case when c.relname like 'pg_toast%' then 'pg_toast' else c.relname end, max(mode::lockmodes) as max_lockmode
1111+
from pg_locks l join pg_class c on l.relation = c.oid
1112+
where virtualtransaction = (
1113+
select virtualtransaction
1114+
from pg_locks
1115+
where transactionid = txid_current()::integer)
1116+
and locktype = 'relation'
1117+
and relnamespace != (select oid from pg_namespace where nspname = 'pg_catalog')
1118+
and c.relname != 'my_locks'
1119+
group by c.relname;
1120+
1121+
create table alterlock (f1 int primary key, f2 text);
1122+
1123+
-- share update exclusive
1124+
begin; alter table alterlock alter column f2 set statistics 150;
1125+
select * from my_locks order by 1;
1126+
rollback;
1127+
1128+
begin; alter table alterlock cluster on alterlock_pkey;
1129+
select * from my_locks order by 1;
1130+
commit;
1131+
1132+
begin; alter table alterlock set without cluster;
1133+
select * from my_locks order by 1;
1134+
commit;
1135+
1136+
begin; alter table alterlock set (fillfactor = 100);
1137+
select * from my_locks order by 1;
1138+
commit;
1139+
1140+
begin; alter table alterlock reset (fillfactor);
1141+
select * from my_locks order by 1;
1142+
commit;
1143+
1144+
begin; alter table alterlock set (toast.autovacuum_enabled = off);
1145+
select * from my_locks order by 1;
1146+
commit;
1147+
1148+
begin; alter table alterlock set (autovacuum_enabled = off);
1149+
select * from my_locks order by 1;
1150+
commit;
1151+
1152+
begin; alter table alterlock alter column f2 set (n_distinct = 1);
1153+
select * from my_locks order by 1;
1154+
rollback;
1155+
1156+
begin; alter table alterlock alter column f2 set storage extended;
1157+
select * from my_locks order by 1;
1158+
rollback;
1159+
1160+
-- share row exclusive
1161+
begin; alter table alterlock alter column f2 set default 'x';
1162+
select * from my_locks order by 1;
1163+
rollback;
1164+
1165+
-- cleanup
1166+
drop table alterlock;
1167+
drop view my_locks;
1168+
drop type lockmodes;
1169+
10931170
--
10941171
-- alter function
10951172
--

0 commit comments

Comments
 (0)