Skip to content

Commit c08d82f

Browse files
sfrostsfrost
committed
Add relkind checks to certain contrib modules
The contrib extensions pageinspect, pg_visibility and pgstattuple only work against regular relations which have storage. They don't work against foreign tables, partitioned (parent) tables, views, et al. Add checks to the user-callable functions to return a useful error message to the user if they mistakenly pass an invalid relation to a function which doesn't accept that kind of relation. In passing, improve some of the existing checks to use ereport() instead of elog(), add a function to consolidate common checks where appropriate, and add some regression tests. Author: Amit Langote, with various changes by me Reviewed by: Michael Paquier and Corey Huinker Discussion: https://postgr.es/m/ab91fd9d-4751-ee77-c87b-4dd704c1e59c@lab.ntt.co.jp
1 parent b54aad8 commit c08d82f

File tree

12 files changed

+517
-21
lines changed

12 files changed

+517
-21
lines changed

‎contrib/pageinspect/expected/page.out‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -71,3 +71,12 @@ SELECT * FROM fsm_page_contents(get_raw_page('test1', 'fsm', 0));
7171
(1 row)
7272

7373
DROP TABLE test1;
74+
-- check that using any of these functions with a partitioned table would fail
75+
create table test_partitioned (a int) partition by range (a);
76+
select get_raw_page('test_partitioned', 0); -- error about partitioned table
77+
ERROR: cannot get raw page from partitioned table "test_partitioned"
78+
-- a regular table which is a member of a partition set should work though
79+
create table test_part1 partition of test_partitioned for values from ( 1 ) to (100);
80+
select get_raw_page('test_part1', 0); -- get farther and error about empty table
81+
ERROR: block number 0 is out of range for relation "test_part1"
82+
drop table test_partitioned;

‎contrib/pageinspect/rawpage.c‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -123,6 +123,11 @@ get_raw_page_internal(text *relname, ForkNumber forknum, BlockNumber blkno)
123123
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
124124
errmsg("cannot get raw page from foreign table \"%s\"",
125125
RelationGetRelationName(rel))));
126+
if (rel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
127+
ereport(ERROR,
128+
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
129+
errmsg("cannot get raw page from partitioned table \"%s\"",
130+
RelationGetRelationName(rel))));
126131

