Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commitf841ceb

Browse files
committed
Improve TRUNCATE by avoiding early lock queue
A caller of TRUNCATE could previously queue for an access exclusive lockon a relation it may not have permission to truncate, potentiallyinterfering with users authorized to work on it. This can be veryintrusive depending on the lock attempted to be taken. For example,pg_authid could be blocked, preventing any authentication attempt tohappen on a PostgreSQL instance.This commit fixes the case of TRUNCATE so as RangeVarGetRelidExtended isused with a callback doing the necessary ACL checks at an earlier stage,avoiding lock queuing issues, so as an immediate failure happens forunprivileged users instead of waiting on a lock that would not betaken.This is rather similar to the type of work done incbe24a6 for CLUSTER,and the code of TRUNCATE is this time refactored so as there is nouser-facing changes. As the commit for CLUSTER, no back-patch is done.Reported-by: Lloyd Albin, Jeremy SchneiderAuthor: Michael PaquierReviewed by: Nathan Bossart, Kyotaro HoriguchiDiscussion:https://postgr.es/m/152512087100.19803.12733865831237526317@wrigleys.postgresql.orgDiscussion:https://postgr.es/m/20180806165816.GA19883@paquier.xyz
1 parent2b13702 commitf841ceb

File tree

4 files changed

+204
-18
lines changed

4 files changed

+204
-18
lines changed

‎src/backend/commands/tablecmds.c‎

Lines changed: 66 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -300,7 +300,10 @@ struct DropRelationCallbackState
300300
#definechild_dependency_type(child_is_partition)\
301301
((child_is_partition) ? DEPENDENCY_AUTO : DEPENDENCY_NORMAL)
302302

303-
staticvoidtruncate_check_rel(Relationrel);
303+
staticvoidtruncate_check_rel(Oidrelid,Form_pg_classreltuple);
304+
staticvoidtruncate_check_activity(Relationrel);
305+
staticvoidRangeVarCallbackForTruncate(constRangeVar*relation,
306+
OidrelId,OidoldRelId,void*arg);
304307
staticList*MergeAttributes(List*schema,List*supers,charrelpersistence,
305308
boolis_partition,List**supOids,List**supconstr,
306309
int*supOidCount);
@@ -1336,15 +1339,26 @@ ExecuteTruncate(TruncateStmt *stmt)
13361339
boolrecurse=rv->inh;
13371340
Oidmyrelid;
13381341

1339-
rel=heap_openrv(rv,AccessExclusiveLock);
1340-
myrelid=RelationGetRelid(rel);
1342+
myrelid=RangeVarGetRelidExtended(rv,AccessExclusiveLock,
1343+
0,RangeVarCallbackForTruncate,
1344+
NULL);
1345+
1346+
/* open the relation, we already hold a lock on it */
1347+
rel=heap_open(myrelid,NoLock);
1348+
13411349
/* don't throw error for "TRUNCATE foo, foo" */
13421350
if (list_member_oid(relids,myrelid))
13431351
{
13441352
heap_close(rel,AccessExclusiveLock);
13451353
continue;
13461354
}
1347-
truncate_check_rel(rel);
1355+
1356+
/*
1357+
* RangeVarGetRelidExtended() has done most checks with its callback,
1358+
* but other checks with the now-opened Relation remain.
1359+
*/
1360+
truncate_check_activity(rel);
1361+
13481362
rels=lappend(rels,rel);
13491363
relids=lappend_oid(relids,myrelid);
13501364
/* Log this relation only if needed for logical decoding */
@@ -1367,7 +1381,9 @@ ExecuteTruncate(TruncateStmt *stmt)
13671381

