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).

1 files changed, 16 insertions, 94 deletions

diff --git a/src/backend/access/table/tableamapi.c b/src/backend/access/table/tableamapi.c
index 51c0deaaf2e..6f3f638965b 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