127132
/*
128133
* Reject attempts to read non-local temporary relations; we would be

‎contrib/pageinspect/sql/page.sql‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,3 +28,12 @@ SELECT tuple_data_split('test1'::regclass, t_data, t_infomask, t_infomask2, t_bi
2828
SELECT * FROM fsm_page_contents(get_raw_page('test1', 'fsm', 0));
2929

3030
DROP TABLE test1;
31+
32+
-- check that using any of these functions with a partitioned table would fail
33+
create table test_partitioned (a int) partition by range (a);
34+
select get_raw_page('test_partitioned', 0); -- error about partitioned table
35+
36+
-- a regular table which is a member of a partition set should work though
37+
create table test_part1 partition of test_partitioned for values from ( 1 ) to (100);
38+
select get_raw_page('test_part1', 0); -- get farther and error about empty table
39+
drop table test_partitioned;

‎contrib/pg_visibility/.gitignore‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
# Generated subdirectories
2+
/log/
3+
/results/
4+
/tmp_check/

‎contrib/pg_visibility/Makefile‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ EXTENSION = pg_visibility
77
DATA = pg_visibility--1.1.sql pg_visibility--1.0--1.1.sql
88
PGFILEDESC = "pg_visibility - page visibility information"
99

10+
REGRESS = pg_visibility
11+
1012
ifdef USE_PGXS
1113
PG_CONFIG = pg_config
1214
PGXS := $(shell $(PG_CONFIG) --pgxs)

‎contrib/pg_visibility/expected/pg_visibility.out‎

Lines changed: 142 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,142 @@
1+
CREATE EXTENSION pg_visibility;
2+
--
3+
-- check that using the module's functions with unsupported relations will fail
4+
--
5+
-- partitioned tables (the parent ones) don't have visibility maps
6+
create table test_partitioned (a int) partition by list (a);
7+
-- these should all fail
8+
select pg_visibility('test_partitioned', 0);
9+
ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
10+
select pg_visibility_map('test_partitioned');
11+
ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
12+
select pg_visibility_map_summary('test_partitioned');
13+
ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
14+
select pg_check_frozen('test_partitioned');
15+
ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
16+
select pg_truncate_visibility_map('test_partitioned');
17+
ERROR: "test_partitioned" is not a table, materialized view, or TOAST table
18+
create table test_partition partition of test_partitioned for values in (1);
19+
create index test_index on test_partition (a);
20+
-- indexes do not, so these all fail
21+
select pg_visibility('test_index', 0);
22+
ERROR: "test_index" is not a table, materialized view, or TOAST table
23+
select pg_visibility_map('test_index');
24+
ERROR: "test_index" is not a table, materialized view, or TOAST table
25+
select pg_visibility_map_summary('test_index');
26+
ERROR: "test_index" is not a table, materialized view, or TOAST table
27+
select pg_check_frozen('test_index');
28+
ERROR: "test_index" is not a table, materialized view, or TOAST table
29+
select pg_truncate_visibility_map('test_index');
30+
ERROR: "test_index" is not a table, materialized view, or TOAST table
31+
create view test_view as select 1;
32+
-- views do not have VMs, so these all fail
33+
select pg_visibility('test_view', 0);
34+
ERROR: "test_view" is not a table, materialized view, or TOAST table
35+
select pg_visibility_map('test_view');
36+
ERROR: "test_view" is not a table, materialized view, or TOAST table
37+
select pg_visibility_map_summary('test_view');
38+
ERROR: "test_view" is not a table, materialized view, or TOAST table
39+
select pg_check_frozen('test_view');
40+
ERROR: "test_view" is not a table, materialized view, or TOAST table
41+
select pg_truncate_visibility_map('test_view');
42+
ERROR: "test_view" is not a table, materialized view, or TOAST table
43+
create sequence test_sequence;
44+
-- sequences do not have VMs, so these all fail
45+
select pg_visibility('test_sequence', 0);
46+
ERROR: "test_sequence" is not a table, materialized view, or TOAST table
47+
select pg_visibility_map('test_sequence');
48+
ERROR: "test_sequence" is not a table, materialized view, or TOAST table
49+
select pg_visibility_map_summary('test_sequence');
50+
ERROR: "test_sequence" is not a table, materialized view, or TOAST table
51+
select pg_check_frozen('test_sequence');
52+
ERROR: "test_sequence" is not a table, materialized view, or TOAST table
53+
select pg_truncate_visibility_map('test_sequence');
54+
ERROR: "test_sequence" is not a table, materialized view, or TOAST table
55+
create foreign data wrapper dummy;
56+
create server dummy_server foreign data wrapper dummy;
57+
create foreign table test_foreign_table () server dummy_server;
58+
-- foreign tables do not have VMs, so these all fail
59+
select pg_visibility('test_foreign_table', 0);
60+
ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table
61+
select pg_visibility_map('test_foreign_table');
62+
ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table
63+
select pg_visibility_map_summary('test_foreign_table');
64+
ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table
65+
select pg_check_frozen('test_foreign_table');
66+
ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table
67+
select pg_truncate_visibility_map('test_foreign_table');
68+
ERROR: "test_foreign_table" is not a table, materialized view, or TOAST table
69+
-- check some of the allowed relkinds
70+
create table regular_table (a int);
71+
insert into regular_table values (1), (2);
72+
vacuum regular_table;
73+
select count(*) > 0 from pg_visibility('regular_table');
74+
?column?
75+
----------
76+
t
77+
(1 row)
78+
79+
truncate regular_table;
80+
select count(*) > 0 from pg_visibility('regular_table');
81+
?column?
82+
----------
83+
f
84+
(1 row)
85+
86+
create materialized view matview_visibility_test as select * from regular_table;
87+
vacuum matview_visibility_test;
88+
select count(*) > 0 from pg_visibility('matview_visibility_test');
89+
?column?
90+
----------
91+
f
92+
(1 row)
93+
94+
insert into regular_table values (1), (2);
95+
refresh materialized view matview_visibility_test;
96+
select count(*) > 0 from pg_visibility('matview_visibility_test');
97+
?column?
98+
----------
99+
t
100+
(1 row)
101+
102+
-- regular tables which are part of a partition *do* have visibility maps
103+
insert into test_partition values (1);
104+
vacuum test_partition;
105+
select count(*) > 0 from pg_visibility('test_partition', 0);
106+
?column?
107+
----------
108+
t
109+
(1 row)
110+
111+
select count(*) > 0 from pg_visibility_map('test_partition');
112+
?column?
113+
----------
114+
t
115+
(1 row)
116+
117+
select count(*) > 0 from pg_visibility_map_summary('test_partition');
118+
?column?
119+
----------
120+
t
121+
(1 row)
122+
123+
select * from pg_check_frozen('test_partition'); -- hopefully none
124+
t_ctid
125+
--------
126+
(0 rows)
127+
128+
select pg_truncate_visibility_map('test_partition');
129+
pg_truncate_visibility_map
130+
----------------------------
131+
132+
(1 row)
133+
134+
-- cleanup
135+
drop table test_partitioned;
136+
drop view test_view;
137+
drop sequence test_sequence;
138+
drop foreign table test_foreign_table;
139+
drop server dummy_server;
140+
drop foreign data wrapper dummy;
141+
drop materialized view matview_visibility_test;
142+
drop table regular_table;

‎contrib/pg_visibility/pg_visibility.c‎

Lines changed: 44 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ static corrupt_items *collect_corrupt_items(Oid relid, bool all_visible,
5353
static void record_corrupt_item(corrupt_items *items, ItemPointer tid);
5454
static bool tuple_all_visible(HeapTuple tup, TransactionId OldestXmin,
5555
Buffer buffer);
56+
static void check_relation_relkind(Relation rel);
5657

5758
/*
5859
* Visibility map information for a single block of a relation.
@@ -75,6 +76,9 @@ pg_visibility_map(PG_FUNCTION_ARGS)
7576

7677
rel = relation_open(relid, AccessShareLock);
7778

79+
/* Only some relkinds have a visibility map */
80+
check_relation_relkind(rel);
81+
7882
if (blkno < 0 || blkno > MaxBlockNumber)
7983
ereport(ERROR,
8084
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -114,6 +118,9 @@ pg_visibility(PG_FUNCTION_ARGS)
114118

115119
rel = relation_open(relid, AccessShareLock);
116120

121+
/* Only some relkinds have a visibility map */
122+
check_relation_relkind(rel);
123+
117124
if (blkno < 0 || blkno > MaxBlockNumber)
118125
ereport(ERROR,
119126
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
@@ -167,6 +174,7 @@ pg_visibility_map_rel(PG_FUNCTION_ARGS)
167174
funcctx = SRF_FIRSTCALL_INIT();
168175
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
169176
funcctx->tuple_desc = pg_visibility_tupdesc(true, false);
177+
/* collect_visibility_data will verify the relkind */
170178
funcctx->user_fctx = collect_visibility_data(relid, false);
171179
MemoryContextSwitchTo(oldcontext);
172180
}
@@ -211,6 +219,7 @@ pg_visibility_rel(PG_FUNCTION_ARGS)
211219
funcctx = SRF_FIRSTCALL_INIT();
212220
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
213221
funcctx->tuple_desc = pg_visibility_tupdesc(true, true);
222+
/* collect_visibility_data will verify the relkind */
214223
funcctx->user_fctx = collect_visibility_data(relid, true);
215224
MemoryContextSwitchTo(oldcontext);
216225
}
@@ -257,6 +266,10 @@ pg_visibility_map_summary(PG_FUNCTION_ARGS)
257266
bool nulls[2];
258267