13681382
/* find_all_inheritors already got lock */
13691383
rel=heap_open(childrelid,NoLock);
1370-
truncate_check_rel(rel);
1384+
truncate_check_rel(RelationGetRelid(rel),rel->rd_rel);
1385+
truncate_check_activity(rel);
1386+
13711387
rels=lappend(rels,rel);
13721388
relids=lappend_oid(relids,childrelid);
13731389
/* Log this relation only if needed for logical decoding */
@@ -1450,7 +1466,8 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
14501466
ereport(NOTICE,
14511467
(errmsg("truncate cascades to table \"%s\"",
14521468
RelationGetRelationName(rel))));
1453-
truncate_check_rel(rel);
1469+
truncate_check_rel(relid,rel->rd_rel);
1470+
truncate_check_activity(rel);
14541471
rels=lappend(rels,rel);
14551472
relids=lappend_oid(relids,relid);
14561473
/* Log this relation only if needed for logical decoding */
@@ -1700,38 +1717,47 @@ ExecuteTruncateGuts(List *explicit_rels, List *relids, List *relids_logged,
17001717
}
17011718

17021719
/*
1703-
* Check that a given rel is safe to truncate. Subroutine for ExecuteTruncate
1720+
* Check that a given relation is safe to truncate. Subroutine for
1721+
* ExecuteTruncate() and RangeVarCallbackForTruncate().
17041722
*/
17051723
staticvoid
1706-
truncate_check_rel(Relationrel)
1724+
truncate_check_rel(Oidrelid,Form_pg_classreltuple)
17071725
{
17081726
AclResultaclresult;
1727+
char*relname=NameStr(reltuple->relname);
17091728

17101729
/*
17111730
* Only allow truncate on regular tables and partitioned tables (although,
17121731
* the latter are only being included here for the following checks; no
17131732
* physical truncation will occur in their case.)
17141733
*/
1715-
if (rel->rd_rel->relkind!=RELKIND_RELATION&&
1716-
rel->rd_rel->relkind!=RELKIND_PARTITIONED_TABLE)
1734+
if (reltuple->relkind!=RELKIND_RELATION&&
1735+
reltuple->relkind!=RELKIND_PARTITIONED_TABLE)
17171736
ereport(ERROR,
17181737
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
1719-
errmsg("\"%s\" is not a table",
1720-
RelationGetRelationName(rel))));
1738+
errmsg("\"%s\" is not a table",relname)));
17211739

17221740
/* Permissions checks */
1723-
aclresult=pg_class_aclcheck(RelationGetRelid(rel),GetUserId(),
1724-
ACL_TRUNCATE);
1741+
aclresult=pg_class_aclcheck(relid,GetUserId(),ACL_TRUNCATE);
17251742
if (aclresult!=ACLCHECK_OK)
1726-
aclcheck_error(aclresult,get_relkind_objtype(rel->rd_rel->relkind),
1727-
RelationGetRelationName(rel));
1743+
aclcheck_error(aclresult,get_relkind_objtype(reltuple->relkind),
1744+
relname);
17281745

1729-
if (!allowSystemTableMods&&IsSystemRelation(rel))
1746+
if (!allowSystemTableMods&&IsSystemClass(relid,reltuple))
17301747
ereport(ERROR,
17311748
(errcode(ERRCODE_INSUFFICIENT_PRIVILEGE),
17321749
errmsg("permission denied: \"%s\" is a system catalog",
1733-
RelationGetRelationName(rel))));
1750+
relname)));
1751+
}
17341752

