Revert "Custom reloptions for table AM"

This reverts commit c95c25f due to multiple
design issues spotted after commit.

Reported-by: Jeff Davis
Discussion: https://postgr.es/m/11550b536211d5748bb2865ed6cb3502ff073bf7.camel%40j-davis.com

Author: akorotkov

src/backend/access/common/reloptions.c

@@ -24,7 +24,6 @@
 #include "access/nbtree.h"
 #include "access/reloptions.h"
 #include "access/spgist_private.h"
-#include "access/tableam.h"
 #include "catalog/pg_type.h"
 #include "commands/defrem.h"
 #include "commands/tablespace.h"
@@ -1378,7 +1377,7 @@ untransformRelOptions(Datum options)
  */
 bytea *
 extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
-				  const TableAmRoutine *tableam, amoptions_function amoptions)
+				  amoptions_function amoptions)
 {
 	bytea	   *options;
 	bool		isnull;
@@ -1400,8 +1399,7 @@ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
 		case RELKIND_RELATION:
 		case RELKIND_TOASTVALUE:
 		case RELKIND_MATVIEW:
-			options = tableam_reloptions(tableam, classForm->relkind,
-										 datum, false);
+			options = heap_reloptions(classForm->relkind, datum, false);
 			break;
 		case RELKIND_PARTITIONED_TABLE:
 			options = partitioned_table_reloptions(datum, false);

src/backend/access/heap/heapam_handler.c

@@ -23,7 +23,6 @@
 #include "access/heapam.h"
 #include "access/heaptoast.h"
 #include "access/multixact.h"
-#include "access/reloptions.h"
 #include "access/rewriteheap.h"
 #include "access/syncscan.h"
 #include "access/tableam.h"
@@ -2158,16 +2157,6 @@ heapam_relation_toast_am(Relation rel)
 	return rel->rd_rel->relam;
 }
 
-static bytea *
-heapam_reloptions(char relkind, Datum reloptions, bool validate)
-{
-	Assert(relkind == RELKIND_RELATION ||
-		   relkind == RELKIND_TOASTVALUE ||
-		   relkind == RELKIND_MATVIEW);
-
-	return heap_reloptions(relkind, reloptions, validate);
-}
-
 
 /* ------------------------------------------------------------------------
  * Planner related callbacks for the heap AM
@@ -2673,7 +2662,6 @@ static const TableAmRoutine heapam_methods = {
 	.relation_needs_toast_table = heapam_relation_needs_toast_table,
 	.relation_toast_am = heapam_relation_toast_am,
 	.relation_fetch_toast_slice = heap_fetch_toast_slice,
-	.reloptions = heapam_reloptions,
 
 	.relation_estimate_size = heapam_estimate_rel_size,
 

src/backend/access/table/tableamapi.c

@@ -13,11 +13,9 @@
 
 #include "access/tableam.h"
 #include "access/xact.h"
-#include "catalog/pg_am.h"
 #include "commands/defrem.h"
 #include "miscadmin.h"
 #include "utils/guc_hooks.h"
-#include "utils/syscache.h"
 
 
 /*
@@ -100,29 +98,6 @@ GetTableAmRoutine(Oid amhandler)
 	return routine;
 }
 
-/*
- * GetTableAmRoutineByAmOid
- *		Given the table access method oid get its TableAmRoutine struct, which
- *		will be palloc'd in the caller's memory context.
- */
-const TableAmRoutine *
-GetTableAmRoutineByAmOid(Oid amoid)
-{
-	HeapTuple	ht_am;
-	Form_pg_am	amrec;
-	const TableAmRoutine *tableam = NULL;
-
-	ht_am = SearchSysCache1(AMOID, ObjectIdGetDatum(amoid));
-	if (!HeapTupleIsValid(ht_am))
-		elog(ERROR, "cache lookup failed for access method %u",
-			 amoid);
-	amrec = (Form_pg_am) GETSTRUCT(ht_am);
-
-	tableam = GetTableAmRoutine(amrec->amhandler);
-	ReleaseSysCache(ht_am);
-	return tableam;
-}
-
 /* check_hook: validate new default_table_access_method */
 bool
 check_default_table_access_method(char **newval, void **extra, GucSource source)

src/backend/commands/tablecmds.c

@@ -715,7 +715,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 	ObjectAddress address;
 	LOCKMODE	parentLockmode;
 	Oid			accessMethodId = InvalidOid;
