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

Commitea97e44

Browse files
committed
Harden tableam against nonexistant / wrong kind of AMs.
Previously it was allowed to set default_table_access_method to anempty string. That makes sense for default_tablespace, where that wascopied from, as it signals falling back to the database's defaulttablespace. As there is no equivalent for table AMs, forbid that.Also make sure to throw a usable error when creating a table using anindex AM, by using get_am_type_oid() to implement get_table_am_oid()instead of a separate copy. Previously we'd error out only later, inGetTableAmRoutine().Thirdly remove GetTableAmRoutineByAmId() - it was only used in anearlier version of8586bf7.Add tests for the above (some for index AMs as well).
1 parent344b7e1 commitea97e44

File tree

9 files changed

+119
-110
lines changed

9 files changed

+119
-110
lines changed

‎src/backend/access/table/tableamapi.c

Lines changed: 16 additions & 94 deletions
Original file line numberDiff line numberDiff line change
@@ -17,14 +17,12 @@
1717
#include"access/xact.h"
1818
#include"catalog/pg_am.h"
1919
#include"catalog/pg_proc.h"
20+
#include"commands/defrem.h"
2021
#include"utils/fmgroids.h"
2122
#include"utils/memutils.h"
2223
#include"utils/syscache.h"
2324

2425

25-
staticOidget_table_am_oid(constchar*tableamname,boolmissing_ok);
26-
27-
2826
/*
2927
* GetTableAmRoutine
3028
*Call the specified access method handler routine to get its
@@ -41,7 +39,7 @@ GetTableAmRoutine(Oid amhandler)
4139
routine= (TableAmRoutine*)DatumGetPointer(datum);
4240

4341
if (routine==NULL|| !IsA(routine,TableAmRoutine))
44-
elog(ERROR,"Table access method handler %u did not return a TableAmRoutine struct",
42+
elog(ERROR,"table access method handler %u did not return a TableAmRoutine struct",
4543
amhandler);
4644

4745
/*
@@ -98,106 +96,30 @@ GetTableAmRoutine(Oid amhandler)
9896
returnroutine;
9997
}
10098

101-
/*
102-
* GetTableAmRoutineByAmId - look up the handler of the table access
103-
* method with the given OID, and get its TableAmRoutine struct.
104-
*/
105-
constTableAmRoutine*
106-
GetTableAmRoutineByAmId(Oidamoid)
107-
{
108-
regprocamhandler;
109-
HeapTupletuple;
110-
Form_pg_amamform;
111-
112-
/* Get handler function OID for the access method */
113-
tuple=SearchSysCache1(AMOID,ObjectIdGetDatum(amoid));
114-
if (!HeapTupleIsValid(tuple))
115-
elog(ERROR,"cache lookup failed for access method %u",
116-
amoid);
117-
amform= (Form_pg_am)GETSTRUCT(tuple);
118-
119-
/* Check that it is a table access method */
120-
if (amform->amtype!=AMTYPE_TABLE)
121-
ereport(ERROR,
122-
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
123-
errmsg("access method \"%s\" is not of type %s",
124-
NameStr(amform->amname),"TABLE")));
125-
126-
amhandler=amform->amhandler;
127-
128-
/* Complain if handler OID is invalid */
129-
if (!RegProcedureIsValid(amhandler))
130-
ereport(ERROR,
131-
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
132-
errmsg("table access method \"%s\" does not have a handler",
133-
NameStr(amform->amname))));
134-
135-
ReleaseSysCache(tuple);
136-
137-
/* And finally, call the handler function to get the API struct. */
138-
returnGetTableAmRoutine(amhandler);
139-
}
140-
141-
/*
142-
* get_table_am_oid - given a table access method name, look up the OID
143-
*
144-
* If missing_ok is false, throw an error if table access method name not
145-
* found. If true, just return InvalidOid.
146-
*/
147-
staticOid
148-
get_table_am_oid(constchar*tableamname,boolmissing_ok)
149-
{
150-
Oidresult;
151-
Relationrel;
152-
TableScanDescscandesc;
153-
HeapTupletuple;
154-
ScanKeyDataentry[1];
155-
156-
/*
157-
* Search pg_am. We use a heapscan here even though there is an index on
158-
* name, on the theory that pg_am will usually have just a few entries and
159-
* so an indexed lookup is a waste of effort.
160-
*/
161-
rel=heap_open(AccessMethodRelationId,AccessShareLock);
162-
163-
ScanKeyInit(&entry[0],
164-
Anum_pg_am_amname,
165-
BTEqualStrategyNumber,F_NAMEEQ,
166-
CStringGetDatum(tableamname));
167-
scandesc=table_beginscan_catalog(rel,1,entry);
168-
tuple=heap_getnext(scandesc,ForwardScanDirection);
169-
170-
/* We assume that there can be at most one matching tuple */
171-
if (HeapTupleIsValid(tuple)&&
172-
((Form_pg_am)GETSTRUCT(tuple))->amtype==AMTYPE_TABLE)
173-
result= ((Form_pg_am)GETSTRUCT(tuple))->oid;
174-
else
175-
result=InvalidOid;
176-
177-
table_endscan(scandesc);
178-
heap_close(rel,AccessShareLock);
179-
180-
if (!OidIsValid(result)&& !missing_ok)
181-
ereport(ERROR,
182-
(errcode(ERRCODE_UNDEFINED_OBJECT),
183-
errmsg("table access method \"%s\" does not exist",
184-
tableamname)));
185-
186-
returnresult;
187-
}
188-
18999
/* check_hook: validate new default_table_access_method */
190100
bool
191101
check_default_table_access_method(char**newval,void**extra,GucSourcesource)
192102
{
103+
if (**newval=='\0')
104+
{
105+
GUC_check_errdetail("default_table_access_method may not be empty.");
106+
return false;
107+
}
108+
109+
if (strlen(*newval) >=NAMEDATALEN)
110+
{
111+
GUC_check_errdetail("default_table_access_method is too long (maximum %d characters).",
112+
NAMEDATALEN-1);
113+
return false;
114+
}
115+
193116
/*
194117
* If we aren't inside a transaction, we cannot do database access so
195118
* cannot verify the name. Must accept the value on faith.
196119
*/
197120
if (IsTransactionState())
198121
{
199-
if (**newval!='\0'&&
200-
!OidIsValid(get_table_am_oid(*newval, true)))
122+
if (!OidIsValid(get_table_am_oid(*newval, true)))
201123
{
202124
/*
203125
* When source == PGC_S_TEST, don't throw a hard error for a

‎src/backend/commands/amcmds.c

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,16 @@ get_index_am_oid(const char *amname, bool missing_ok)
189189
returnget_am_type_oid(amname,AMTYPE_INDEX,missing_ok);
190190
}
191191

192+
/*
193+
* get_table_am_oid - given an access method name, look up its OID
194+
*and verify it corresponds to an table AM.
195+
*/
196+
Oid
197+
get_table_am_oid(constchar*amname,boolmissing_ok)
198+
{
199+
returnget_am_type_oid(amname,AMTYPE_TABLE,missing_ok);
200+
}
201+
192202
/*
193203
* get_am_oid - given an access method name, look up its OID.
194204
*The type is not checked.

‎src/backend/commands/tablecmds.c

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -832,22 +832,9 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
832832
relkind == RELKIND_MATVIEW)
833833
accessMethod = default_table_access_method;
834834

835-
/*
836-
* look up the access method, verify it can handle the requested features
837-
*/
835+
/* look up the access method, verify it is for a table */
838836
if (accessMethod != NULL)
839-
{
840-
HeapTupletuple;
841-
842-
tuple = SearchSysCache1(AMNAME, PointerGetDatum(accessMethod));
843-
if (!HeapTupleIsValid(tuple))
844-
ereport(ERROR,
845-
(errcode(ERRCODE_UNDEFINED_OBJECT),
846-
errmsg("table access method \"%s\" does not exist",
847-
accessMethod)));
848-
accessMethodId = ((Form_pg_am) GETSTRUCT(tuple))->oid;
849-
ReleaseSysCache(tuple);
850-
}
837+
accessMethodId = get_table_am_oid(accessMethod, false);
851838

852839
/*
853840
* Create the relation. Inherited defaults and constraints are passed in

‎src/include/access/tableam.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1616,7 +1616,6 @@ extern void table_block_parallelscan_startblock_init(Relation rel,
16161616
*/
16171617

16181618
externconstTableAmRoutine*GetTableAmRoutine(Oidamhandler);
1619-
externconstTableAmRoutine*GetTableAmRoutineByAmId(Oidamoid);
16201619
externconstTableAmRoutine*GetHeapamTableAmRoutine(void);
16211620
externboolcheck_default_table_access_method(char**newval,void**extra,
16221621
GucSourcesource);

‎src/include/commands/defrem.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -156,6 +156,7 @@ extern Datum transformGenericOptions(Oid catalogId,
156156
externObjectAddressCreateAccessMethod(CreateAmStmt*stmt);
157157
externvoidRemoveAccessMethodById(OidamOid);
158158
externOidget_index_am_oid(constchar*amname,boolmissing_ok);
159+
externOidget_table_am_oid(constchar*amname,boolmissing_ok);
159160
externOidget_am_oid(constchar*amname,boolmissing_ok);
160161
externchar*get_am_name(OidamOid);
161162

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,11 @@
33
--
44
-- Make gist2 over gisthandler. In fact, it would be a synonym to gist.
55
CREATE ACCESS METHOD gist2 TYPE INDEX HANDLER gisthandler;
6+
-- Verify return type checks for handlers
7+
CREATE ACCESS METHOD bogus TYPE INDEX HANDLER int4in;
8+
ERROR: function int4in(internal) does not exist
9+
CREATE ACCESS METHOD bogus TYPE INDEX HANDLER heap_tableam_handler;
10+
ERROR: function heap_tableam_handler must return type index_am_handler
611
-- Try to create gist2 index on fast_emp4000: fail because opclass doesn't exist
712
CREATE INDEX grect2ind2 ON fast_emp4000 USING gist2 (home_base);
813
ERROR: data type box has no default operator class for access method "gist2"
@@ -102,8 +107,24 @@ NOTICE: drop cascades to index grect2ind2
102107
--
103108
-- Test table access methods
104109
--
110+
-- prevent empty values
111+
SET default_table_access_method = '';
112+
ERROR: invalid value for parameter "default_table_access_method": ""
113+
DETAIL: default_table_access_method may not be empty.
114+
-- prevent nonexistant values
115+
SET default_table_access_method = 'I do not exist AM';
116+
ERROR: invalid value for parameter "default_table_access_method": "I do not exist AM"
117+
DETAIL: Table access method "I do not exist AM" does not exist.
118+
-- prevent setting it to an index AM
119+
SET default_table_access_method = 'btree';
120+
ERROR: access method "btree" is not of type TABLE
105121
-- Create a heap2 table am handler with heapam handler
106122
CREATE ACCESS METHOD heap2 TYPE TABLE HANDLER heap_tableam_handler;
123+
-- Verify return type checks for handlers
124+
CREATE ACCESS METHOD bogus TYPE TABLE HANDLER int4in;
125+
ERROR: function int4in(internal) does not exist
126+
CREATE ACCESS METHOD bogus TYPE TABLE HANDLER bthandler;
127+
ERROR: function bthandler must return type table_am_handler
107128
SELECT amname, amhandler, amtype FROM pg_am where amtype = 't' ORDER BY 1, 2;
108129
amname | amhandler | amtype
109130
--------+----------------------+--------
@@ -253,6 +274,18 @@ ORDER BY 3, 1, 2;
253274

254275
-- don't want to keep those tables, nor the default
255276
ROLLBACK;
277+
-- Third, check that we can neither create a table using a nonexistant
278+
-- AM, nor using an index AM
279+
CREATE TABLE i_am_a_failure() USING "";
280+
ERROR: zero-length delimited identifier at or near """"
281+
LINE 1: CREATE TABLE i_am_a_failure() USING "";
282+
^
283+
CREATE TABLE i_am_a_failure() USING i_do_not_exist_am;
284+
ERROR: access method "i_do_not_exist_am" does not exist
285+
CREATE TABLE i_am_a_failure() USING "I do not exist AM";
286+
ERROR: access method "I do not exist AM" does not exist
287+
CREATE TABLE i_am_a_failure() USING "btree";
288+
ERROR: access method "btree" is not of type TABLE
256289
-- Drop table access method, which fails as objects depends on it
257290
DROP ACCESS METHOD heap2;
258291
ERROR: cannot drop access method heap2 because other objects depend on it

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -520,6 +520,24 @@ WHERE p1.relkind IN ('S', 'v', 'f', 'c') and
520520
-----+---------
521521
(0 rows)
522522

523+
-- Indexes should have AMs of type 'i'
524+
SELECT pc.oid, pc.relname, pa.amname, pa.amtype
525+
FROM pg_class as pc JOIN pg_am AS pa ON (pc.relam = pa.oid)
526+
WHERE pc.relkind IN ('i') and
527+
pa.amtype != 'i';
528+
oid | relname | amname | amtype
529+
-----+---------+--------+--------
530+
(0 rows)
531+
532+
-- Tables, matviews etc should have AMs of type 't'
533+
SELECT pc.oid, pc.relname, pa.amname, pa.amtype
534+
FROM pg_class as pc JOIN pg_am AS pa ON (pc.relam = pa.oid)
535+
WHERE pc.relkind IN ('r', 't', 'm') and
536+
pa.amtype != 't';
537+
oid | relname | amname | amtype
538+
-----+---------+--------+--------
539+
(0 rows)
540+
523541
-- **************** pg_attribute ****************
524542
-- Look for illegal values in pg_attribute fields
525543
SELECT p1.attrelid, p1.attname

‎src/test/regress/sql/create_am.sql

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@
55
-- Make gist2 over gisthandler. In fact, it would be a synonym to gist.
66
CREATE ACCESS METHOD gist2 TYPE INDEX HANDLER gisthandler;
77

8+
-- Verify return type checks for handlers
9+
CREATE ACCESS METHOD bogus TYPE INDEX HANDLER int4in;
10+
CREATE ACCESS METHOD bogus TYPE INDEX HANDLER heap_tableam_handler;
11+
12+
813
-- Try to create gist2 index on fast_emp4000: fail because opclass doesn't exist
914
CREATEINDEXgrect2ind2ON fast_emp4000 USING gist2 (home_base);
1015

@@ -72,11 +77,26 @@ DROP ACCESS METHOD gist2 CASCADE;
7277
-- Test table access methods
7378
--
7479

80+
-- prevent empty values
81+
SET default_table_access_method='';
82+
83+
-- prevent nonexistant values
84+
SET default_table_access_method='I do not exist AM';
85+
86+
-- prevent setting it to an index AM
87+
SET default_table_access_method='btree';
88+
89+
7590
-- Create a heap2 table am handler with heapam handler
7691
CREATE ACCESS METHOD heap2 TYPE TABLE HANDLER heap_tableam_handler;
7792

93+
-- Verify return type checks for handlers
94+
CREATE ACCESS METHOD bogus TYPE TABLE HANDLER int4in;
95+
CREATE ACCESS METHOD bogus TYPE TABLE HANDLER bthandler;
96+
7897
SELECT amname, amhandler, amtypeFROM pg_amwhere amtype='t'ORDER BY1,2;
7998

99+
80100
-- First create tables employing the new AM using USING
81101

82102
-- plain CREATE TABLE
@@ -178,6 +198,13 @@ ORDER BY 3, 1, 2;
178198
-- don't want to keep those tables, nor the default
179199
ROLLBACK;
180200

201+
-- Third, check that we can neither create a table using a nonexistant
202+
-- AM, nor using an index AM
203+
CREATETABLEi_am_a_failure() USING"";
204+
CREATETABLEi_am_a_failure() USING i_do_not_exist_am;
205+
CREATETABLEi_am_a_failure() USING"I do not exist AM";
206+
CREATETABLEi_am_a_failure() USING"btree";
207+
181208
-- Drop table access method, which fails as objects depends on it
182209
DROP ACCESS METHOD heap2;
183210

‎src/test/regress/sql/type_sanity.sql

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -379,6 +379,18 @@ FROM pg_class as p1
379379
WHEREp1.relkindIN ('S','v','f','c')and
380380
p1.relam!=0;
381381

382+
-- Indexes should have AMs of type 'i'
383+
SELECTpc.oid,pc.relname,pa.amname,pa.amtype
384+
FROM pg_classas pcJOIN pg_amAS paON (pc.relam=pa.oid)
385+
WHEREpc.relkindIN ('i')and
386+
pa.amtype!='i';
387+
388+
-- Tables, matviews etc should have AMs of type 't'
389+
SELECTpc.oid,pc.relname,pa.amname,pa.amtype
390+
FROM pg_classas pcJOIN pg_amAS paON (pc.relam=pa.oid)
391+
WHEREpc.relkindIN ('r','t','m')and
392+
pa.amtype!='t';
393+
382394
-- **************** pg_attribute ****************
383395

384396
-- Look for illegal values in pg_attribute fields

0 commit comments

Comments
 (0)

[8]ページ先頭

©2009-2025 Movatter.jp