259268
rel = relation_open(relid, AccessShareLock);
269+
270+
/* Only some relkinds have a visibility map */
271+
check_relation_relkind(rel);
272+
260273
nblocks = RelationGetNumberOfBlocks(rel);
261274

262275
for (blkno = 0; blkno < nblocks; ++blkno)
@@ -309,6 +322,7 @@ pg_check_frozen(PG_FUNCTION_ARGS)
309322

310323
funcctx = SRF_FIRSTCALL_INIT();
311324
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
325+
/* collect_corrupt_items will verify the relkind */
312326
funcctx->user_fctx = collect_corrupt_items(relid, false, true);
313327
MemoryContextSwitchTo(oldcontext);
314328
}
@@ -340,6 +354,7 @@ pg_check_visible(PG_FUNCTION_ARGS)
340354

341355
funcctx = SRF_FIRSTCALL_INIT();
342356
oldcontext = MemoryContextSwitchTo(funcctx->multi_call_memory_ctx);
357+
/* collect_corrupt_items will verify the relkind */
343358
funcctx->user_fctx = collect_corrupt_items(relid, true, false);
344359
MemoryContextSwitchTo(oldcontext);
345360
}
@@ -369,13 +384,8 @@ pg_truncate_visibility_map(PG_FUNCTION_ARGS)
369384