-	const TableAmRoutine *tableam = NULL;
 
 	/*
 	 * Truncate relname to appropriate length (probably a waste of time, as
@@ -851,28 +850,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 	if (!OidIsValid(ownerId))
 		ownerId = GetUserId();
 
-	/*
-	 * For relations with table AM and partitioned tables, select access
-	 * method to use: an explicitly indicated one, or (in the case of a
-	 * partitioned table) the parent's, if it has one.
-	 */
-	if (stmt->accessMethod != NULL)
-	{
-		Assert(RELKIND_HAS_TABLE_AM(relkind) || relkind == RELKIND_PARTITIONED_TABLE);
-		accessMethodId = get_table_am_oid(stmt->accessMethod, false);
-	}
-	else if (RELKIND_HAS_TABLE_AM(relkind) || relkind == RELKIND_PARTITIONED_TABLE)
-	{
-		if (stmt->partbound)
-		{
-			Assert(list_length(inheritOids) == 1);
-			accessMethodId = get_rel_relam(linitial_oid(inheritOids));
-		}
-
-		if (RELKIND_HAS_TABLE_AM(relkind) && !OidIsValid(accessMethodId))
-			accessMethodId = get_table_am_oid(default_table_access_method, false);
-	}
-
 	/*
 	 * Parse and validate reloptions, if any.
 	 */
@@ -881,12 +858,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 
 	switch (relkind)
 	{
-		case RELKIND_RELATION:
-		case RELKIND_TOASTVALUE:
-		case RELKIND_MATVIEW:
-			tableam = GetTableAmRoutineByAmOid(accessMethodId);
-			(void) tableam_reloptions(tableam, relkind, reloptions, true);
-			break;
 		case RELKIND_VIEW:
 			(void) view_reloptions(reloptions, true);
 			break;
@@ -895,7 +866,6 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 			break;
 		default:
 			(void) heap_reloptions(relkind, reloptions, true);
-			break;
 	}
 
 	if (stmt->ofTypename)
@@ -987,6 +957,28 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 		}
 	}
 
+	/*
+	 * For relations with table AM and partitioned tables, select access
+	 * method to use: an explicitly indicated one, or (in the case of a
+	 * partitioned table) the parent's, if it has one.
+	 */
+	if (stmt->accessMethod != NULL)
+	{
+		Assert(RELKIND_HAS_TABLE_AM(relkind) || relkind == RELKIND_PARTITIONED_TABLE);
+		accessMethodId = get_table_am_oid(stmt->accessMethod, false);
+	}
+	else if (RELKIND_HAS_TABLE_AM(relkind) || relkind == RELKIND_PARTITIONED_TABLE)
+	{
+		if (stmt->partbound)
+		{
+			Assert(list_length(inheritOids) == 1);
+			accessMethodId = get_rel_relam(linitial_oid(inheritOids));
+		}
+
+		if (RELKIND_HAS_TABLE_AM(relkind) && !OidIsValid(accessMethodId))
+			accessMethodId = get_table_am_oid(default_table_access_method, false);
+	}
+
 	/*
 	 * Create the relation.  Inherited defaults and constraints are passed in
 	 * for immediate handling --- since they don't need parsing, they can be
@@ -15536,8 +15528,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 		case RELKIND_RELATION:
 		case RELKIND_TOASTVALUE:
 		case RELKIND_MATVIEW:
-			(void) table_reloptions(rel, rel->rd_rel->relkind,
-									newOptions, true);
+			(void) heap_reloptions(rel->rd_rel->relkind, newOptions, true);
 			break;
 		case RELKIND_PARTITIONED_TABLE:
 			(void) partitioned_table_reloptions(newOptions, true);

src/backend/postmaster/autovacuum.c

@@ -2661,9 +2661,7 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
 		   ((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_MATVIEW ||
 		   ((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_TOASTVALUE);
 
-	relopts = extractRelOptions(tup, pg_class_desc,
-								GetTableAmRoutineByAmOid(((Form_pg_class) GETSTRUCT(tup))->relam),
-								NULL);
+	relopts = extractRelOptions(tup, pg_class_desc, NULL);
 	if (relopts == NULL)
 		return NULL;
 

src/backend/utils/cache/relcache.c

@@ -33,7 +33,6 @@
 #include "access/htup_details.h"
 #include "access/multixact.h"
 #include "access/parallel.h"
-#include "access/relation.h"
 #include "access/reloptions.h"
 #include "access/sysattr.h"
 #include "access/table.h"
@@ -465,7 +464,6 @@ RelationParseRelOptions(Relation relation, HeapTuple tuple)
 {
 	bytea	   *options;
 	amoptions_function amoptsfn;
-	const TableAmRoutine *tableam = NULL;
 
 	relation->rd_options = NULL;
 
@@ -480,7 +478,6 @@ RelationParseRelOptions(Relation relation, HeapTuple tuple)
 		case RELKIND_VIEW:
 		case RELKIND_MATVIEW:
 		case RELKIND_PARTITIONED_TABLE:
-			tableam = relation->rd_tableam;
 			amoptsfn = NULL;
 			break;
 		case RELKIND_INDEX:
@@ -496,8 +493,7 @@ RelationParseRelOptions(Relation relation, HeapTuple tuple)
 	 * we might not have any other for pg_class yet (consider executing this
 	 * code for pg_class itself)
 	 */