1753+
/*
1754+
* Set of extra sanity checks to check if a given relation is safe to
1755+
* truncate. This is split with truncate_check_rel() as
1756+
* RangeVarCallbackForTruncate() cannot open a Relation yet.
1757+
*/
1758+
staticvoid
1759+
truncate_check_activity(Relationrel)
1760+
{
17351761
/*
17361762
* Don't allow truncate on temp tables of other backends ... their local
17371763
* buffer manager is not going to cope.
@@ -13419,6 +13445,28 @@ RangeVarCallbackOwnsTable(const RangeVar *relation,
1341913445
aclcheck_error(ACLCHECK_NOT_OWNER,get_relkind_objtype(get_rel_relkind(relId)),relation->relname);
1342013446
}
1342113447

13448+
/*
13449+
* Callback to RangeVarGetRelidExtended() for TRUNCATE processing.
13450+
*/
13451+
staticvoid
13452+
RangeVarCallbackForTruncate(constRangeVar*relation,
13453+
OidrelId,OidoldRelId,void*arg)
13454+
{
13455+
HeapTupletuple;
13456+
13457+
/* Nothing to do if the relation was not found. */
13458+
if (!OidIsValid(relId))
13459+
return;
13460+
13461+
tuple=SearchSysCache1(RELOID,ObjectIdGetDatum(relId));
13462+
if (!HeapTupleIsValid(tuple))/* should not happen */
13463+
elog(ERROR,"cache lookup failed for relation %u",relId);
13464+
13465+
truncate_check_rel(relId, (Form_pg_class)GETSTRUCT(tuple));
13466+
13467+
ReleaseSysCache(tuple);
13468+
}
13469+
1342213470
/*
1342313471
* Callback to RangeVarGetRelidExtended(), similar to
1342413472
* RangeVarCallbackOwnsTable() but without checks on the type of the relation.
Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
Parsed test spec with 2 sessions
2+
3+
starting permutation: s1_begin s1_tab_lookup s2_auth s2_truncate s1_commit s2_reset
4+
step s1_begin: BEGIN;
5+
step s1_tab_lookup: SELECT count(*) >= 0 FROM truncate_tab;
6+
?column?
7+
8+
t
9+
step s2_auth: SET ROLE regress_truncate_conflict;
10+
step s2_truncate: TRUNCATE truncate_tab;
11+
ERROR: permission denied for table truncate_tab
12+
step s1_commit: COMMIT;
13+
step s2_reset: RESET ROLE;
14+
15+
starting permutation: s1_begin s2_auth s2_truncate s1_tab_lookup s1_commit s2_reset
16+
step s1_begin: BEGIN;
17+
step s2_auth: SET ROLE regress_truncate_conflict;
18+
step s2_truncate: TRUNCATE truncate_tab;
19+
ERROR: permission denied for table truncate_tab
20+
step s1_tab_lookup: SELECT count(*) >= 0 FROM truncate_tab;
21+
?column?
22+
23+
t
24+
step s1_commit: COMMIT;
25+
step s2_reset: RESET ROLE;
26+
27+
starting permutation: s1_begin s2_auth s1_tab_lookup s2_truncate s1_commit s2_reset
28+
step s1_begin: BEGIN;
29+
step s2_auth: SET ROLE regress_truncate_conflict;
30+
step s1_tab_lookup: SELECT count(*) >= 0 FROM truncate_tab;
31+
?column?
32+
33+
t
34+
step s2_truncate: TRUNCATE truncate_tab;
35+
ERROR: permission denied for table truncate_tab
36+
step s1_commit: COMMIT;
37+
step s2_reset: RESET ROLE;
38+
39+
starting permutation: s2_auth s2_truncate s1_begin s1_tab_lookup s1_commit s2_reset
40+
step s2_auth: SET ROLE regress_truncate_conflict;
41+
step s2_truncate: TRUNCATE truncate_tab;
42+
ERROR: permission denied for table truncate_tab
43+
step s1_begin: BEGIN;
44+
step s1_tab_lookup: SELECT count(*) >= 0 FROM truncate_tab;
45+
?column?
46+
47+
t
48+
step s1_commit: COMMIT;
49+
step s2_reset: RESET ROLE;
50+
51+
starting permutation: s1_begin s1_tab_lookup s2_grant s2_auth s2_truncate s1_commit s2_reset
52+
step s1_begin: BEGIN;
53+
step s1_tab_lookup: SELECT count(*) >= 0 FROM truncate_tab;
54+
?column?
55+
56+
t
57+
step s2_grant: GRANT TRUNCATE ON truncate_tab TO regress_truncate_conflict;
58+
step s2_auth: SET ROLE regress_truncate_conflict;
59+
step s2_truncate: TRUNCATE truncate_tab; <waiting ...>
60+
step s1_commit: COMMIT;
61+
step s2_truncate: <... completed>
62+
step s2_reset: RESET ROLE;
63+
64+
starting permutation: s1_begin s2_grant s2_auth s2_truncate s1_tab_lookup s1_commit s2_reset
65+
step s1_begin: BEGIN;
66+
step s2_grant: GRANT TRUNCATE ON truncate_tab TO regress_truncate_conflict;
67+
step s2_auth: SET ROLE regress_truncate_conflict;
68+
step s2_truncate: TRUNCATE truncate_tab;
69+
step s1_tab_lookup: SELECT count(*) >= 0 FROM truncate_tab;
70+
?column?
71+
72+
t
73+
step s1_commit: COMMIT;
74+
step s2_reset: RESET ROLE;
75+
76+
starting permutation: s1_begin s2_grant s2_auth s1_tab_lookup s2_truncate s1_commit s2_reset
77+
step s1_begin: BEGIN;
78+
step s2_grant: GRANT TRUNCATE ON truncate_tab TO regress_truncate_conflict;
79+
step s2_auth: SET ROLE regress_truncate_conflict;
80+
step s1_tab_lookup: SELECT count(*) >= 0 FROM truncate_tab;
81+
?column?
82+
83+
t
84+
step s2_truncate: TRUNCATE truncate_tab; <waiting ...>
85+
step s1_commit: COMMIT;
86+
step s2_truncate: <... completed>
87+
step s2_reset: RESET ROLE;
88+
89+
starting permutation: s2_grant s2_auth s2_truncate s1_begin s1_tab_lookup s1_commit s2_reset
90+
step s2_grant: GRANT TRUNCATE ON truncate_tab TO regress_truncate_conflict;
91+
step s2_auth: SET ROLE regress_truncate_conflict;
92+
step s2_truncate: TRUNCATE truncate_tab;
93+
step s1_begin: BEGIN;
94+
step s1_tab_lookup: SELECT count(*) >= 0 FROM truncate_tab;
95+
?column?
96+
97+
t
98+
step s1_commit: COMMIT;
99+
step s2_reset: RESET ROLE;

‎src/test/isolation/isolation_schedule‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,4 @@ test: partition-key-update-2
7676
test: partition-key-update-3
7777
test: partition-key-update-4
7878
test: plpgsql-toast
79+
test: truncate-conflict
Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
# Tests for locking conflicts with TRUNCATE commands.
2+
3+
setup
4+
{
5+
CREATEROLEregress_truncate_conflict;
6+
CREATETABLEtruncate_tab (aint);
7+
}
8+
9+
teardown
10+
{
11+
DROPTABLEtruncate_tab;
12+
DROPROLEregress_truncate_conflict;
13+
}
14+
15+
session"s1"
16+
step"s1_begin" {BEGIN; }
17+
step"s1_tab_lookup" {SELECTcount(*)>=0FROMtruncate_tab; }
18+
step"s1_commit" {COMMIT; }
19+
20+
session"s2"
21+
step"s2_grant" {GRANTTRUNCATEONtruncate_tabTOregress_truncate_conflict; }
22+
step"s2_auth" {SETROLEregress_truncate_conflict; }
23+
step"s2_truncate" {TRUNCATEtruncate_tab; }
24+
step"s2_reset" {RESETROLE; }
25+
26+
# The role doesn't have privileges to truncate the table, so TRUNCATE should
27+
# immediately fail without waiting for a lock.
28+
permutation"s1_begin""s1_tab_lookup""s2_auth""s2_truncate""s1_commit""s2_reset"
29+
permutation"s1_begin""s2_auth""s2_truncate""s1_tab_lookup""s1_commit""s2_reset"
30+
permutation"s1_begin""s2_auth""s1_tab_lookup""s2_truncate""s1_commit""s2_reset"
31+
permutation"s2_auth""s2_truncate""s1_begin""s1_tab_lookup""s1_commit""s2_reset"
32+
33+
# The role has privileges to truncate the table, TRUNCATE will block if
34+
# another session holds a lock on the table and succeed in all cases.
35+
permutation"s1_begin""s1_tab_lookup""s2_grant""s2_auth""s2_truncate""s1_commit""s2_reset"
36+
permutation"s1_begin""s2_grant""s2_auth""s2_truncate""s1_tab_lookup""s1_commit""s2_reset"
37+
permutation"s1_begin""s2_grant""s2_auth""s1_tab_lookup""s2_truncate""s1_commit""s2_reset"
38+
permutation"s2_grant""s2_auth""s2_truncate""s1_begin""s1_tab_lookup""s1_commit""s2_reset"

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp