Skip to content

Commit 9f12da7

Browse files
jeff-davisjeff-davis
committed
Lock table in ShareUpdateExclusive when importing index stats.
Follow locking behavior of ANALYZE when importing statistics. In particular, when importing index statistics, the table must be locked in ShareUpdateExclusive mode. Fixes bug reportd by Jian He. ANALYZE doesn't update statistics on partitioned indexes, and the locking requirements are slightly different for in-place updates on partitioned indexes versus normal indexes. To be conservative, lock both the partitioned table and the partitioned index in ShareUpdateExclusive mode when importing stats for a partitioned index. Author: Corey Huinker Reported-by: Jian He Reviewed-by: Michael Paquier Discussion: https://www.postgresql.org/message-id/CACJufxGreTY7qsCV8%2BBkuv0p5SXGTScgh%3DD%2BDq6%3D%2B_%3DXTp7FWg%40mail.gmail.com
1 parent 979205e commit 9f12da7

File tree

3 files changed

+228
-13
lines changed

3 files changed

+228
-13
lines changed

‎src/backend/statistics/stat_utils.c‎

Lines changed: 57 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -17,13 +17,16 @@
1717
#include "postgres.h"
1818

1919
#include "access/relation.h"
20+
#include "catalog/index.h"
2021
#include "catalog/pg_database.h"
2122
#include "funcapi.h"
2223
#include "miscadmin.h"
2324
#include "statistics/stat_utils.h"
25+
#include "storage/lmgr.h"
2426
#include "utils/acl.h"
2527
#include "utils/array.h"
2628
#include "utils/builtins.h"
29+
#include "utils/lsyscache.h"
2730
#include "utils/rel.h"
2831

2932
/*
@@ -126,45 +129,86 @@ stats_check_arg_pair(FunctionCallInfo fcinfo,
126129
void
127130
stats_lock_check_privileges(Oid reloid)
128131
{
129-
Relation rel = relation_open(reloid, ShareUpdateExclusiveLock);
130-
const char relkind = rel->rd_rel->relkind;
132+
Relation table;
133+
Oid table_oid = reloid;
134+
Oid index_oid = InvalidOid;
135+
LOCKMODE index_lockmode = NoLock;
131136

132-
/* All of the types that can be used with ANALYZE, plus indexes */
133-
switch (relkind)
137+
/*
138+
* For indexes, we follow the locking behavior in do_analyze_rel() and
139+
* check_inplace_rel_lock(), which is to lock the table first in
140+
* ShareUpdateExclusive mode and then the index in AccessShare mode.
141+
*
142+
* Partitioned indexes are treated differently than normal indexes in
143+
* check_inplace_rel_lock(), so we take a ShareUpdateExclusive lock on
144+
* both the partitioned table and the partitioned index.
145+
*/
146+
switch (get_rel_relkind(reloid))
134147
{
135-
case RELKIND_RELATION:
136148
case RELKIND_INDEX:
149+
index_oid = reloid;
150+
table_oid = IndexGetRelation(index_oid, false);
151+
index_lockmode = AccessShareLock;
152+
break;
153+
case RELKIND_PARTITIONED_INDEX:
154+
index_oid = reloid;
155+
table_oid = IndexGetRelation(index_oid, false);
156+
index_lockmode = ShareUpdateExclusiveLock;
157+
break;
158+
default:
159+
break;
160+
}
161+
162+
table = relation_open(table_oid, ShareUpdateExclusiveLock);
163+
164+
/* the relkinds that can be used with ANALYZE */
165+
switch (table->rd_rel->relkind)
166+
{
167+
case RELKIND_RELATION:
137168
case RELKIND_MATVIEW:
138169
case RELKIND_FOREIGN_TABLE:
139170
case RELKIND_PARTITIONED_TABLE:
140-
case RELKIND_PARTITIONED_INDEX:
141171
break;
142172
default:
143173
ereport(ERROR,
144174
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
145175
errmsg("cannot modify statistics for relation \"%s\"",
146-
RelationGetRelationName(rel)),
147-
errdetail_relkind_not_supported(rel->rd_rel->relkind)));
176+
RelationGetRelationName(table)),
177+
errdetail_relkind_not_supported(table->rd_rel->relkind)));
178+
}
179+
180+
if (OidIsValid(index_oid))
181+
{
182+
Relation index;
183+
184+
Assert(index_lockmode != NoLock);
185+
index = relation_open(index_oid, index_lockmode);
186+
187+
Assert(index->rd_index && index->rd_index->indrelid == table_oid);
188+
189+
/* retain lock on index */
190+
relation_close(index, NoLock);
148191
}
149192

150-
if (rel->rd_rel->relisshared)
193+
if (table->rd_rel->relisshared)
151194
ereport(ERROR,
152195
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
153196
errmsg("cannot modify statistics for shared relation")));
154197

155198
if (!object_ownercheck(DatabaseRelationId, MyDatabaseId, GetUserId()))
156199
{
157-
AclResult aclresult = pg_class_aclcheck(RelationGetRelid(rel),
200+
AclResult aclresult = pg_class_aclcheck(RelationGetRelid(table),
158201
GetUserId(),
159202
ACL_MAINTAIN);
160203

161204
if (aclresult != ACLCHECK_OK)
162205
aclcheck_error(aclresult,
163-
get_relkind_objtype(rel->rd_rel->relkind),
164-
NameStr(rel->rd_rel->relname));
206+
get_relkind_objtype(table->rd_rel->relkind),
207+
NameStr(table->rd_rel->relname));
165208
}
166209

167-
relation_close(rel, NoLock);
210+
/* retain lock on table */
211+
relation_close(table, NoLock);
168212
}
169213

170214
/*

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

Lines changed: 103 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,44 @@ WHERE oid = 'stats_import.test'::regclass;
8585
17 | 400 | 4
8686
(1 row)
8787

88+
CREATE INDEX test_i ON stats_import.test(id);
89+
BEGIN;
90+
-- regular indexes have special case locking rules
91+
SELECT
92+
pg_catalog.pg_set_relation_stats(
93+
relation => 'stats_import.test_i'::regclass,
94+
relpages => 18::integer);
95+
pg_set_relation_stats
96+
-----------------------
97+
98+
(1 row)
99+
100+
SELECT mode FROM pg_locks
101+
WHERE relation = 'stats_import.test'::regclass AND
102+
pid = pg_backend_pid() AND granted;
103+
mode
104+
--------------------------
105+
ShareUpdateExclusiveLock
106+
(1 row)
107+
108+
SELECT mode FROM pg_locks
109+
WHERE relation = 'stats_import.test_i'::regclass AND
110+
pid = pg_backend_pid() AND granted;
111+
mode
112+
-----------------
113+
AccessShareLock
114+
(1 row)
115+
116+
COMMIT;
117+
SELECT
118+
pg_catalog.pg_restore_relation_stats(
119+
'relation', 'stats_import.test_i'::regclass,
120+
'relpages', 19::integer );
121+
pg_restore_relation_stats
122+
---------------------------
123+
t
124+
(1 row)
125+
88126
-- positional arguments
89127
SELECT
90128
pg_catalog.pg_set_relation_stats(
@@ -182,6 +220,7 @@ CREATE TABLE stats_import.part_child_1
182220
PARTITION OF stats_import.part_parent
183221
FOR VALUES FROM (0) TO (10)
184222
WITH (autovacuum_enabled = false);
223+
CREATE INDEX part_parent_i ON stats_import.part_parent(i);
185224
ANALYZE stats_import.part_parent;
186225
SELECT relpages
187226
FROM pg_class
@@ -193,6 +232,15 @@ WHERE oid = 'stats_import.part_parent'::regclass;
193232

194233
-- although partitioned tables have no storage, setting relpages to a
195234
-- positive value is still allowed
235+
SELECT
236+
pg_catalog.pg_set_relation_stats(
237+
relation => 'stats_import.part_parent_i'::regclass,
238+
relpages => 2::integer);
239+
pg_set_relation_stats
240+
-----------------------
241+
242+
(1 row)
243+
196244
SELECT
197245
pg_catalog.pg_set_relation_stats(
198246
relation => 'stats_import.part_parent'::regclass,
@@ -202,6 +250,48 @@ SELECT
202250

203251
(1 row)
204252

253+
--
254+
-- Partitioned indexes aren't analyzed but it is possible to set
255+
-- stats. The locking rules are different from normal indexes due to
256+
-- the rules for in-place updates: both the partitioned table and the
257+
-- partitioned index are locked in ShareUpdateExclusive mode.
258+
--
259+
BEGIN;
260+
SELECT
261+
pg_catalog.pg_set_relation_stats(
262+
relation => 'stats_import.part_parent_i'::regclass,
263+
relpages => 2::integer);
264+
pg_set_relation_stats
265+
-----------------------
266+
267+
(1 row)
268+
269+
SELECT mode FROM pg_locks
270+
WHERE relation = 'stats_import.part_parent'::regclass AND
271+
pid = pg_backend_pid() AND granted;
272+
mode
273+
--------------------------
274+
ShareUpdateExclusiveLock
275+
(1 row)
276+
277+
SELECT mode FROM pg_locks
278+
WHERE relation = 'stats_import.part_parent_i'::regclass AND
279+
pid = pg_backend_pid() AND granted;
280+
mode
281+
--------------------------
282+
ShareUpdateExclusiveLock
283+
(1 row)
284+
285+
COMMIT;
286+
SELECT
287+
pg_catalog.pg_restore_relation_stats(
288+
'relation', 'stats_import.part_parent_i'::regclass,
289+
'relpages', 2::integer);
290+
pg_restore_relation_stats
291+
---------------------------
292+
t
293+
(1 row)
294+
205295
-- nothing stops us from setting it to -1
206296
SELECT
207297
pg_catalog.pg_set_relation_stats(
@@ -1414,6 +1504,19 @@ SELECT 3, 'tre', (3, 3.3, 'TRE', '2003-03-03', NULL)::stats_import.complex_type,
14141504
UNION ALL
14151505
SELECT 4, 'four', NULL, int4range(0,100), NULL;
14161506
CREATE INDEX is_odd ON stats_import.test(((comp).a % 2 = 1));
1507+
-- restoring stats on index
1508+
SELECT * FROM pg_catalog.pg_restore_relation_stats(
1509+
'relation', 'stats_import.is_odd'::regclass,
1510+
'version', '180000'::integer,
1511+
'relpages', '11'::integer,
1512+
'reltuples', '10000'::real,
1513+
'relallvisible', '0'::integer
1514+
);
1515+
pg_restore_relation_stats
1516+
---------------------------
1517+
t
1518+
(1 row)
1519+
14171520
-- Generate statistics on table with data
14181521
ANALYZE stats_import.test;
14191522
CREATE TABLE stats_import.test_clone ( LIKE stats_import.test )

‎src/test/regress/sql/stats_import.sql‎

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,30 @@ SELECT relpages, reltuples, relallvisible
6464
FROM pg_class
6565
WHERE oid = 'stats_import.test'::regclass;
6666

67+
CREATE INDEX test_i ON stats_import.test(id);
68+
69+
BEGIN;
70+
-- regular indexes have special case locking rules
71+
SELECT
72+
pg_catalog.pg_set_relation_stats(
73+
relation => 'stats_import.test_i'::regclass,
74+
relpages => 18::integer);
75+
76+
SELECT mode FROM pg_locks
77+
WHERE relation = 'stats_import.test'::regclass AND
78+
pid = pg_backend_pid() AND granted;
79+
80+
SELECT mode FROM pg_locks
81+
WHERE relation = 'stats_import.test_i'::regclass AND
82+
pid = pg_backend_pid() AND granted;
83+
84+
COMMIT;
85+
86+
SELECT
87+
pg_catalog.pg_restore_relation_stats(
88+
'relation', 'stats_import.test_i'::regclass,
89+
'relpages', 19::integer );
90+
6791
-- positional arguments
6892
SELECT
6993
pg_catalog.pg_set_relation_stats(
@@ -127,6 +151,8 @@ CREATE TABLE stats_import.part_child_1
127151
FOR VALUES FROM (0) TO (10)
128152
WITH (autovacuum_enabled = false);
129153

154+
CREATE INDEX part_parent_i ON stats_import.part_parent(i);
155+
130156
ANALYZE stats_import.part_parent;
131157

132158
SELECT relpages
@@ -135,11 +161,44 @@ WHERE oid = 'stats_import.part_parent'::regclass;
135161

136162
-- although partitioned tables have no storage, setting relpages to a
137163
-- positive value is still allowed
164+
SELECT
165+
pg_catalog.pg_set_relation_stats(
166+
relation => 'stats_import.part_parent_i'::regclass,
167+
relpages => 2::integer);
168+
138169
SELECT
139170
pg_catalog.pg_set_relation_stats(
140171
relation => 'stats_import.part_parent'::regclass,
141172
relpages => 2::integer);
142173

174+
--
175+
-- Partitioned indexes aren't analyzed but it is possible to set
176+
-- stats. The locking rules are different from normal indexes due to
177+
-- the rules for in-place updates: both the partitioned table and the
178+
-- partitioned index are locked in ShareUpdateExclusive mode.
179+
--
180+
BEGIN;
181+
182+
SELECT
183+
pg_catalog.pg_set_relation_stats(
184+
relation => 'stats_import.part_parent_i'::regclass,
185+
relpages => 2::integer);
186+
187+
SELECT mode FROM pg_locks
188+
WHERE relation = 'stats_import.part_parent'::regclass AND
189+
pid = pg_backend_pid() AND granted;
190+
191+
SELECT mode FROM pg_locks
192+
WHERE relation = 'stats_import.part_parent_i'::regclass AND
193+
pid = pg_backend_pid() AND granted;
194+
195+
COMMIT;
196+
197+
SELECT
198+
pg_catalog.pg_restore_relation_stats(
199+
'relation', 'stats_import.part_parent_i'::regclass,
200+
'relpages', 2::integer);
201+
143202
-- nothing stops us from setting it to -1
144203
SELECT
145204
pg_catalog.pg_set_relation_stats(
@@ -1062,6 +1121,15 @@ SELECT 4, 'four', NULL, int4range(0,100), NULL;
10621121

10631122
CREATE INDEX is_odd ON stats_import.test(((comp).a % 2 = 1));
10641123

1124+
-- restoring stats on index
1125+
SELECT * FROM pg_catalog.pg_restore_relation_stats(
1126+
'relation', 'stats_import.is_odd'::regclass,
1127+
'version', '180000'::integer,
1128+
'relpages', '11'::integer,
1129+
'reltuples', '10000'::real,
1130+
'relallvisible', '0'::integer
1131+
);
1132+
10651133
-- Generate statistics on table with data
10661134
ANALYZE stats_import.test;
10671135

0 commit comments

Comments
 (0)