-	options = extractRelOptions(tuple, GetPgClassDescriptor(),
-								tableam, amoptsfn);
+	options = extractRelOptions(tuple, GetPgClassDescriptor(), amoptsfn);
 
 	/*
 	 * Copy parsed data into CacheMemoryContext.  To guard against the

src/include/access/reloptions.h

@@ -21,7 +21,6 @@
 
 #include "access/amapi.h"
 #include "access/htup.h"
-#include "access/tableam.h"
 #include "access/tupdesc.h"
 #include "nodes/pg_list.h"
 #include "storage/lock.h"
@@ -225,7 +224,6 @@ extern Datum transformRelOptions(Datum oldOptions, List *defList,
 								 bool acceptOidsOff, bool isReset);
 extern List *untransformRelOptions(Datum options);
 extern bytea *extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
-								const TableAmRoutine *tableam,
 								amoptions_function amoptions);
 extern void *build_reloptions(Datum reloptions, bool validate,
 							  relopt_kind kind,

src/include/access/tableam.h

@@ -739,28 +739,6 @@ typedef struct TableAmRoutine
 											   int32 slicelength,
 											   struct varlena *result);
 
-	/*
-	 * This callback parses and validates the reloptions array for a table.
-	 *
-	 * This is called only when a non-null reloptions array exists for the
-	 * table.  'reloptions' is a text array containing entries of the form
-	 * "name=value".  The function should construct a bytea value, which will
-	 * be copied into the rd_options field of the table's relcache entry. The
-	 * data contents of the bytea value are open for the access method to
-	 * define.
-	 *
-	 * When 'validate' is true, the function should report a suitable error
-	 * message if any of the options are unrecognized or have invalid values;
-	 * when 'validate' is false, invalid entries should be silently ignored.
-	 * ('validate' is false when loading options already stored in pg_catalog;
-	 * an invalid entry could only be found if the access method has changed
-	 * its rules for options, and in that case ignoring obsolete entries is
-	 * appropriate.)
-	 *
-	 * It is OK to return NULL if default behavior is wanted.
-	 */
-	bytea	   *(*reloptions) (char relkind, Datum reloptions, bool validate);
-
 
 	/* ------------------------------------------------------------------------
 	 * Planner related functions.
@@ -1957,26 +1935,6 @@ table_relation_fetch_toast_slice(Relation toastrel, Oid valueid,
 													 result);
 }
 
-/*
- * Parse options for given table.
- */
-static inline bytea *
-table_reloptions(Relation rel, char relkind,
-				 Datum reloptions, bool validate)
-{
-	return rel->rd_tableam->reloptions(relkind, reloptions, validate);
-}
-
-/*
- * Parse table options without knowledge of particular table.
- */
-static inline bytea *
-tableam_reloptions(const TableAmRoutine *tableam, char relkind,
-				   Datum reloptions, bool validate)
-{
-	return tableam->reloptions(relkind, reloptions, validate);
-}
-
 
 /* ----------------------------------------------------------------------------
  * Planner related functionality
@@ -2155,7 +2113,6 @@ extern void table_block_relation_estimate_size(Relation rel,
  */
 
 extern const TableAmRoutine *GetTableAmRoutine(Oid amhandler);
-extern const TableAmRoutine *GetTableAmRoutineByAmOid(Oid amoid);
 
 /* ----------------------------------------------------------------------------
  * Functions in heapam_handler.c