mirror of
https://git.postgresql.org/git/postgresql.git
synced 2024-09-28 00:21:50 +02:00
Harden tableam against nonexistant / wrong kind of AMs.
Previously it was allowed to set default_table_access_method to an
empty string. That makes sense for default_tablespace, where that was
copied from, as it signals falling back to the database's default
tablespace. As there is no equivalent for table AMs, forbid that.
Also make sure to throw a usable error when creating a table using an
index 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, in
GetTableAmRoutine().
Thirdly remove GetTableAmRoutineByAmId() - it was only used in an
earlier version of 8586bf7ed8
.
Add tests for the above (some for index AMs as well).
This commit is contained in:
parent
344b7e11bb
commit
ea97e440b8
@ -17,14 +17,12 @@
|
||||
#include "access/xact.h"
|
||||
#include "catalog/pg_am.h"
|
||||
#include "catalog/pg_proc.h"
|
||||
#include "commands/defrem.h"
|
||||
#include "utils/fmgroids.h"
|
||||
#include "utils/memutils.h"
|
||||
#include "utils/syscache.h"
|
||||
|
||||
|
||||
static Oid get_table_am_oid(const char *tableamname, bool missing_ok);
|
||||
|
||||
|
||||
/*
|
||||
* GetTableAmRoutine
|
||||
* Call the specified access method handler routine to get its
|
||||
@ -41,7 +39,7 @@ GetTableAmRoutine(Oid amhandler)
|
||||
routine = (TableAmRoutine *) DatumGetPointer(datum);
|
||||
|
||||
if (routine == NULL || !IsA(routine, TableAmRoutine))
|
||||
elog(ERROR, "Table access method handler %u did not return a TableAmRoutine struct",
|
||||
elog(ERROR, "table access method handler %u did not return a TableAmRoutine struct",
|
||||
amhandler);
|
||||
|
||||
/*
|
||||
@ -98,106 +96,30 @@ GetTableAmRoutine(Oid amhandler)
|
||||
return routine;
|
||||
}
|
||||
|
||||
/*
|
||||
* GetTableAmRoutineByAmId - look up the handler of the table access
|
||||
* method with the given OID, and get its TableAmRoutine struct.
|
||||
*/
|
||||
const TableAmRoutine *
|
||||
GetTableAmRoutineByAmId(Oid amoid)
|
||||
{
|
||||
regproc amhandler;
|
||||
HeapTuple tuple;
|
||||
Form_pg_am amform;
|
||||
|
||||
/* Get handler function OID for the access method */
|
||||
tuple = SearchSysCache1(AMOID, ObjectIdGetDatum(amoid));
|
||||
if (!HeapTupleIsValid(tuple))
|
||||
elog(ERROR, "cache lookup failed for access method %u",
|
||||
amoid);
|
||||
amform = (Form_pg_am) GETSTRUCT(tuple);
|
||||
|
||||
/* Check that it is a table access method */
|
||||
if (amform->amtype != AMTYPE_TABLE)
|
||||
ereport(ERROR,
|
||||
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
|
||||
errmsg("access method \"%s\" is not of type %s",
|
||||
NameStr(amform->amname), "TABLE")));
|
||||
|
||||
amhandler = amform->amhandler;
|
||||
|
||||
/* Complain if handler OID is invalid */
|
||||
if (!RegProcedureIsValid(amhandler))
|
||||
ereport(ERROR,
|
||||
(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
|
||||
errmsg("table access method \"%s\" does not have a handler",
|
||||
NameStr(amform->amname))));
|
||||
|
||||
ReleaseSysCache(tuple);
|
||||
|
||||
/* And finally, call the handler function to get the API struct. */
|
||||
return GetTableAmRoutine(amhandler);
|
||||
}
|
||||
|
||||
/*
|
||||
* get_table_am_oid - given a table access method name, look up the OID
|
||||
*
|
||||
* If missing_ok is false, throw an error if table access method name not
|
||||
* found. If true, just return InvalidOid.
|
||||
*/
|
||||
static Oid
|
||||
get_table_am_oid(const char *tableamname, bool missing_ok)
|
||||
{
|
||||
Oid result;
|
||||
Relation rel;
|
||||
TableScanDesc scandesc;
|
||||
HeapTuple tuple;
|
||||
ScanKeyData entry[1];
|
||||
|
||||
/*
|
||||
* Search pg_am. We use a heapscan here even though there is an index on
|
||||
* name, on the theory that pg_am will usually have just a few entries and
|
||||
* so an indexed lookup is a waste of effort.
|
||||
*/
|
||||
rel = heap_open(AccessMethodRelationId, AccessShareLock);
|
||||
|
||||
ScanKeyInit(&entry[0],
|
||||
Anum_pg_am_amname,
|
||||
BTEqualStrategyNumber, F_NAMEEQ,
|
||||
CStringGetDatum(tableamname));
|
||||
scandesc = table_beginscan_catalog(rel, 1, entry);
|
||||
tuple = heap_getnext(scandesc, ForwardScanDirection);
|
||||
|
||||
/* We assume that there can be at most one matching tuple */
|
||||
if (HeapTupleIsValid(tuple) &&
|
||||
((Form_pg_am) GETSTRUCT(tuple))->amtype == AMTYPE_TABLE)
|
||||
result = ((Form_pg_am) GETSTRUCT(tuple))->oid;
|
||||
else
|
||||
result = InvalidOid;
|
||||
|
||||
table_endscan(scandesc);
|
||||
heap_close(rel, AccessShareLock);
|
||||
|
||||
if (!OidIsValid(result) && !missing_ok)
|
||||
ereport(ERROR,
|
||||
(errcode(ERRCODE_UNDEFINED_OBJECT),
|
||||
errmsg("table access method \"%s\" does not exist",
|
||||
tableamname)));
|
||||
|
||||
return result;
|
||||
}
|
||||
|
||||
/* check_hook: validate new default_table_access_method */
|
||||
bool
|
||||
check_default_table_access_method(char **newval, void **extra, GucSource source)
|
||||
{
|
||||
if (**newval == '\0')
|
||||
{
|
||||
GUC_check_errdetail("default_table_access_method may not be empty.");
|
||||
return false;
|
||||
}
|
||||
|
||||
if (strlen(*newval) >= NAMEDATALEN)
|
||||
{
|
||||
GUC_check_errdetail("default_table_access_method is too long (maximum %d characters).",
|
||||
NAMEDATALEN - 1);
|
||||
return false;
|
||||
}
|
||||
|
||||
/*
|
||||
* If we aren't inside a transaction, we cannot do database access so
|
||||
* cannot verify the name. Must accept the value on faith.
|
||||
*/
|
||||
if (IsTransactionState())
|
||||
{
|
||||
if (**newval != '\0' &&
|
||||
!OidIsValid(get_table_am_oid(*newval, true)))
|
||||
if (!OidIsValid(get_table_am_oid(*newval, true)))
|
||||
{
|
||||
/*
|
||||
* When source == PGC_S_TEST, don't throw a hard error for a
|
||||
|
@ -189,6 +189,16 @@ get_index_am_oid(const char *amname, bool missing_ok)
|
||||
return get_am_type_oid(amname, AMTYPE_INDEX, missing_ok);
|
||||
}
|
||||
|
||||
/*
|
||||
* get_table_am_oid - given an access method name, look up its OID
|
||||
* and verify it corresponds to an table AM.
|
||||
*/
|
||||
Oid
|
||||
get_table_am_oid(const char *amname, bool missing_ok)
|
||||
{
|
||||
return get_am_type_oid(amname, AMTYPE_TABLE, missing_ok);
|
||||
}
|
||||
|
||||
/*
|
||||
* get_am_oid - given an access method name, look up its OID.
|
||||
* The type is not checked.
|
||||
|
@ -832,22 +832,9 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
|
||||
relkind == RELKIND_MATVIEW)
|
||||
accessMethod = default_table_access_method;
|
||||
|
||||
/*
|
||||
* look up the access method, verify it can handle the requested features
|
||||
*/
|
||||
/* look up the access method, verify it is for a table */
|
||||
if (accessMethod != NULL)
|
||||
{
|
||||
HeapTuple tuple;
|
||||
|
||||
tuple = SearchSysCache1(AMNAME, PointerGetDatum(accessMethod));
|
||||
if (!HeapTupleIsValid(tuple))
|
||||
ereport(ERROR,
|
||||
(errcode(ERRCODE_UNDEFINED_OBJECT),
|
||||
errmsg("table access method \"%s\" does not exist",
|
||||
accessMethod)));
|
||||
accessMethodId = ((Form_pg_am) GETSTRUCT(tuple))->oid;
|
||||
ReleaseSysCache(tuple);
|
||||
}
|
||||
accessMethodId = get_table_am_oid(accessMethod, false);
|
||||
|
||||
/*
|
||||
* Create the relation. Inherited defaults and constraints are passed in
|
||||
|
@ -1616,7 +1616,6 @@ extern void table_block_parallelscan_startblock_init(Relation rel,
|
||||
*/
|
||||
|
||||
extern const TableAmRoutine *GetTableAmRoutine(Oid amhandler);
|
||||
extern const TableAmRoutine *GetTableAmRoutineByAmId(Oid amoid);
|
||||
extern const TableAmRoutine *GetHeapamTableAmRoutine(void);
|
||||
extern bool check_default_table_access_method(char **newval, void **extra,
|
||||
GucSource source);
|
||||
|
@ -156,6 +156,7 @@ extern Datum transformGenericOptions(Oid catalogId,
|
||||
extern ObjectAddress CreateAccessMethod(CreateAmStmt *stmt);
|
||||
extern void RemoveAccessMethodById(Oid amOid);
|
||||
extern Oid get_index_am_oid(const char *amname, bool missing_ok);
|
||||
extern Oid get_table_am_oid(const char *amname, bool missing_ok);
|
||||
extern Oid get_am_oid(const char *amname, bool missing_ok);
|
||||
extern char *get_am_name(Oid amOid);
|
||||
|
||||
|
@ -3,6 +3,11 @@
|
||||
--
|
||||
-- Make gist2 over gisthandler. In fact, it would be a synonym to gist.
|
||||
CREATE ACCESS METHOD gist2 TYPE INDEX HANDLER gisthandler;
|
||||
-- Verify return type checks for handlers
|
||||
CREATE ACCESS METHOD bogus TYPE INDEX HANDLER int4in;
|
||||
ERROR: function int4in(internal) does not exist
|
||||
CREATE ACCESS METHOD bogus TYPE INDEX HANDLER heap_tableam_handler;
|
||||
ERROR: function heap_tableam_handler must return type index_am_handler
|
||||
-- Try to create gist2 index on fast_emp4000: fail because opclass doesn't exist
|
||||
CREATE INDEX grect2ind2 ON fast_emp4000 USING gist2 (home_base);
|
||||
ERROR: data type box has no default operator class for access method "gist2"
|
||||
@ -102,8 +107,24 @@ NOTICE: drop cascades to index grect2ind2
|
||||
--
|
||||
-- Test table access methods
|
||||
--
|
||||
-- prevent empty values
|
||||
SET default_table_access_method = '';
|
||||
ERROR: invalid value for parameter "default_table_access_method": ""
|
||||
DETAIL: default_table_access_method may not be empty.
|
||||
-- prevent nonexistant values
|
||||
SET default_table_access_method = 'I do not exist AM';
|
||||
ERROR: invalid value for parameter "default_table_access_method": "I do not exist AM"
|
||||
DETAIL: Table access method "I do not exist AM" does not exist.
|
||||
-- prevent setting it to an index AM
|
||||
SET default_table_access_method = 'btree';
|
||||
ERROR: access method "btree" is not of type TABLE
|
||||
-- Create a heap2 table am handler with heapam handler
|
||||
CREATE ACCESS METHOD heap2 TYPE TABLE HANDLER heap_tableam_handler;
|
||||
-- Verify return type checks for handlers
|
||||
CREATE ACCESS METHOD bogus TYPE TABLE HANDLER int4in;
|
||||
ERROR: function int4in(internal) does not exist
|
||||
CREATE ACCESS METHOD bogus TYPE TABLE HANDLER bthandler;
|
||||
ERROR: function bthandler must return type table_am_handler
|
||||
SELECT amname, amhandler, amtype FROM pg_am where amtype = 't' ORDER BY 1, 2;
|
||||
amname | amhandler | amtype
|
||||
--------+----------------------+--------
|
||||
@ -253,6 +274,18 @@ ORDER BY 3, 1, 2;
|
||||
|
||||
-- don't want to keep those tables, nor the default
|
||||
ROLLBACK;
|
||||
-- Third, check that we can neither create a table using a nonexistant
|
||||
-- AM, nor using an index AM
|
||||
CREATE TABLE i_am_a_failure() USING "";
|
||||
ERROR: zero-length delimited identifier at or near """"
|
||||
LINE 1: CREATE TABLE i_am_a_failure() USING "";
|
||||
^
|
||||
CREATE TABLE i_am_a_failure() USING i_do_not_exist_am;
|
||||
ERROR: access method "i_do_not_exist_am" does not exist
|
||||
CREATE TABLE i_am_a_failure() USING "I do not exist AM";
|
||||
ERROR: access method "I do not exist AM" does not exist
|
||||
CREATE TABLE i_am_a_failure() USING "btree";
|
||||
ERROR: access method "btree" is not of type TABLE
|
||||
-- Drop table access method, which fails as objects depends on it
|
||||
DROP ACCESS METHOD heap2;
|
||||
ERROR: cannot drop access method heap2 because other objects depend on it
|
||||
|
@ -520,6 +520,24 @@ WHERE p1.relkind IN ('S', 'v', 'f', 'c') and
|
||||
-----+---------
|
||||
(0 rows)
|
||||
|
||||
-- Indexes should have AMs of type 'i'
|
||||
SELECT pc.oid, pc.relname, pa.amname, pa.amtype
|
||||
FROM pg_class as pc JOIN pg_am AS pa ON (pc.relam = pa.oid)
|
||||
WHERE pc.relkind IN ('i') and
|
||||
pa.amtype != 'i';
|
||||
oid | relname | amname | amtype
|
||||
-----+---------+--------+--------
|
||||
(0 rows)
|
||||
|
||||
-- Tables, matviews etc should have AMs of type 't'
|
||||
SELECT pc.oid, pc.relname, pa.amname, pa.amtype
|
||||
FROM pg_class as pc JOIN pg_am AS pa ON (pc.relam = pa.oid)
|
||||
WHERE pc.relkind IN ('r', 't', 'm') and
|
||||
pa.amtype != 't';
|
||||
oid | relname | amname | amtype
|
||||
-----+---------+--------+--------
|
||||
(0 rows)
|
||||
|
||||
-- **************** pg_attribute ****************
|
||||
-- Look for illegal values in pg_attribute fields
|
||||
SELECT p1.attrelid, p1.attname
|
||||
|
@ -5,6 +5,11 @@
|
||||
-- Make gist2 over gisthandler. In fact, it would be a synonym to gist.
|
||||
CREATE ACCESS METHOD gist2 TYPE INDEX HANDLER gisthandler;
|
||||
|
||||
-- Verify return type checks for handlers
|
||||
CREATE ACCESS METHOD bogus TYPE INDEX HANDLER int4in;
|
||||
CREATE ACCESS METHOD bogus TYPE INDEX HANDLER heap_tableam_handler;
|
||||
|
||||
|
||||
-- Try to create gist2 index on fast_emp4000: fail because opclass doesn't exist
|
||||
CREATE INDEX grect2ind2 ON fast_emp4000 USING gist2 (home_base);
|
||||
|
||||
@ -72,11 +77,26 @@ DROP ACCESS METHOD gist2 CASCADE;
|
||||
-- Test table access methods
|
||||
--
|
||||
|
||||
-- prevent empty values
|
||||
SET default_table_access_method = '';
|
||||
|
||||
-- prevent nonexistant values
|
||||
SET default_table_access_method = 'I do not exist AM';
|
||||
|
||||
-- prevent setting it to an index AM
|
||||
SET default_table_access_method = 'btree';
|
||||
|
||||
|
||||
-- Create a heap2 table am handler with heapam handler
|
||||
CREATE ACCESS METHOD heap2 TYPE TABLE HANDLER heap_tableam_handler;
|
||||
|
||||
-- Verify return type checks for handlers
|
||||
CREATE ACCESS METHOD bogus TYPE TABLE HANDLER int4in;
|
||||
CREATE ACCESS METHOD bogus TYPE TABLE HANDLER bthandler;
|
||||
|
||||
SELECT amname, amhandler, amtype FROM pg_am where amtype = 't' ORDER BY 1, 2;
|
||||
|
||||
|
||||
-- First create tables employing the new AM using USING
|
||||
|
||||
-- plain CREATE TABLE
|
||||
@ -178,6 +198,13 @@ ORDER BY 3, 1, 2;
|
||||
-- don't want to keep those tables, nor the default
|
||||
ROLLBACK;
|
||||
|
||||
-- Third, check that we can neither create a table using a nonexistant
|
||||
-- AM, nor using an index AM
|
||||
CREATE TABLE i_am_a_failure() USING "";
|
||||
CREATE TABLE i_am_a_failure() USING i_do_not_exist_am;
|
||||
CREATE TABLE i_am_a_failure() USING "I do not exist AM";
|
||||
CREATE TABLE i_am_a_failure() USING "btree";
|
||||
|
||||
-- Drop table access method, which fails as objects depends on it
|
||||
DROP ACCESS METHOD heap2;
|
||||
|
||||
|
@ -379,6 +379,18 @@ FROM pg_class as p1
|
||||
WHERE p1.relkind IN ('S', 'v', 'f', 'c') and
|
||||
p1.relam != 0;
|
||||
|
||||
-- Indexes should have AMs of type 'i'
|
||||
SELECT pc.oid, pc.relname, pa.amname, pa.amtype
|
||||
FROM pg_class as pc JOIN pg_am AS pa ON (pc.relam = pa.oid)
|
||||
WHERE pc.relkind IN ('i') and
|
||||
pa.amtype != 'i';
|
||||
|
||||
-- Tables, matviews etc should have AMs of type 't'
|
||||
SELECT pc.oid, pc.relname, pa.amname, pa.amtype
|
||||
FROM pg_class as pc JOIN pg_am AS pa ON (pc.relam = pa.oid)
|
||||
WHERE pc.relkind IN ('r', 't', 'm') and
|
||||
pa.amtype != 't';
|
||||
|
||||
-- **************** pg_attribute ****************
|
||||
|
||||
-- Look for illegal values in pg_attribute fields
|
||||
|
Loading…
Reference in New Issue
Block a user