370385
rel = relation_open(relid, AccessExclusiveLock);
371386

372-
if (rel->rd_rel->relkind != RELKIND_RELATION &&
373-
rel->rd_rel->relkind != RELKIND_MATVIEW &&
374-
rel->rd_rel->relkind != RELKIND_TOASTVALUE)
375-
ereport(ERROR,
376-
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
377-
errmsg("\"%s\" is not a table, materialized view, or TOAST table",
378-
RelationGetRelationName(rel))));
387+
/* Only some relkinds have a visibility map */
388+
check_relation_relkind(rel);
379389

380390
RelationOpenSmgr(rel);
381391
rel->rd_smgr->smgr_vm_nblocks = InvalidBlockNumber;
@@ -451,6 +461,9 @@ pg_visibility_tupdesc(bool include_blkno, bool include_pd)
451461

452462
/*
453463
* Collect visibility data about a relation.
464+
*
465+
* Checks relkind of relid and will throw an error if the relation does not
466+
* have a VM.
454467
*/
455468
static vbits *
456469
collect_visibility_data(Oid relid, bool include_pd)
@@ -464,6 +477,9 @@ collect_visibility_data(Oid relid, bool include_pd)
464477

465478
rel = relation_open(relid, AccessShareLock);
466479

480+
/* Only some relkinds have a visibility map */
481+
check_relation_relkind(rel);
482+
467483
nblocks = RelationGetNumberOfBlocks(rel);
468484
info = palloc0(offsetof(vbits, bits) +nblocks);
469485
info->next = 0;
@@ -523,6 +539,9 @@ collect_visibility_data(Oid relid, bool include_pd)
523539
*
524540
* If all_frozen is passed as true, this will include all items which are
525541
* on pages marked as all-frozen but which do not seem to in fact be frozen.
542+
*
543+
* Checks relkind of relid and will throw an error if the relation does not
544+
* have a VM.
526545
*/
527546
static corrupt_items *
528547
collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
@@ -543,13 +562,8 @@ collect_corrupt_items(Oid relid, bool all_visible, bool all_frozen)
543562

544563
rel = relation_open(relid, AccessShareLock);
545564

546-
if (rel->rd_rel->relkind != RELKIND_RELATION &&
547-
rel->rd_rel->relkind != RELKIND_MATVIEW &&
548-
rel->rd_rel->relkind != RELKIND_TOASTVALUE)
549-
ereport(ERROR,
550-
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
551-
errmsg("\"%s\" is not a table, materialized view, or TOAST table",
552-
RelationGetRelationName(rel))));
565+
/* Only some relkinds have a visibility map */
566+
check_relation_relkind(rel);
553567

554568
nblocks = RelationGetNumberOfBlocks(rel);
555569

@@ -747,3 +761,19 @@ tuple_all_visible(HeapTuple tup, TransactionId OldestXmin, Buffer buffer)
747761

748762
return true;
749763
}
764+
765+
/*
766+
* check_relation_relkind - convenience routine to check that relation
767+
* is of the relkind supported by the callers
768+
*/
769+
static void
770+
check_relation_relkind(Relation rel)
771+
{
772+
if (rel->rd_rel->relkind != RELKIND_RELATION &&
773+
rel->rd_rel->relkind != RELKIND_MATVIEW &&
774+
rel->rd_rel->relkind != RELKIND_TOASTVALUE)
775+
ereport(ERROR,
776+
(errcode(ERRCODE_WRONG_OBJECT_TYPE),
777+
errmsg("\"%s\" is not a table, materialized view, or TOAST table",
778+
RelationGetRelationName(rel))));
779+
}

0 commit comments

Comments
 (0)