diff --git a/src/backend/access/table/tableamapi.c b/src/backend/access/table/tableamapi.c index 51c0deaaf2..6f3f638965 100644 --- a/src/backend/access/table/tableamapi.c +++ b/src/backend/access/table/tableamapi.c @@ -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 diff --git a/src/backend/commands/amcmds.c b/src/backend/commands/amcmds.c index 24ca18018e..c1603737eb 100644 --- a/src/backend/commands/amcmds.c +++ b/src/backend/commands/amcmds.c @@ -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. diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 58eb7e1d8e..e842f9152b 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -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 diff --git a/src/include/access/tableam.h b/src/include/access/tableam.h index 90c329a88d..a647e7db32 100644 --- a/src/include/access/tableam.h +++ b/src/include/access/tableam.h @@ -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); diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h index 7f49625987..4003080323 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -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); diff --git a/src/test/regress/expected/create_am.out b/src/test/regress/expected/create_am.out index bf6addc8c5..352959b751 100644 --- a/src/test/regress/expected/create_am.out +++ b/src/test/regress/expected/create_am.out @@ -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 diff --git a/src/test/regress/expected/type_sanity.out b/src/test/regress/expected/type_sanity.out index 8f7f532f41..cd7fc03b04 100644 --- a/src/test/regress/expected/type_sanity.out +++ b/src/test/regress/expected/type_sanity.out @@ -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 diff --git a/src/test/regress/sql/create_am.sql b/src/test/regress/sql/create_am.sql index 8529105445..dc6c8ba2e9 100644 --- a/src/test/regress/sql/create_am.sql +++ b/src/test/regress/sql/create_am.sql @@ -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; diff --git a/src/test/regress/sql/type_sanity.sql b/src/test/regress/sql/type_sanity.sql index 821337b002..fed413bf98 100644 --- a/src/test/regress/sql/type_sanity.sql +++ b/src/test/regress/sql/type_sanity.sql @@ -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