[PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions

Started by Nikolay Shaplovabout 6 years ago15 messages

dhyan@nataraj.su

about 6 years ago

1 attachment(s)

In the thread
/messages/by-id/2620882.s52SJui4ql@x200m
I've suggested to split one big StdRdOption that is used for options storage
into into Options structures individual for each relkind and each relam

That patch have been split into smaller parts, most of them were already
commit:
https://commitfest.postgresql.org/25/2294/ - Remove StdRdOptions from AM
https://commitfest.postgresql.org/25/2297/ - Do not use StdRdOption for
partitioned tables
https://commitfest.postgresql.org/25/2295/ - Some useful Asserts for view-
related macroses.

And here goes the last part of StrRdOptions removal patch, where StdRdOptions
is replaced with HeapOptions and ToastOptions.

What did I do here.

- Added HeapOptions and ToastOptions structues
- Moved options building tab for autovacuum into AUTOVACUUM_RELOPTIONS macro,
so it can be used in relopt_parse_elt tab both for heap and toast
- Changed everywhere in the code, where old heap_reloptions is used, to use
new heap_reloptions or toast_reloptions
- Changed heap & toast option fetching macros to use HeapOptions and
ToastOptions
- Added Asserts to heap and toast options macros. Now we finally can do it.

What I did not do

- I've split fillfactor related macros into heap and toast like
RelationGetFillFactor will become HeapGetFillFactor and ToastGetFillFactor. I
have to do it, because now they handle different structure.
but there are heap only option macros like RelationGetParallelWorkers that
should be better called HeapGetParallelWorkers, as it is heap related. But I
did not change the name, as somebody from core team (I think it was Alvaro, it
was a while ago) asked me not to change macros names unless in is inavoidable.
So I kept the names, though I still think that naming them with Heap prefix
will make code more clear.

- vacuum_index_cleanup and vacuum_truncate options were added recently. They
were added into StdRdOptions. I think their place is inside AutoVacOpts not in
StdRdOptions, but did not dare to change it. If you see it the same way as I
see, please let me know, I will move it to a proper place.

--
Software Developer: https://www.upwork.com/freelancers/~014a87e140ff02c0da
Body-oriented Therapist: https://vk.com/nataraj_rebalancing (Russian)

Attachments:

StdRdOptions_to_HeapOptions_and_ToastOptions_v1.difftext/x-patch; charset=UTF-8; name=StdRdOptions_to_HeapOptions_and_ToastOptions_v1.diffDownload

diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 48377ac..8b18381 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -46,9 +46,8 @@
  * upper and lower bounds (if applicable); for strings, consider a validation
  * routine.
  * (ii) add a record below (or use add_<type>_reloption).
- * (iii) add it to the appropriate options struct (perhaps StdRdOptions)
- * (iv) add it to the appropriate handling routine (perhaps
- * default_reloptions)
+ * (iii) add it to the appropriate options struct
+ * (iv) add it to the appropriate handling routine
  * (v) make sure the lock level is set correctly for that operation
  * (vi) don't forget to document the option
  *
@@ -1106,9 +1105,11 @@ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
 	switch (classForm->relkind)
 	{
 		case RELKIND_RELATION:
-		case RELKIND_TOASTVALUE:
 		case RELKIND_MATVIEW:
-			options = heap_reloptions(classForm->relkind, datum, false);
+			options = heap_reloptions(datum, false);
+			break;
+		case RELKIND_TOASTVALUE:
+			options = toast_reloptions(datum, false);
 			break;
 		case RELKIND_PARTITIONED_TABLE:
 			options = partitioned_table_reloptions(datum, false);
@@ -1480,55 +1481,62 @@ fillRelOptions(void *rdopts, Size basesize,
 
 
 /*
- * Option parser for anything that uses StdRdOptions.
+ * Option parsing definition for autovacuum. Used in toast and heap options.
+ */
+
+#define AUTOVACUUM_RELOPTIONS(OFFSET)                                \
+		{"autovacuum_enabled", RELOPT_TYPE_BOOL,                     \
+		OFFSET + offsetof(AutoVacOpts, enabled)},                    \
+		{"autovacuum_vacuum_threshold", RELOPT_TYPE_INT,             \
+		OFFSET + offsetof(AutoVacOpts, vacuum_threshold)},           \
+		{"autovacuum_analyze_threshold", RELOPT_TYPE_INT,            \
+		OFFSET + offsetof(AutoVacOpts, analyze_threshold)},          \
+		{"autovacuum_vacuum_cost_delay", RELOPT_TYPE_REAL,           \
+		OFFSET + offsetof(AutoVacOpts, vacuum_cost_delay)},          \
+		{"autovacuum_vacuum_cost_limit", RELOPT_TYPE_INT,            \
+		OFFSET + offsetof(AutoVacOpts, vacuum_cost_limit)},          \
+		{"autovacuum_freeze_min_age", RELOPT_TYPE_INT,               \
+		OFFSET + offsetof(AutoVacOpts, freeze_min_age)},             \
+		{"autovacuum_freeze_max_age", RELOPT_TYPE_INT,               \
+		OFFSET + offsetof(AutoVacOpts, freeze_max_age)},             \
+		{"autovacuum_freeze_table_age", RELOPT_TYPE_INT,             \
+		OFFSET + offsetof(AutoVacOpts, freeze_table_age)},           \
+		{"autovacuum_multixact_freeze_min_age", RELOPT_TYPE_INT,     \
+		OFFSET + offsetof(AutoVacOpts, multixact_freeze_min_age)},   \
+		{"autovacuum_multixact_freeze_max_age", RELOPT_TYPE_INT,     \
+		OFFSET + offsetof(AutoVacOpts, multixact_freeze_max_age)},   \
+		{"autovacuum_multixact_freeze_table_age", RELOPT_TYPE_INT,   \
+		OFFSET + offsetof(AutoVacOpts, multixact_freeze_table_age)}, \
+		{"log_autovacuum_min_duration", RELOPT_TYPE_INT,             \
+		OFFSET + offsetof(AutoVacOpts, log_min_duration)},           \
+		{"autovacuum_vacuum_scale_factor", RELOPT_TYPE_REAL,         \
+		OFFSET + offsetof(AutoVacOpts, vacuum_scale_factor)},        \
+		{"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL,        \
+		OFFSET + offsetof(AutoVacOpts, analyze_scale_factor)}
+/*
+ * Option parser for heap
  */
 bytea *
-default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
+heap_reloptions(Datum reloptions, bool validate)
 {
 	static const relopt_parse_elt tab[] = {
-		{"fillfactor", RELOPT_TYPE_INT, offsetof(StdRdOptions, fillfactor)},
-		{"autovacuum_enabled", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, enabled)},
-		{"autovacuum_vacuum_threshold", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_threshold)},
-		{"autovacuum_analyze_threshold", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, analyze_threshold)},
-		{"autovacuum_vacuum_cost_limit", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_limit)},
-		{"autovacuum_freeze_min_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_min_age)},
-		{"autovacuum_freeze_max_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_max_age)},
-		{"autovacuum_freeze_table_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_table_age)},
-		{"autovacuum_multixact_freeze_min_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_min_age)},
-		{"autovacuum_multixact_freeze_max_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_max_age)},
-		{"autovacuum_multixact_freeze_table_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_table_age)},
-		{"log_autovacuum_min_duration", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, log_min_duration)},
+		{"fillfactor", RELOPT_TYPE_INT, offsetof(HeapOptions, fillfactor)},
+		AUTOVACUUM_RELOPTIONS(offsetof(HeapOptions, autovacuum)),
 		{"toast_tuple_target", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, toast_tuple_target)},
-		{"autovacuum_vacuum_cost_delay", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_delay)},
-		{"autovacuum_vacuum_scale_factor", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_scale_factor)},
-		{"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, analyze_scale_factor)},
+		offsetof(HeapOptions, toast_tuple_target)},
 		{"user_catalog_table", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, user_catalog_table)},
+		offsetof(HeapOptions, user_catalog_table)},
 		{"parallel_workers", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, parallel_workers)},
+		offsetof(HeapOptions, parallel_workers)},
 		{"vacuum_index_cleanup", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, vacuum_index_cleanup)},
+		offsetof(HeapOptions, vacuum_index_cleanup)},
 		{"vacuum_truncate", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, vacuum_truncate)}
+		offsetof(HeapOptions, vacuum_truncate)}
 	};
 
-	return (bytea *) build_reloptions(reloptions, validate, kind,
-									  sizeof(StdRdOptions),
+	return (bytea *) build_reloptions(reloptions, validate,
+									  RELOPT_KIND_HEAP,
+									  sizeof(HeapOptions),
 									  tab, lengthof(tab));
 }
 
@@ -1596,6 +1604,32 @@ partitioned_table_reloptions(Datum reloptions, bool validate)
 }
 
 /*
+ * Option parser for toast
+ */
+bytea *
+toast_reloptions(Datum reloptions, bool validate)
+{
+	ToastOptions *rdopts;
+	static const relopt_parse_elt tab[] = {
+		AUTOVACUUM_RELOPTIONS(offsetof(ToastOptions, autovacuum)),
+		{"vacuum_index_cleanup", RELOPT_TYPE_BOOL,
+		offsetof(ToastOptions, vacuum_index_cleanup)},
+		{"vacuum_truncate", RELOPT_TYPE_BOOL,
+		offsetof(ToastOptions, vacuum_truncate)}
+	};
+
+	rdopts = build_reloptions(reloptions, validate,
+									  RELOPT_KIND_TOAST,
+									  sizeof(ToastOptions),
+									  tab, lengthof(tab));
+	/* adjust default-only parameters for TOAST relations */
+	rdopts->autovacuum.analyze_threshold = -1;
+	rdopts->autovacuum.analyze_scale_factor = -1;
+
+	return (bytea *) rdopts;
+}
+
+/*
  * Option parser for views
  */
 bytea *
@@ -1615,37 +1649,6 @@ view_reloptions(Datum reloptions, bool validate)
 }
 
 /*
- * Parse options for heaps, views and toast tables.
- */
-bytea *
-heap_reloptions(char relkind, Datum reloptions, bool validate)
-{
-	StdRdOptions *rdopts;
-
-	switch (relkind)
-	{
-		case RELKIND_TOASTVALUE:
-			rdopts = (StdRdOptions *)
-				default_reloptions(reloptions, validate, RELOPT_KIND_TOAST);
-			if (rdopts != NULL)
-			{
-				/* adjust default-only parameters for TOAST relations */
-				rdopts->fillfactor = 100;
-				rdopts->autovacuum.analyze_threshold = -1;
-				rdopts->autovacuum.analyze_scale_factor = -1;
-			}
-			return (bytea *) rdopts;
-		case RELKIND_RELATION:
-		case RELKIND_MATVIEW:
-			return default_reloptions(reloptions, validate, RELOPT_KIND_HEAP);
-		default:
-			/* other relkinds are not supported */
-			return NULL;
-	}
-}
-
-
-/*
  * Parse options for indexes.
  *
  *	amoptions	index AM's option parser function
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 0128bb3..8c0d86c 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2120,8 +2120,10 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
 	AssertArg(!(options & HEAP_INSERT_NO_LOGICAL));
 
 	needwal = !(options & HEAP_INSERT_SKIP_WAL) && RelationNeedsWAL(relation);
-	saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
-												   HEAP_DEFAULT_FILLFACTOR);
+	if (IsToastRelation(relation))
+		saveFreeSpace = ToastGetTargetPageFreeSpace();
+	else
+		saveFreeSpace = HeapGetTargetPageFreeSpace(relation);
 
 	/* Toast and set header data in all the slots */
 	heaptuples = palloc(ntuples * sizeof(HeapTuple));
diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index d41d318..dad8c0d 100644
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -19,6 +19,7 @@
 #include "access/hio.h"
 #include "access/htup_details.h"
 #include "access/visibilitymap.h"
+#include "catalog/catalog.h"
 #include "storage/bufmgr.h"
 #include "storage/freespace.h"
 #include "storage/lmgr.h"
@@ -346,8 +347,10 @@ RelationGetBufferForTuple(Relation relation, Size len,
 						len, MaxHeapTupleSize)));
 
 	/* Compute desired extra freespace due to fillfactor option */
-	saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
-												   HEAP_DEFAULT_FILLFACTOR);
+	if (IsToastRelation(relation))
+		saveFreeSpace = ToastGetTargetPageFreeSpace();
+	else
+		saveFreeSpace = HeapGetTargetPageFreeSpace(relation);
 
 	if (otherBuffer != InvalidBuffer)
 		otherBlock = BufferGetBlockNumber(otherBuffer);
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 6d62a96..af9acf9 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -129,8 +129,11 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
 	 * important than sometimes getting a wrong answer in what is after all
 	 * just a heuristic estimate.
 	 */
-	minfree = RelationGetTargetPageFreeSpace(relation,
-											 HEAP_DEFAULT_FILLFACTOR);
+
+	if (IsToastRelation(relation))
+		minfree = ToastGetTargetPageFreeSpace();
+	else
+		minfree = HeapGetTargetPageFreeSpace(relation);
 	minfree = Max(minfree, BLCKSZ / 10);
 
 	if (PageIsFull(page) || PageGetHeapFreeSpace(page) < minfree)
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index d285b1f..a3f8095 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -674,8 +674,10 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
 						len, MaxHeapTupleSize)));
 
 	/* Compute desired extra freespace due to fillfactor option */
-	saveFreeSpace = RelationGetTargetPageFreeSpace(state->rs_new_rel,
-												   HEAP_DEFAULT_FILLFACTOR);
+	if (IsToastRelation(state->rs_new_rel))
+		saveFreeSpace = ToastGetTargetPageFreeSpace();
+	else
+		saveFreeSpace = HeapGetTargetPageFreeSpace(state->rs_new_rel);
 
 	/* Now we can check to see if there's enough free space already. */
 	if (state->rs_buffer_valid)
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 2bf7083..c30cafd 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -130,7 +130,7 @@ create_ctas_internal(List *attrList, IntoClause *into)
 										validnsps,
 										true, false);
 
-	(void) heap_reloptions(RELKIND_TOASTVALUE, toast_options, true);
+	(void) toast_reloptions(toast_options, true);
 
 	NewRelationCreateToastTable(intoRelationAddr.objectId, toast_options);
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 5440eb9..2130896 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -727,8 +727,12 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 		case RELKIND_PARTITIONED_TABLE:
 			(void) partitioned_table_reloptions(reloptions, true);
 			break;
-		default:
-			(void) heap_reloptions(relkind, reloptions, true);
+		case RELKIND_TOASTVALUE:
+			(void) toast_reloptions(reloptions, true);
+			break;
+		case RELKIND_RELATION:
+		case RELKIND_MATVIEW:
+			(void) heap_reloptions(reloptions, true);
 	}
 
 	if (stmt->ofTypename)
@@ -12194,9 +12198,11 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 	switch (rel->rd_rel->relkind)
 	{
 		case RELKIND_RELATION:
-		case RELKIND_TOASTVALUE:
 		case RELKIND_MATVIEW:
-			(void) heap_reloptions(rel->rd_rel->relkind, newOptions, true);
+			(void) heap_reloptions( newOptions, true);
+			break;
+		case RELKIND_TOASTVALUE:
+			(void) toast_reloptions(newOptions, true);
 			break;
 		case RELKIND_PARTITIONED_TABLE:
 			(void) partitioned_table_reloptions(newOptions, true);
@@ -12308,7 +12314,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 										 defList, "toast", validnsps, false,
 										 operation == AT_ResetRelOptions);
 
-		(void) heap_reloptions(RELKIND_TOASTVALUE, newOptions, true);
+		(void) toast_reloptions(newOptions, true);
 
 		memset(repl_val, 0, sizeof(repl_val));
 		memset(repl_null, false, sizeof(repl_null));
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index da1da23..49cac91 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -31,6 +31,7 @@
 #include "access/tableam.h"
 #include "access/transam.h"
 #include "access/xact.h"
+#include "catalog/catalog.h"
 #include "catalog/namespace.h"
 #include "catalog/pg_database.h"
 #include "catalog/pg_inherits.h"
@@ -1769,7 +1770,10 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	if (params->index_cleanup == VACOPT_TERNARY_DEFAULT)
 	{
 		if (onerel->rd_options == NULL ||
-			((StdRdOptions *) onerel->rd_options)->vacuum_index_cleanup)
+			(!IsToastRelation(onerel) &&
+			 ((HeapOptions *) onerel->rd_options)->vacuum_index_cleanup) ||
+			(IsToastRelation(onerel) &&
+			 ((ToastOptions *) onerel->rd_options)->vacuum_index_cleanup))
 			params->index_cleanup = VACOPT_TERNARY_ENABLED;
 		else
 			params->index_cleanup = VACOPT_TERNARY_DISABLED;
@@ -1779,7 +1783,10 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	if (params->truncate == VACOPT_TERNARY_DEFAULT)
 	{
 		if (onerel->rd_options == NULL ||
-			((StdRdOptions *) onerel->rd_options)->vacuum_truncate)
+			(!IsToastRelation(onerel) &&
+			 ((HeapOptions *) onerel->rd_options)->vacuum_truncate) ||
+			(IsToastRelation(onerel) &&
+			 ((ToastOptions *) onerel->rd_options)->vacuum_truncate))
 			params->truncate = VACOPT_TERNARY_ENABLED;
 		else
 			params->truncate = VACOPT_TERNARY_DISABLED;
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 5e889d1..cd564c9 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -150,8 +150,15 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
 		estimate_rel_size(relation, rel->attr_widths - rel->min_attr,
 						  &rel->pages, &rel->tuples, &rel->allvisfrac);
 
-	/* Retrieve the parallel_workers reloption, or -1 if not set. */
-	rel->rel_parallel_workers = RelationGetParallelWorkers(relation, -1);
+	/*
+	 * Retrieve the parallel_workers for heap and mat.view relations.
+	 * Use -1 if not set, or if we are dealing with other relation kinds
+	 */
+	if (relation->rd_rel->relkind == RELKIND_RELATION ||
+		relation->rd_rel->relkind == RELKIND_MATVIEW)
+		rel->rel_parallel_workers = RelationGetParallelWorkers(relation, -1);
+	else
+		rel->rel_parallel_workers = -1;
 
 	/*
 	 * Make list of indexes.  Ignore indexes on system catalogs if told to.
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index c1dd816..0e8596b 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2722,6 +2722,7 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
 {
 	bytea	   *relopts;
 	AutoVacOpts *av;
+	AutoVacOpts *src;
 
 	Assert(((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_RELATION ||
 		   ((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_MATVIEW ||
@@ -2731,8 +2732,13 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
 	if (relopts == NULL)
 		return NULL;
 
+	if (((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_TOASTVALUE)
+		src = &(((ToastOptions *) relopts)->autovacuum);
+	else
+		src = &(((HeapOptions *) relopts)->autovacuum);
+
 	av = palloc(sizeof(AutoVacOpts));
-	memcpy(av, &(((StdRdOptions *) relopts)->autovacuum), sizeof(AutoVacOpts));
+	memcpy(av, src, sizeof(AutoVacOpts));
 	pfree(relopts);
 
 	return av;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 3a03ca7..6cc6119 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1030,9 +1030,7 @@ ProcessUtilitySlow(ParseState *pstate,
 																validnsps,
 																true,
 																false);
-							(void) heap_reloptions(RELKIND_TOASTVALUE,
-												   toast_options,
-												   true);
+							(void) toast_reloptions(toast_options, true);
 
 							NewRelationCreateToastTable(address.objectId,
 														toast_options);
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index d21c513..e642c3c 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -186,9 +186,8 @@ extern void *build_reloptions(Datum reloptions, bool validate,
 							  const relopt_parse_elt *relopt_elems,
 							  int num_relopt_elems);
 
-extern bytea *default_reloptions(Datum reloptions, bool validate,
-								 relopt_kind kind);
-extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
+extern bytea *toast_reloptions(Datum reloptions, bool validate);
+extern bytea *heap_reloptions(Datum reloptions, bool validate);
 extern bytea *view_reloptions(Datum reloptions, bool validate);
 extern bytea *partitioned_table_reloptions(Datum reloptions, bool validate);
 extern bytea *index_reloptions(amoptions_function amoptions, Datum reloptions,
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 31d8a1a..72f216f 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -234,14 +234,9 @@ typedef struct ForeignKeyCacheInfo
 
 
 /*
- * StdRdOptions
- *		Standard contents of rd_options for heaps.
- *
- * RelationGetFillFactor() and RelationGetTargetPageFreeSpace() can only
- * be applied to relations that use this format or a superset for
- * private options data.
+ * AutoVacOpts
+ *		Auto Vacuum options used both in Heap and Toast relations
  */
- /* autovacuum-related reloptions. */
 typedef struct AutoVacOpts
 {
 	bool		enabled;
@@ -260,7 +255,11 @@ typedef struct AutoVacOpts
 	float8		analyze_scale_factor;
 } AutoVacOpts;
 
-typedef struct StdRdOptions
+/*
+ * HeapOptions
+ *		Binary representation of relation options for Heap relations.
+ */
+typedef struct HeapOptions
 {
 	int32		vl_len_;		/* varlena header (do not touch directly!) */
 	int			fillfactor;		/* page fill factor in percent (0..100) */
@@ -271,40 +270,60 @@ typedef struct StdRdOptions
 	int			parallel_workers;	/* max number of parallel workers */
 	bool		vacuum_index_cleanup;	/* enables index vacuuming and cleanup */
 	bool		vacuum_truncate;	/* enables vacuum to truncate a relation */
-} StdRdOptions;
+} HeapOptions;
 
 #define HEAP_MIN_FILLFACTOR			10
 #define HEAP_DEFAULT_FILLFACTOR		100
 
 /*
+ * ToastOptions
+ *		Binary representation of relation options for Toast relations.
+ */
+typedef struct ToastOptions
+{
+	int32		vl_len_;		/* varlena header (do not touch directly!) */
+	AutoVacOpts autovacuum;		/* autovacuum-related options */
+	bool		vacuum_index_cleanup; /* enables index vacuuming and cleanup */
+	bool		vacuum_truncate;	/* enables vacuum to truncate a relation */
+} ToastOptions;
+
+#define TOAST_DEFAULT_FILLFACTOR	100 /* Only default is actually used */
+
+/*
  * RelationGetToastTupleTarget
- *		Returns the relation's toast_tuple_target.  Note multiple eval of argument!
+ *		Returns the heap's toast_tuple_target.  Note multiple eval of argument!
  */
-#define RelationGetToastTupleTarget(relation, defaulttarg) \
-	((relation)->rd_options ? \
-	 ((StdRdOptions *) (relation)->rd_options)->toast_tuple_target : (defaulttarg))
+#define RelationGetToastTupleTarget(relation, defaulttarg)				\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION ||		\
+				 relation->rd_rel->relkind == RELKIND_MATVIEW),			\
+	 (relation)->rd_options ? 											\
+	  ((HeapOptions *) (relation)->rd_options)->toast_tuple_target : \
+			(defaulttarg))
 
 /*
- * RelationGetFillFactor
- *		Returns the relation's fillfactor.  Note multiple eval of argument!
+ * HeapGetFillFactor
+ *		Returns the heap's fillfactor.  Note multiple eval of argument!
  */
-#define RelationGetFillFactor(relation, defaultff) \
-	((relation)->rd_options ? \
-	 ((StdRdOptions *) (relation)->rd_options)->fillfactor : (defaultff))
+#define HeapGetFillFactor(relation)									\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION ||		\
+				 relation->rd_rel->relkind == RELKIND_MATVIEW),			\
+	 (relation)->rd_options ? 											\
+	  ((HeapOptions *) (relation)->rd_options)->fillfactor : 		\
+									(HEAP_DEFAULT_FILLFACTOR))
 
 /*
- * RelationGetTargetPageUsage
+ * HeapGetTargetPageUsage
  *		Returns the relation's desired space usage per page in bytes.
  */
-#define RelationGetTargetPageUsage(relation, defaultff) \
-	(BLCKSZ * RelationGetFillFactor(relation, defaultff) / 100)
+#define HeapGetTargetPageUsage(relation) \
+	(BLCKSZ * HeapGetFillFactor(relation) / 100)
 
 /*
- * RelationGetTargetPageFreeSpace
+ * HeapGetTargetPageFreeSpace
  *		Returns the relation's desired freespace per page in bytes.
  */
-#define RelationGetTargetPageFreeSpace(relation, defaultff) \
-	(BLCKSZ * (100 - RelationGetFillFactor(relation, defaultff)) / 100)
+#define HeapGetTargetPageFreeSpace(relation) \
+	(BLCKSZ * (100 - HeapGetFillFactor(relation)) / 100)
 
 /*
  * RelationIsUsedAsCatalogTable
@@ -315,16 +334,27 @@ typedef struct StdRdOptions
 	((relation)->rd_options && \
 	 ((relation)->rd_rel->relkind == RELKIND_RELATION || \
 	  (relation)->rd_rel->relkind == RELKIND_MATVIEW) ? \
-	 ((StdRdOptions *) (relation)->rd_options)->user_catalog_table : false)
+	 ((HeapOptions *) (relation)->rd_options)->user_catalog_table : false)
 
 /*
  * RelationGetParallelWorkers
- *		Returns the relation's parallel_workers reloption setting.
+ *		Returns the heap's parallel_workers reloption setting.
  *		Note multiple eval of argument!
  */
-#define RelationGetParallelWorkers(relation, defaultpw) \
-	((relation)->rd_options ? \
-	 ((StdRdOptions *) (relation)->rd_options)->parallel_workers : (defaultpw))
+#define RelationGetParallelWorkers(relation, defaultpw) 				\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION ||		\
+				 relation->rd_rel->relkind == RELKIND_MATVIEW),			\
+	 (relation)->rd_options ? 											\
+	  ((HeapOptions *) (relation)->rd_options)->parallel_workers : 	\
+			(defaultpw))
+
+/*
+ * ToastGetTargetPageFreeSpace
+ *		Returns the TOAST relation's desired freespace per page in bytes.
+ *		Always calculated using default fillfactor value.
+ */
+#define ToastGetTargetPageFreeSpace() \
+	(BLCKSZ * (100 - TOAST_DEFAULT_FILLFACTOR) / 100)
 
 /* ViewOptions->check_option values */
 typedef enum ViewOptCheckOption

Michael Paquier

michael@paquier.xyz

about 6 years ago

In reply to: Nikolay Shaplov (#1)

Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions

On Fri, Dec 06, 2019 at 06:18:48PM +0300, Nikolay Shaplov wrote:

In the thread
/messages/by-id/2620882.s52SJui4ql@x200m
I've suggested to split one big StdRdOption that is used for options storage
into into Options structures individual for each relkind and each relam

And here goes the last part of StrRdOptions removal patch, where StdRdOptions
is replaced with HeapOptions and ToastOptions.

-typedef struct StdRdOptions
+/*
+ * HeapOptions
+ *     Binary representation of relation options for Heap relations.
+ */
+typedef struct HeapOptions

I think that it makes sense to split relation options dedicated to
heap into their own parsing structure, because those options are
actually related to the table AM heap. However, I think that this
patch is not ambitious enough in the work which is done and that
things could move into a more generic direction. At the end of the
day, I'd like to think that we should have something like:
- Heap-related reloptions are built as part of its AM handler in
heapam_handler.c, with reloptions.c holding no more references to
heap. At all.
- The table AM option parsing follows a model close to what is done
for indexes in terms of option parsing, moving the responsibility to
define relation options to each table AM.
- Toast is an interesting case, as table AMs may want to use toast
tables. Or not. Robert may be able to comment more on that as he has
worked in this area for bd12499.
--
Michael

Nikolay Shaplov

dhyan@nataraj.su

almost 6 years ago

In reply to: Michael Paquier (#2)

1 attachment(s)

Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions

В письме от понедельник, 9 декабря 2019 г. 12:11:17 MSK пользователь Michael
Paquier написал:

On Fri, Dec 06, 2019 at 06:18:48PM +0300, Nikolay Shaplov wrote:

In the thread
/messages/by-id/2620882.s52SJui4ql@x200m
I've suggested to split one big StdRdOption that is used for options
storage into into Options structures individual for each relkind and each
relam

And here goes the last part of StrRdOptions removal patch, where
StdRdOptions is replaced with HeapOptions and ToastOptions.
-typedef struct StdRdOptions
+/*
+ * HeapOptions
+ *     Binary representation of relation options for Heap relations.
+ */
+typedef struct HeapOptions
I think that it makes sense to split relation options dedicated to
heap into their own parsing structure, because those options are
actually related to the table AM heap. However, I think that this
patch is not ambitious enough in the work which is done and that
things could move into a more generic direction. At the end of the
day, I'd like to think that we should have something like:
- Heap-related reloptions are built as part of its AM handler in
heapam_handler.c, with reloptions.c holding no more references to
heap. At all.
- The table AM option parsing follows a model close to what is done
for indexes in terms of option parsing, moving the responsibility to
define relation options to each table AM.
- Toast is an interesting case, as table AMs may want to use toast
tables. Or not. Robert may be able to comment more on that as he has
worked in this area for bd12499.

Oh, yeah, I forget that relations now also have AM :-)

But the truth is that my goal is to move all code that defines all option
names, min/max values etc, move it inside am code. To move data from
boolRelOpts, intRelOpts, realRelOpts, enumRelOpts, enumRelOpts from
reloptions.c into the code that implement AMs that uses these options.

I did it for indexes in patch I've offered several years ago. Now we have also
relaion AM.

But I would prefer to fix index AM relioptions first, and then copy that
solution for relations.

Because if I frist copy AM solution from indexes to relation, then I will have
to fix it in two places.

So I would prefer to keep reloptions for relations in relations.c, only split
them into HeapOptions and ToastOptions, then change AM for indexes moving
option definition into AM's and then clone the solution for relations.

This seems to be most simple and most logical way.

PS. I've checked the patch against current master. No changes were needed, but
I am attaching a diff made against current master, just in case.

Attachments:

StdRdOptions_to_HeapOptions_and_ToastOptions_v1a.difftext/x-patch; charset=UTF-8; name=StdRdOptions_to_HeapOptions_and_ToastOptions_v1a.diffDownload

diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index c3d45c7..73d5e4f 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -46,9 +46,8 @@
  * upper and lower bounds (if applicable); for strings, consider a validation
  * routine.
  * (ii) add a record below (or use add_<type>_reloption).
- * (iii) add it to the appropriate options struct (perhaps StdRdOptions)
- * (iv) add it to the appropriate handling routine (perhaps
- * default_reloptions)
+ * (iii) add it to the appropriate options struct
+ * (iv) add it to the appropriate handling routine
  * (v) make sure the lock level is set correctly for that operation
  * (vi) don't forget to document the option
  *
@@ -1116,9 +1115,11 @@ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
 	switch (classForm->relkind)
 	{
 		case RELKIND_RELATION:
-		case RELKIND_TOASTVALUE:
 		case RELKIND_MATVIEW:
-			options = heap_reloptions(classForm->relkind, datum, false);
+			options = heap_reloptions(datum, false);
+			break;
+		case RELKIND_TOASTVALUE:
+			options = toast_reloptions(datum, false);
 			break;
 		case RELKIND_PARTITIONED_TABLE:
 			options = partitioned_table_reloptions(datum, false);
@@ -1490,55 +1491,62 @@ fillRelOptions(void *rdopts, Size basesize,
 
 
 /*
- * Option parser for anything that uses StdRdOptions.
+ * Option parsing definition for autovacuum. Used in toast and heap options.
+ */
+
+#define AUTOVACUUM_RELOPTIONS(OFFSET)                                \
+		{"autovacuum_enabled", RELOPT_TYPE_BOOL,                     \
+		OFFSET + offsetof(AutoVacOpts, enabled)},                    \
+		{"autovacuum_vacuum_threshold", RELOPT_TYPE_INT,             \
+		OFFSET + offsetof(AutoVacOpts, vacuum_threshold)},           \
+		{"autovacuum_analyze_threshold", RELOPT_TYPE_INT,            \
+		OFFSET + offsetof(AutoVacOpts, analyze_threshold)},          \
+		{"autovacuum_vacuum_cost_delay", RELOPT_TYPE_REAL,           \
+		OFFSET + offsetof(AutoVacOpts, vacuum_cost_delay)},          \
+		{"autovacuum_vacuum_cost_limit", RELOPT_TYPE_INT,            \
+		OFFSET + offsetof(AutoVacOpts, vacuum_cost_limit)},          \
+		{"autovacuum_freeze_min_age", RELOPT_TYPE_INT,               \
+		OFFSET + offsetof(AutoVacOpts, freeze_min_age)},             \
+		{"autovacuum_freeze_max_age", RELOPT_TYPE_INT,               \
+		OFFSET + offsetof(AutoVacOpts, freeze_max_age)},             \
+		{"autovacuum_freeze_table_age", RELOPT_TYPE_INT,             \
+		OFFSET + offsetof(AutoVacOpts, freeze_table_age)},           \
+		{"autovacuum_multixact_freeze_min_age", RELOPT_TYPE_INT,     \
+		OFFSET + offsetof(AutoVacOpts, multixact_freeze_min_age)},   \
+		{"autovacuum_multixact_freeze_max_age", RELOPT_TYPE_INT,     \
+		OFFSET + offsetof(AutoVacOpts, multixact_freeze_max_age)},   \
+		{"autovacuum_multixact_freeze_table_age", RELOPT_TYPE_INT,   \
+		OFFSET + offsetof(AutoVacOpts, multixact_freeze_table_age)}, \
+		{"log_autovacuum_min_duration", RELOPT_TYPE_INT,             \
+		OFFSET + offsetof(AutoVacOpts, log_min_duration)},           \
+		{"autovacuum_vacuum_scale_factor", RELOPT_TYPE_REAL,         \
+		OFFSET + offsetof(AutoVacOpts, vacuum_scale_factor)},        \
+		{"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL,        \
+		OFFSET + offsetof(AutoVacOpts, analyze_scale_factor)}
+/*
+ * Option parser for heap
  */
 bytea *
-default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
+heap_reloptions(Datum reloptions, bool validate)
 {
 	static const relopt_parse_elt tab[] = {
-		{"fillfactor", RELOPT_TYPE_INT, offsetof(StdRdOptions, fillfactor)},
-		{"autovacuum_enabled", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, enabled)},
-		{"autovacuum_vacuum_threshold", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_threshold)},
-		{"autovacuum_analyze_threshold", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, analyze_threshold)},
-		{"autovacuum_vacuum_cost_limit", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_limit)},
-		{"autovacuum_freeze_min_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_min_age)},
-		{"autovacuum_freeze_max_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_max_age)},
-		{"autovacuum_freeze_table_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_table_age)},
-		{"autovacuum_multixact_freeze_min_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_min_age)},
-		{"autovacuum_multixact_freeze_max_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_max_age)},
-		{"autovacuum_multixact_freeze_table_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_table_age)},
-		{"log_autovacuum_min_duration", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, log_min_duration)},
+		{"fillfactor", RELOPT_TYPE_INT, offsetof(HeapOptions, fillfactor)},
+		AUTOVACUUM_RELOPTIONS(offsetof(HeapOptions, autovacuum)),
 		{"toast_tuple_target", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, toast_tuple_target)},
-		{"autovacuum_vacuum_cost_delay", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_delay)},
-		{"autovacuum_vacuum_scale_factor", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_scale_factor)},
-		{"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, analyze_scale_factor)},
+		offsetof(HeapOptions, toast_tuple_target)},
 		{"user_catalog_table", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, user_catalog_table)},
+		offsetof(HeapOptions, user_catalog_table)},
 		{"parallel_workers", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, parallel_workers)},
+		offsetof(HeapOptions, parallel_workers)},
 		{"vacuum_index_cleanup", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, vacuum_index_cleanup)},
+		offsetof(HeapOptions, vacuum_index_cleanup)},
 		{"vacuum_truncate", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, vacuum_truncate)}
+		offsetof(HeapOptions, vacuum_truncate)}
 	};
 
-	return (bytea *) build_reloptions(reloptions, validate, kind,
-									  sizeof(StdRdOptions),
+	return (bytea *) build_reloptions(reloptions, validate,
+									  RELOPT_KIND_HEAP,
+									  sizeof(HeapOptions),
 									  tab, lengthof(tab));
 }
 
@@ -1606,6 +1614,32 @@ partitioned_table_reloptions(Datum reloptions, bool validate)
 }
 
 /*
+ * Option parser for toast
+ */
+bytea *
+toast_reloptions(Datum reloptions, bool validate)
+{
+	ToastOptions *rdopts;
+	static const relopt_parse_elt tab[] = {
+		AUTOVACUUM_RELOPTIONS(offsetof(ToastOptions, autovacuum)),
+		{"vacuum_index_cleanup", RELOPT_TYPE_BOOL,
+		offsetof(ToastOptions, vacuum_index_cleanup)},
+		{"vacuum_truncate", RELOPT_TYPE_BOOL,
+		offsetof(ToastOptions, vacuum_truncate)}
+	};
+
+	rdopts = build_reloptions(reloptions, validate,
+									  RELOPT_KIND_TOAST,
+									  sizeof(ToastOptions),
+									  tab, lengthof(tab));
+	/* adjust default-only parameters for TOAST relations */
+	rdopts->autovacuum.analyze_threshold = -1;
+	rdopts->autovacuum.analyze_scale_factor = -1;
+
+	return (bytea *) rdopts;
+}
+
+/*
  * Option parser for views
  */
 bytea *
@@ -1625,37 +1659,6 @@ view_reloptions(Datum reloptions, bool validate)
 }
 
 /*
- * Parse options for heaps, views and toast tables.
- */
-bytea *
-heap_reloptions(char relkind, Datum reloptions, bool validate)
-{
-	StdRdOptions *rdopts;
-
-	switch (relkind)
-	{
-		case RELKIND_TOASTVALUE:
-			rdopts = (StdRdOptions *)
-				default_reloptions(reloptions, validate, RELOPT_KIND_TOAST);
-			if (rdopts != NULL)
-			{
-				/* adjust default-only parameters for TOAST relations */
-				rdopts->fillfactor = 100;
-				rdopts->autovacuum.analyze_threshold = -1;
-				rdopts->autovacuum.analyze_scale_factor = -1;
-			}
-			return (bytea *) rdopts;
-		case RELKIND_RELATION:
-		case RELKIND_MATVIEW:
-			return default_reloptions(reloptions, validate, RELOPT_KIND_HEAP);
-		default:
-			/* other relkinds are not supported */
-			return NULL;
-	}
-}
-
-
-/*
  * Parse options for indexes.
  *
  *	amoptions	index AM's option parser function
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 5a32e62..e67e8f3 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2123,8 +2123,10 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
 	AssertArg(!(options & HEAP_INSERT_NO_LOGICAL));
 
 	needwal = !(options & HEAP_INSERT_SKIP_WAL) && RelationNeedsWAL(relation);
-	saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
-												   HEAP_DEFAULT_FILLFACTOR);
+	if (IsToastRelation(relation))
+		saveFreeSpace = ToastGetTargetPageFreeSpace();
+	else
+		saveFreeSpace = HeapGetTargetPageFreeSpace(relation);
 
 	/* Toast and set header data in all the slots */
 	heaptuples = palloc(ntuples * sizeof(HeapTuple));
diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index aa3f14c..7b0cb75 100644
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -19,6 +19,7 @@
 #include "access/hio.h"
 #include "access/htup_details.h"
 #include "access/visibilitymap.h"
+#include "catalog/catalog.h"
 #include "storage/bufmgr.h"
 #include "storage/freespace.h"
 #include "storage/lmgr.h"
@@ -346,8 +347,10 @@ RelationGetBufferForTuple(Relation relation, Size len,
 						len, MaxHeapTupleSize)));
 
 	/* Compute desired extra freespace due to fillfactor option */
-	saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
-												   HEAP_DEFAULT_FILLFACTOR);
+	if (IsToastRelation(relation))
+		saveFreeSpace = ToastGetTargetPageFreeSpace();
+	else
+		saveFreeSpace = HeapGetTargetPageFreeSpace(relation);
 
 	if (otherBuffer != InvalidBuffer)
 		otherBlock = BufferGetBlockNumber(otherBuffer);
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 1794cfd..cf020e2 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -129,8 +129,11 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
 	 * important than sometimes getting a wrong answer in what is after all
 	 * just a heuristic estimate.
 	 */
-	minfree = RelationGetTargetPageFreeSpace(relation,
-											 HEAP_DEFAULT_FILLFACTOR);
+
+	if (IsToastRelation(relation))
+		minfree = ToastGetTargetPageFreeSpace();
+	else
+		minfree = HeapGetTargetPageFreeSpace(relation);
 	minfree = Max(minfree, BLCKSZ / 10);
 
 	if (PageIsFull(page) || PageGetHeapFreeSpace(page) < minfree)
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 9c29bc0..d872ce3 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -674,8 +674,10 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
 						len, MaxHeapTupleSize)));
 
 	/* Compute desired extra freespace due to fillfactor option */
-	saveFreeSpace = RelationGetTargetPageFreeSpace(state->rs_new_rel,
-												   HEAP_DEFAULT_FILLFACTOR);
+	if (IsToastRelation(state->rs_new_rel))
+		saveFreeSpace = ToastGetTargetPageFreeSpace();
+	else
+		saveFreeSpace = HeapGetTargetPageFreeSpace(state->rs_new_rel);
 
 	/* Now we can check to see if there's enough free space already. */
 	if (state->rs_buffer_valid)
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 3a5676f..0843413 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -130,7 +130,7 @@ create_ctas_internal(List *attrList, IntoClause *into)
 										validnsps,
 										true, false);
 
-	(void) heap_reloptions(RELKIND_TOASTVALUE, toast_options, true);
+	(void) toast_reloptions(toast_options, true);
 
 	NewRelationCreateToastTable(intoRelationAddr.objectId, toast_options);
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 7a13b97..26e06c6 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -751,8 +751,12 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 		case RELKIND_PARTITIONED_TABLE:
 			(void) partitioned_table_reloptions(reloptions, true);
 			break;
-		default:
-			(void) heap_reloptions(relkind, reloptions, true);
+		case RELKIND_TOASTVALUE:
+			(void) toast_reloptions(reloptions, true);
+			break;
+		case RELKIND_RELATION:
+		case RELKIND_MATVIEW:
+			(void) heap_reloptions(reloptions, true);
 	}
 
 	if (stmt->ofTypename)
@@ -12680,9 +12684,11 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 	switch (rel->rd_rel->relkind)
 	{
 		case RELKIND_RELATION:
-		case RELKIND_TOASTVALUE:
 		case RELKIND_MATVIEW:
-			(void) heap_reloptions(rel->rd_rel->relkind, newOptions, true);
+			(void) heap_reloptions( newOptions, true);
+			break;
+		case RELKIND_TOASTVALUE:
+			(void) toast_reloptions(newOptions, true);
 			break;
 		case RELKIND_PARTITIONED_TABLE:
 			(void) partitioned_table_reloptions(newOptions, true);
@@ -12794,7 +12800,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 										 defList, "toast", validnsps, false,
 										 operation == AT_ResetRelOptions);
 
-		(void) heap_reloptions(RELKIND_TOASTVALUE, newOptions, true);
+		(void) toast_reloptions(newOptions, true);
 
 		memset(repl_val, 0, sizeof(repl_val));
 		memset(repl_null, false, sizeof(repl_null));
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index d625d17..ff26549 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -31,6 +31,7 @@
 #include "access/tableam.h"
 #include "access/transam.h"
 #include "access/xact.h"
+#include "catalog/catalog.h"
 #include "catalog/namespace.h"
 #include "catalog/pg_database.h"
 #include "catalog/pg_inherits.h"
@@ -1824,7 +1825,10 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	if (params->index_cleanup == VACOPT_TERNARY_DEFAULT)
 	{
 		if (onerel->rd_options == NULL ||
-			((StdRdOptions *) onerel->rd_options)->vacuum_index_cleanup)
+			(!IsToastRelation(onerel) &&
+			 ((HeapOptions *) onerel->rd_options)->vacuum_index_cleanup) ||
+			(IsToastRelation(onerel) &&
+			 ((ToastOptions *) onerel->rd_options)->vacuum_index_cleanup))
 			params->index_cleanup = VACOPT_TERNARY_ENABLED;
 		else
 			params->index_cleanup = VACOPT_TERNARY_DISABLED;
@@ -1834,7 +1838,10 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	if (params->truncate == VACOPT_TERNARY_DEFAULT)
 	{
 		if (onerel->rd_options == NULL ||
-			((StdRdOptions *) onerel->rd_options)->vacuum_truncate)
+			(!IsToastRelation(onerel) &&
+			 ((HeapOptions *) onerel->rd_options)->vacuum_truncate) ||
+			(IsToastRelation(onerel) &&
+			 ((ToastOptions *) onerel->rd_options)->vacuum_truncate))
 			params->truncate = VACOPT_TERNARY_ENABLED;
 		else
 			params->truncate = VACOPT_TERNARY_DISABLED;
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index d82fc5a..425ccf7 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -150,8 +150,15 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
 		estimate_rel_size(relation, rel->attr_widths - rel->min_attr,
 						  &rel->pages, &rel->tuples, &rel->allvisfrac);
 
-	/* Retrieve the parallel_workers reloption, or -1 if not set. */
-	rel->rel_parallel_workers = RelationGetParallelWorkers(relation, -1);
+	/*
+	 * Retrieve the parallel_workers for heap and mat.view relations.
+	 * Use -1 if not set, or if we are dealing with other relation kinds
+	 */
+	if (relation->rd_rel->relkind == RELKIND_RELATION ||
+		relation->rd_rel->relkind == RELKIND_MATVIEW)
+		rel->rel_parallel_workers = RelationGetParallelWorkers(relation, -1);
+	else
+		rel->rel_parallel_workers = -1;
 
 	/*
 	 * Make list of indexes.  Ignore indexes on system catalogs if told to.
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index e3a43d3..94922ed 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2718,6 +2718,7 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
 {
 	bytea	   *relopts;
 	AutoVacOpts *av;
+	AutoVacOpts *src;
 
 	Assert(((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_RELATION ||
 		   ((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_MATVIEW ||
@@ -2727,8 +2728,13 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
 	if (relopts == NULL)
 		return NULL;
 
+	if (((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_TOASTVALUE)
+		src = &(((ToastOptions *) relopts)->autovacuum);
+	else
+		src = &(((HeapOptions *) relopts)->autovacuum);
+
 	av = palloc(sizeof(AutoVacOpts));
-	memcpy(av, &(((StdRdOptions *) relopts)->autovacuum), sizeof(AutoVacOpts));
+	memcpy(av, src, sizeof(AutoVacOpts));
 	pfree(relopts);
 
 	return av;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 1b460a2..c7d02d1 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1174,9 +1174,7 @@ ProcessUtilitySlow(ParseState *pstate,
 																validnsps,
 																true,
 																false);
-							(void) heap_reloptions(RELKIND_TOASTVALUE,
-												   toast_options,
-												   true);
+							(void) toast_reloptions(toast_options, true);
 
 							NewRelationCreateToastTable(address.objectId,
 														toast_options);
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index 36e6472..450322d 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -186,9 +186,8 @@ extern void *build_reloptions(Datum reloptions, bool validate,
 							  const relopt_parse_elt *relopt_elems,
 							  int num_relopt_elems);
 
-extern bytea *default_reloptions(Datum reloptions, bool validate,
-								 relopt_kind kind);
-extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
+extern bytea *toast_reloptions(Datum reloptions, bool validate);
+extern bytea *heap_reloptions(Datum reloptions, bool validate);
 extern bytea *view_reloptions(Datum reloptions, bool validate);
 extern bytea *partitioned_table_reloptions(Datum reloptions, bool validate);
 extern bytea *index_reloptions(amoptions_function amoptions, Datum reloptions,
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 44ed04d..820d172 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -240,14 +240,9 @@ typedef struct ForeignKeyCacheInfo
 
 
 /*
- * StdRdOptions
- *		Standard contents of rd_options for heaps.
- *
- * RelationGetFillFactor() and RelationGetTargetPageFreeSpace() can only
- * be applied to relations that use this format or a superset for
- * private options data.
+ * AutoVacOpts
+ *		Auto Vacuum options used both in Heap and Toast relations
  */
- /* autovacuum-related reloptions. */
 typedef struct AutoVacOpts
 {
 	bool		enabled;
@@ -266,7 +261,11 @@ typedef struct AutoVacOpts
 	float8		analyze_scale_factor;
 } AutoVacOpts;
 
-typedef struct StdRdOptions
+/*
+ * HeapOptions
+ *		Binary representation of relation options for Heap relations.
+ */
+typedef struct HeapOptions
 {
 	int32		vl_len_;		/* varlena header (do not touch directly!) */
 	int			fillfactor;		/* page fill factor in percent (0..100) */
@@ -277,40 +276,60 @@ typedef struct StdRdOptions
 	int			parallel_workers;	/* max number of parallel workers */
 	bool		vacuum_index_cleanup;	/* enables index vacuuming and cleanup */
 	bool		vacuum_truncate;	/* enables vacuum to truncate a relation */
-} StdRdOptions;
+} HeapOptions;
 
 #define HEAP_MIN_FILLFACTOR			10
 #define HEAP_DEFAULT_FILLFACTOR		100
 
 /*
+ * ToastOptions
+ *		Binary representation of relation options for Toast relations.
+ */
+typedef struct ToastOptions
+{
+	int32		vl_len_;		/* varlena header (do not touch directly!) */
+	AutoVacOpts autovacuum;		/* autovacuum-related options */
+	bool		vacuum_index_cleanup; /* enables index vacuuming and cleanup */
+	bool		vacuum_truncate;	/* enables vacuum to truncate a relation */
+} ToastOptions;
+
+#define TOAST_DEFAULT_FILLFACTOR	100 /* Only default is actually used */
+
+/*
  * RelationGetToastTupleTarget
- *		Returns the relation's toast_tuple_target.  Note multiple eval of argument!
+ *		Returns the heap's toast_tuple_target.  Note multiple eval of argument!
  */
-#define RelationGetToastTupleTarget(relation, defaulttarg) \
-	((relation)->rd_options ? \
-	 ((StdRdOptions *) (relation)->rd_options)->toast_tuple_target : (defaulttarg))
+#define RelationGetToastTupleTarget(relation, defaulttarg)				\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION ||		\
+				 relation->rd_rel->relkind == RELKIND_MATVIEW),			\
+	 (relation)->rd_options ? 											\
+	  ((HeapOptions *) (relation)->rd_options)->toast_tuple_target : \
+			(defaulttarg))
 
 /*
- * RelationGetFillFactor
- *		Returns the relation's fillfactor.  Note multiple eval of argument!
+ * HeapGetFillFactor
+ *		Returns the heap's fillfactor.  Note multiple eval of argument!
  */
-#define RelationGetFillFactor(relation, defaultff) \
-	((relation)->rd_options ? \
-	 ((StdRdOptions *) (relation)->rd_options)->fillfactor : (defaultff))
+#define HeapGetFillFactor(relation)									\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION ||		\
+				 relation->rd_rel->relkind == RELKIND_MATVIEW),			\
+	 (relation)->rd_options ? 											\
+	  ((HeapOptions *) (relation)->rd_options)->fillfactor : 		\
+									(HEAP_DEFAULT_FILLFACTOR))
 
 /*
- * RelationGetTargetPageUsage
+ * HeapGetTargetPageUsage
  *		Returns the relation's desired space usage per page in bytes.
  */
-#define RelationGetTargetPageUsage(relation, defaultff) \
-	(BLCKSZ * RelationGetFillFactor(relation, defaultff) / 100)
+#define HeapGetTargetPageUsage(relation) \
+	(BLCKSZ * HeapGetFillFactor(relation) / 100)
 
 /*
- * RelationGetTargetPageFreeSpace
+ * HeapGetTargetPageFreeSpace
  *		Returns the relation's desired freespace per page in bytes.
  */
-#define RelationGetTargetPageFreeSpace(relation, defaultff) \
-	(BLCKSZ * (100 - RelationGetFillFactor(relation, defaultff)) / 100)
+#define HeapGetTargetPageFreeSpace(relation) \
+	(BLCKSZ * (100 - HeapGetFillFactor(relation)) / 100)
 
 /*
  * RelationIsUsedAsCatalogTable
@@ -321,16 +340,27 @@ typedef struct StdRdOptions
 	((relation)->rd_options && \
 	 ((relation)->rd_rel->relkind == RELKIND_RELATION || \
 	  (relation)->rd_rel->relkind == RELKIND_MATVIEW) ? \
-	 ((StdRdOptions *) (relation)->rd_options)->user_catalog_table : false)
+	 ((HeapOptions *) (relation)->rd_options)->user_catalog_table : false)
 
 /*
  * RelationGetParallelWorkers
- *		Returns the relation's parallel_workers reloption setting.
+ *		Returns the heap's parallel_workers reloption setting.
  *		Note multiple eval of argument!
  */
-#define RelationGetParallelWorkers(relation, defaultpw) \
-	((relation)->rd_options ? \
-	 ((StdRdOptions *) (relation)->rd_options)->parallel_workers : (defaultpw))
+#define RelationGetParallelWorkers(relation, defaultpw) 				\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION ||		\
+				 relation->rd_rel->relkind == RELKIND_MATVIEW),			\
+	 (relation)->rd_options ? 											\
+	  ((HeapOptions *) (relation)->rd_options)->parallel_workers : 	\
+			(defaultpw))
+
+/*
+ * ToastGetTargetPageFreeSpace
+ *		Returns the TOAST relation's desired freespace per page in bytes.
+ *		Always calculated using default fillfactor value.
+ */
+#define ToastGetTargetPageFreeSpace() \
+	(BLCKSZ * (100 - TOAST_DEFAULT_FILLFACTOR) / 100)
 
 /* ViewOptions->check_option values */
 typedef enum ViewOptCheckOption

Michael Paquier

michael@paquier.xyz

almost 6 years ago

In reply to: Nikolay Shaplov (#3)

Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions

On Wed, Mar 04, 2020 at 10:58:31PM +0300, Nikolay Shaplov wrote:

But the truth is that my goal is to move all code that defines all option
names, min/max values etc, move it inside am code. To move data from
boolRelOpts, intRelOpts, realRelOpts, enumRelOpts, enumRelOpts from
reloptions.c into the code that implement AMs that uses these options.

I did it for indexes in patch I've offered several years ago. Now we have also
relaion AM.

But I would prefer to fix index AM relioptions first, and then copy that
solution for relations.

How do you think that this part should be changed then, if this needs
any changes? It seems to me that we have a rather correct layer for
index AMs by requiring each one to define the available option set
using indoptions through their handler, with option fetching macros
located within each AM.

Because if I first copy AM solution from indexes to relation, then I will have
to fix it in two places.

So I would prefer to keep reloptions for relations in relations.c, only split
them into HeapOptions and ToastOptions, then change AM for indexes moving
option definition into AM's and then clone the solution for relations.

Then, for table AMs, it seems to me that you are right for long-term
perspective to have the toast-related options in reloptions.c, or
perhaps directly located within more toast-related file (?) as table
AMs interact with toast using heapam_relation_needs_toast_table and
such callbacks. So for heap, moving the option handling to roughly
heapam_handler.c is a natural move, though this requires a redesign of
the existing structure to use option handling closer to what
indoptions does, but for tables.
--
Michael

Nikolay Shaplov

dhyan@nataraj.su

almost 6 years ago

In reply to: Michael Paquier (#4)

Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions

В письме от суббота, 7 марта 2020 г. 10:03:40 MSK пользователь Michael Paquier
написал:

On Wed, Mar 04, 2020 at 10:58:31PM +0300, Nikolay Shaplov wrote:

But the truth is that my goal is to move all code that defines all option
names, min/max values etc, move it inside am code. To move data from
boolRelOpts, intRelOpts, realRelOpts, enumRelOpts, enumRelOpts from
reloptions.c into the code that implement AMs that uses these options.

I did it for indexes in patch I've offered several years ago. Now we have
also relaion AM.

But I would prefer to fix index AM relioptions first, and then copy that
solution for relations.

How do you think that this part should be changed then, if this needs
any changes? It seems to me that we have a rather correct layer for
index AMs by requiring each one to define the available option set
using indoptions through their handler, with option fetching macros
located within each AM.

My idea is like this:

Now information about what reloption AM has and how they should be parsed are
stored in two places. One is relioptions.c and boolRelOpts, intRelOpts,
realRelOpts, enumRelOpts, enumRelOpts arrays, another is
static const relopt_parse_elt tab[] inside amoptions_function amoptions;
function of each AM.

My suggestion is to merge all this data into one structure. Like
option_definition

/* generic struct to hold shared data */
typedef struct option_definition_basic
{
const char *name; /* must be first (used as list termination
* marker) */
const char *desc;
LOCKMODE lockmode;
option_definition_flags flags;
option_type type;
int struct_offset; /* offset of the value in Bytea representation
*/
} option_definition_basic;

typedef struct option_definition_bool
{
option_definition_basic base;
bool default_val;
} option_definition_bool;

typedef struct option_definition_int
{
option_definition_basic base;
int default_val;
int min;
int max;
} option_definition_int;

typedef struct option_definition_real
{
option_definition_basic base;
double default_val;
double min;
double max;
} option_definition_real;

typedef struct option_definition_enum
{
option_definition_basic base;
const char **allowed_values;/* Null terminated array of allowed values for
* the option */
int default_val; /* Number of item of allowed_values array */
} option_definition_enum;

This example from my old code, I guess I should add a union here, this will
make code more readable... But the idea is the same, we have one structure
that describes how this option should be parsed.

Then we gather all options definitions for one object (for example for index)
into a structure called OptionsDefSet

typedef struct OptionDefSet
{
option_definition_basic **definitions;
int num; /* Number of catalog items in use */
int num_allocated; /* Number of catalog items allocated */
bool forbid_realloc; /* If number of items of the catalog were
* strictly set to certain value do no allow
* adding more idems */
Size struct_size; /* Size of a structure for options in binary
* representation */
postprocess_bytea_options_function postprocess_fun; /* This function is
* called after options
* were converted in
* Bytea represenation.
* Can be used for extra
* validation and so on */
char *namespace; /* Catalog is used for options from this
* namespase */
} OptionDefSet;

This DefSet will have all we need to parse options for certain object.

This all will be stored in the AM code.

Then we replace amoptions_function amoptions; function of the AM with
amoptions_def_set_function amoptions_def_set; function. This function will
return OptionDefSet to whoever called it.

So whenever we need an option in index_reloptions from reloption.c we get this
DefSet, and then call a function that parses options using this data.

Why do we need this.

1. To have all options related data in one place. Now when a developer wants
to add an option, he needs to find all places in the code this option should
be added. It is not good thing. It brings troubles and errors.

2. Now we have two different options storages for in-core AM options and for
contib AM options. They are boolRelOpts, intRelOpts, realRelOpts, enumRelOpts,
enumRelOpts arrays for in-core AMm's and static relopt_gen **custom_options
for contrib AM. It if better ho have same tool for both. My idea will allow to
have same code both in contrib and in-core AM.

3. Then we will be able to have reloptions-like options just anywhere, just
define an OptionDefSet, and put code that provides option values from SQL and
pg_catalog. I came to the idea of rewriting option code when I have been
working on the task of adding opclass parameters, Nikita Glukhov works on it
now. It is very simulator to options, but now you can not use reloptions code
for that. When we will have OptionDefSet I've described, it can be used for
opclass parameters too.

The example of how it can be done can be found at https://
commitfest.postgresql.org/15/992/
The code is quite outdated, and has a lot of extra code that already commited
or is not really needed. But you can see the idea.

I will try to provide new version of it soon, with no extra code, that can be
applied to current master, but it would be good to apply it to the master
branch where there is no StdRdOptions. This will make patch more readable and
understandable.

So, can I please have it in the code :-)

Because if I first copy AM solution from indexes to relation, then I will
have to fix it in two places.

So I would prefer to keep reloptions for relations in relations.c, only
split them into HeapOptions and ToastOptions, then change AM for indexes
moving option definition into AM's and then clone the solution for
relations.

Then, for table AMs, it seems to me that you are right for long-term
perspective to have the toast-related options in reloptions.c, or
perhaps directly located within more toast-related file (?) as table
AMs interact with toast using heapam_relation_needs_toast_table and
such callbacks. So for heap, moving the option handling to roughly
heapam_handler.c is a natural move, though this requires a redesign of
the existing structure to use option handling closer to what
indoptions does, but for tables.

We also have view reloptions and attribute options, as well as toast options.
We can keep them in reloption.c or find better place for them. It is not a big
problem I think. And as for heap options, yes, as heap now has AM, they should
be moved inside AM. I can do it when we are finished with index options.

Nikolay Shaplov

dhyan@nataraj.su

almost 6 years ago

In reply to: Nikolay Shaplov (#3)

1 attachment(s)

Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions

A new version of the patch.
Autovacuum options were extended in b07642db

So I added that options to the current patch.

Attachments:

StdRdOptions_to_HeapOptions_and_ToastOptions_v1b.difftext/x-patch; charset=UTF-8; name=StdRdOptions_to_HeapOptions_and_ToastOptions_v1b.diffDownload

diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index e136116..55bd1ec 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -46,9 +46,8 @@
  * upper and lower bounds (if applicable); for strings, consider a validation
  * routine.
  * (ii) add a record below (or use add_<type>_reloption).
- * (iii) add it to the appropriate options struct (perhaps StdRdOptions)
- * (iv) add it to the appropriate handling routine (perhaps
- * default_reloptions)
+ * (iii) add it to the appropriate options struct
+ * (iv) add it to the appropriate handling routine
  * (v) make sure the lock level is set correctly for that operation
  * (vi) don't forget to document the option
  *
@@ -1147,9 +1146,11 @@ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
 	switch (classForm->relkind)
 	{
 		case RELKIND_RELATION:
-		case RELKIND_TOASTVALUE:
 		case RELKIND_MATVIEW:
-			options = heap_reloptions(classForm->relkind, datum, false);
+			options = heap_reloptions(datum, false);
+			break;
+		case RELKIND_TOASTVALUE:
+			options = toast_reloptions(datum, false);
 			break;
 		case RELKIND_PARTITIONED_TABLE:
 			options = partitioned_table_reloptions(datum, false);
@@ -1521,59 +1522,66 @@ fillRelOptions(void *rdopts, Size basesize,
 
 
 /*
- * Option parser for anything that uses StdRdOptions.
+ * Option parsing definition for autovacuum. Used in toast and heap options.
+ */
+
+#define AUTOVACUUM_RELOPTIONS(OFFSET)                                \
+		{"autovacuum_enabled", RELOPT_TYPE_BOOL,                     \
+		OFFSET + offsetof(AutoVacOpts, enabled)},                    \
+		{"autovacuum_vacuum_threshold", RELOPT_TYPE_INT,             \
+		OFFSET + offsetof(AutoVacOpts, vacuum_threshold)},           \
+		{"autovacuum_vacuum_insert_threshold", RELOPT_TYPE_INT,      \
+		OFFSET + offsetof(AutoVacOpts, vacuum_ins_threshold)},       \
+		{"autovacuum_analyze_threshold", RELOPT_TYPE_INT,            \
+		OFFSET + offsetof(AutoVacOpts, analyze_threshold)},          \
+		{"autovacuum_vacuum_cost_delay", RELOPT_TYPE_REAL,           \
+		OFFSET + offsetof(AutoVacOpts, vacuum_cost_delay)},          \
+		{"autovacuum_vacuum_cost_limit", RELOPT_TYPE_INT,            \
+		OFFSET + offsetof(AutoVacOpts, vacuum_cost_limit)},          \
+		{"autovacuum_freeze_min_age", RELOPT_TYPE_INT,               \
+		OFFSET + offsetof(AutoVacOpts, freeze_min_age)},             \
+		{"autovacuum_freeze_max_age", RELOPT_TYPE_INT,               \
+		OFFSET + offsetof(AutoVacOpts, freeze_max_age)},             \
+		{"autovacuum_freeze_table_age", RELOPT_TYPE_INT,             \
+		OFFSET + offsetof(AutoVacOpts, freeze_table_age)},           \
+		{"autovacuum_multixact_freeze_min_age", RELOPT_TYPE_INT,     \
+		OFFSET + offsetof(AutoVacOpts, multixact_freeze_min_age)},   \
+		{"autovacuum_multixact_freeze_max_age", RELOPT_TYPE_INT,     \
+		OFFSET + offsetof(AutoVacOpts, multixact_freeze_max_age)},   \
+		{"autovacuum_multixact_freeze_table_age", RELOPT_TYPE_INT,   \
+		OFFSET + offsetof(AutoVacOpts, multixact_freeze_table_age)}, \
+		{"log_autovacuum_min_duration", RELOPT_TYPE_INT,             \
+		OFFSET + offsetof(AutoVacOpts, log_min_duration)},           \
+		{"autovacuum_vacuum_scale_factor", RELOPT_TYPE_REAL,         \
+		OFFSET + offsetof(AutoVacOpts, vacuum_scale_factor)},        \
+		{"autovacuum_vacuum_insert_scale_factor", RELOPT_TYPE_REAL,  \
+		OFFSET  + offsetof(AutoVacOpts, vacuum_ins_scale_factor)},   \
+		{"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL,        \
+		OFFSET + offsetof(AutoVacOpts, analyze_scale_factor)}
+/*
+ * Option parser for heap
  */
 bytea *
-default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
+heap_reloptions(Datum reloptions, bool validate)
 {
 	static const relopt_parse_elt tab[] = {
-		{"fillfactor", RELOPT_TYPE_INT, offsetof(StdRdOptions, fillfactor)},
-		{"autovacuum_enabled", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, enabled)},
-		{"autovacuum_vacuum_threshold", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_threshold)},
-		{"autovacuum_vacuum_insert_threshold", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_ins_threshold)},
-		{"autovacuum_analyze_threshold", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, analyze_threshold)},
-		{"autovacuum_vacuum_cost_limit", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_limit)},
-		{"autovacuum_freeze_min_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_min_age)},
-		{"autovacuum_freeze_max_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_max_age)},
-		{"autovacuum_freeze_table_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_table_age)},
-		{"autovacuum_multixact_freeze_min_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_min_age)},
-		{"autovacuum_multixact_freeze_max_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_max_age)},
-		{"autovacuum_multixact_freeze_table_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_table_age)},
-		{"log_autovacuum_min_duration", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, log_min_duration)},
+		{"fillfactor", RELOPT_TYPE_INT, offsetof(HeapOptions, fillfactor)},
+		AUTOVACUUM_RELOPTIONS(offsetof(HeapOptions, autovacuum)),
 		{"toast_tuple_target", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, toast_tuple_target)},
-		{"autovacuum_vacuum_cost_delay", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_delay)},
-		{"autovacuum_vacuum_scale_factor", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_scale_factor)},
-		{"autovacuum_vacuum_insert_scale_factor", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_ins_scale_factor)},
-		{"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, analyze_scale_factor)},
+		offsetof(HeapOptions, toast_tuple_target)},
 		{"user_catalog_table", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, user_catalog_table)},
+		offsetof(HeapOptions, user_catalog_table)},
 		{"parallel_workers", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, parallel_workers)},
+		offsetof(HeapOptions, parallel_workers)},
 		{"vacuum_index_cleanup", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, vacuum_index_cleanup)},
+		offsetof(HeapOptions, vacuum_index_cleanup)},
 		{"vacuum_truncate", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, vacuum_truncate)}
+		offsetof(HeapOptions, vacuum_truncate)}
 	};
 
-	return (bytea *) build_reloptions(reloptions, validate, kind,
-									  sizeof(StdRdOptions),
+	return (bytea *) build_reloptions(reloptions, validate,
+									  RELOPT_KIND_HEAP,
+									  sizeof(HeapOptions),
 									  tab, lengthof(tab));
 }
 
@@ -1641,6 +1649,32 @@ partitioned_table_reloptions(Datum reloptions, bool validate)
 }
 
 /*
+ * Option parser for toast
+ */
+bytea *
+toast_reloptions(Datum reloptions, bool validate)
+{
+	ToastOptions *rdopts;
+	static const relopt_parse_elt tab[] = {
+		AUTOVACUUM_RELOPTIONS(offsetof(ToastOptions, autovacuum)),
+		{"vacuum_index_cleanup", RELOPT_TYPE_BOOL,
+		offsetof(ToastOptions, vacuum_index_cleanup)},
+		{"vacuum_truncate", RELOPT_TYPE_BOOL,
+		offsetof(ToastOptions, vacuum_truncate)}
+	};
+
+	rdopts = build_reloptions(reloptions, validate,
+									  RELOPT_KIND_TOAST,
+									  sizeof(ToastOptions),
+									  tab, lengthof(tab));
+	/* adjust default-only parameters for TOAST relations */
+	rdopts->autovacuum.analyze_threshold = -1;
+	rdopts->autovacuum.analyze_scale_factor = -1;
+
+	return (bytea *) rdopts;
+}
+
+/*
  * Option parser for views
  */
 bytea *
@@ -1660,37 +1694,6 @@ view_reloptions(Datum reloptions, bool validate)
 }
 
 /*
- * Parse options for heaps, views and toast tables.
- */
-bytea *
-heap_reloptions(char relkind, Datum reloptions, bool validate)
-{
-	StdRdOptions *rdopts;
-
-	switch (relkind)
-	{
-		case RELKIND_TOASTVALUE:
-			rdopts = (StdRdOptions *)
-				default_reloptions(reloptions, validate, RELOPT_KIND_TOAST);
-			if (rdopts != NULL)
-			{
-				/* adjust default-only parameters for TOAST relations */
-				rdopts->fillfactor = 100;
-				rdopts->autovacuum.analyze_threshold = -1;
-				rdopts->autovacuum.analyze_scale_factor = -1;
-			}
-			return (bytea *) rdopts;
-		case RELKIND_RELATION:
-		case RELKIND_MATVIEW:
-			return default_reloptions(reloptions, validate, RELOPT_KIND_HEAP);
-		default:
-			/* other relkinds are not supported */
-			return NULL;
-	}
-}
-
-
-/*
  * Parse options for indexes.
  *
  *	amoptions	index AM's option parser function
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 29694b8..10865a3 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2123,8 +2123,10 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
 	AssertArg(!(options & HEAP_INSERT_NO_LOGICAL));
 
 	needwal = !(options & HEAP_INSERT_SKIP_WAL) && RelationNeedsWAL(relation);
-	saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
-												   HEAP_DEFAULT_FILLFACTOR);
+	if (IsToastRelation(relation))
+		saveFreeSpace = ToastGetTargetPageFreeSpace();
+	else
+		saveFreeSpace = HeapGetTargetPageFreeSpace(relation);
 
 	/* Toast and set header data in all the slots */
 	heaptuples = palloc(ntuples * sizeof(HeapTuple));
diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index aa3f14c..7b0cb75 100644
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -19,6 +19,7 @@
 #include "access/hio.h"
 #include "access/htup_details.h"
 #include "access/visibilitymap.h"
+#include "catalog/catalog.h"
 #include "storage/bufmgr.h"
 #include "storage/freespace.h"
 #include "storage/lmgr.h"
@@ -346,8 +347,10 @@ RelationGetBufferForTuple(Relation relation, Size len,
 						len, MaxHeapTupleSize)));
 
 	/* Compute desired extra freespace due to fillfactor option */
-	saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
-												   HEAP_DEFAULT_FILLFACTOR);
+	if (IsToastRelation(relation))
+		saveFreeSpace = ToastGetTargetPageFreeSpace();
+	else
+		saveFreeSpace = HeapGetTargetPageFreeSpace(relation);
 
 	if (otherBuffer != InvalidBuffer)
 		otherBlock = BufferGetBlockNumber(otherBuffer);
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 1794cfd..cf020e2 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -129,8 +129,11 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
 	 * important than sometimes getting a wrong answer in what is after all
 	 * just a heuristic estimate.
 	 */
-	minfree = RelationGetTargetPageFreeSpace(relation,
-											 HEAP_DEFAULT_FILLFACTOR);
+
+	if (IsToastRelation(relation))
+		minfree = ToastGetTargetPageFreeSpace();
+	else
+		minfree = HeapGetTargetPageFreeSpace(relation);
 	minfree = Max(minfree, BLCKSZ / 10);
 
 	if (PageIsFull(page) || PageGetHeapFreeSpace(page) < minfree)
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 9c29bc0..d872ce3 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -674,8 +674,10 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
 						len, MaxHeapTupleSize)));
 
 	/* Compute desired extra freespace due to fillfactor option */
-	saveFreeSpace = RelationGetTargetPageFreeSpace(state->rs_new_rel,
-												   HEAP_DEFAULT_FILLFACTOR);
+	if (IsToastRelation(state->rs_new_rel))
+		saveFreeSpace = ToastGetTargetPageFreeSpace();
+	else
+		saveFreeSpace = HeapGetTargetPageFreeSpace(state->rs_new_rel);
 
 	/* Now we can check to see if there's enough free space already. */
 	if (state->rs_buffer_valid)
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index 3a5676f..0843413 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -130,7 +130,7 @@ create_ctas_internal(List *attrList, IntoClause *into)
 										validnsps,
 										true, false);
 
-	(void) heap_reloptions(RELKIND_TOASTVALUE, toast_options, true);
+	(void) toast_reloptions(toast_options, true);
 
 	NewRelationCreateToastTable(intoRelationAddr.objectId, toast_options);
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 8e35c5b..08bdd75 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -752,8 +752,12 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 		case RELKIND_PARTITIONED_TABLE:
 			(void) partitioned_table_reloptions(reloptions, true);
 			break;
-		default:
-			(void) heap_reloptions(relkind, reloptions, true);
+		case RELKIND_TOASTVALUE:
+			(void) toast_reloptions(reloptions, true);
+			break;
+		case RELKIND_RELATION:
+		case RELKIND_MATVIEW:
+			(void) heap_reloptions(reloptions, true);
 	}
 
 	if (stmt->ofTypename)
@@ -12722,9 +12726,11 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 	switch (rel->rd_rel->relkind)
 	{
 		case RELKIND_RELATION:
-		case RELKIND_TOASTVALUE:
 		case RELKIND_MATVIEW:
-			(void) heap_reloptions(rel->rd_rel->relkind, newOptions, true);
+			(void) heap_reloptions( newOptions, true);
+			break;
+		case RELKIND_TOASTVALUE:
+			(void) toast_reloptions(newOptions, true);
 			break;
 		case RELKIND_PARTITIONED_TABLE:
 			(void) partitioned_table_reloptions(newOptions, true);
@@ -12836,7 +12842,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 										 defList, "toast", validnsps, false,
 										 operation == AT_ResetRelOptions);
 
-		(void) heap_reloptions(RELKIND_TOASTVALUE, newOptions, true);
+		(void) toast_reloptions(newOptions, true);
 
 		memset(repl_val, 0, sizeof(repl_val));
 		memset(repl_null, false, sizeof(repl_null));
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index 59731d6..76f7f4f 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -31,6 +31,7 @@
 #include "access/tableam.h"
 #include "access/transam.h"
 #include "access/xact.h"
+#include "catalog/catalog.h"
 #include "catalog/namespace.h"
 #include "catalog/pg_database.h"
 #include "catalog/pg_inherits.h"
@@ -1824,7 +1825,10 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	if (params->index_cleanup == VACOPT_TERNARY_DEFAULT)
 	{
 		if (onerel->rd_options == NULL ||
-			((StdRdOptions *) onerel->rd_options)->vacuum_index_cleanup)
+			(!IsToastRelation(onerel) &&
+			 ((HeapOptions *) onerel->rd_options)->vacuum_index_cleanup) ||
+			(IsToastRelation(onerel) &&
+			 ((ToastOptions *) onerel->rd_options)->vacuum_index_cleanup))
 			params->index_cleanup = VACOPT_TERNARY_ENABLED;
 		else
 			params->index_cleanup = VACOPT_TERNARY_DISABLED;
@@ -1834,7 +1838,10 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	if (params->truncate == VACOPT_TERNARY_DEFAULT)
 	{
 		if (onerel->rd_options == NULL ||
-			((StdRdOptions *) onerel->rd_options)->vacuum_truncate)
+			(!IsToastRelation(onerel) &&
+			 ((HeapOptions *) onerel->rd_options)->vacuum_truncate) ||
+			(IsToastRelation(onerel) &&
+			 ((ToastOptions *) onerel->rd_options)->vacuum_truncate))
 			params->truncate = VACOPT_TERNARY_ENABLED;
 		else
 			params->truncate = VACOPT_TERNARY_DISABLED;
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index d82fc5a..425ccf7 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -150,8 +150,15 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
 		estimate_rel_size(relation, rel->attr_widths - rel->min_attr,
 						  &rel->pages, &rel->tuples, &rel->allvisfrac);
 
-	/* Retrieve the parallel_workers reloption, or -1 if not set. */
-	rel->rel_parallel_workers = RelationGetParallelWorkers(relation, -1);
+	/*
+	 * Retrieve the parallel_workers for heap and mat.view relations.
+	 * Use -1 if not set, or if we are dealing with other relation kinds
+	 */
+	if (relation->rd_rel->relkind == RELKIND_RELATION ||
+		relation->rd_rel->relkind == RELKIND_MATVIEW)
+		rel->rel_parallel_workers = RelationGetParallelWorkers(relation, -1);
+	else
+		rel->rel_parallel_workers = -1;
 
 	/*
 	 * Make list of indexes.  Ignore indexes on system catalogs if told to.
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 7e97ffa..24ec296 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2720,6 +2720,7 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
 {
 	bytea	   *relopts;
 	AutoVacOpts *av;
+	AutoVacOpts *src;
 
 	Assert(((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_RELATION ||
 		   ((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_MATVIEW ||
@@ -2729,8 +2730,13 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
 	if (relopts == NULL)
 		return NULL;
 
+	if (((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_TOASTVALUE)
+		src = &(((ToastOptions *) relopts)->autovacuum);
+	else
+		src = &(((HeapOptions *) relopts)->autovacuum);
+
 	av = palloc(sizeof(AutoVacOpts));
-	memcpy(av, &(((StdRdOptions *) relopts)->autovacuum), sizeof(AutoVacOpts));
+	memcpy(av, src, sizeof(AutoVacOpts));
 	pfree(relopts);
 
 	return av;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index b1f7f6e..e0ba6d8 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1175,9 +1175,7 @@ ProcessUtilitySlow(ParseState *pstate,
 																validnsps,
 																true,
 																false);
-							(void) heap_reloptions(RELKIND_TOASTVALUE,
-												   toast_options,
-												   true);
+							(void) toast_reloptions(toast_options, true);
 
 							NewRelationCreateToastTable(address.objectId,
 														toast_options);
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index 36e6472..450322d 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -186,9 +186,8 @@ extern void *build_reloptions(Datum reloptions, bool validate,
 							  const relopt_parse_elt *relopt_elems,
 							  int num_relopt_elems);
 
-extern bytea *default_reloptions(Datum reloptions, bool validate,
-								 relopt_kind kind);
-extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
+extern bytea *toast_reloptions(Datum reloptions, bool validate);
+extern bytea *heap_reloptions(Datum reloptions, bool validate);
 extern bytea *view_reloptions(Datum reloptions, bool validate);
 extern bytea *partitioned_table_reloptions(Datum reloptions, bool validate);
 extern bytea *index_reloptions(amoptions_function amoptions, Datum reloptions,
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 2a13d8a..81ab4c9 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -245,14 +245,9 @@ typedef struct ForeignKeyCacheInfo
 
 
 /*
- * StdRdOptions
- *		Standard contents of rd_options for heaps.
- *
- * RelationGetFillFactor() and RelationGetTargetPageFreeSpace() can only
- * be applied to relations that use this format or a superset for
- * private options data.
+ * AutoVacOpts
+ *		Auto Vacuum options used both in Heap and Toast relations
  */
- /* autovacuum-related reloptions. */
 typedef struct AutoVacOpts
 {
 	bool		enabled;
@@ -273,7 +268,11 @@ typedef struct AutoVacOpts
 	float8		analyze_scale_factor;
 } AutoVacOpts;
 
-typedef struct StdRdOptions
+/*
+ * HeapOptions
+ *		Binary representation of relation options for Heap relations.
+ */
+typedef struct HeapOptions
 {
 	int32		vl_len_;		/* varlena header (do not touch directly!) */
 	int			fillfactor;		/* page fill factor in percent (0..100) */
@@ -284,40 +283,60 @@ typedef struct StdRdOptions
 	int			parallel_workers;	/* max number of parallel workers */
 	bool		vacuum_index_cleanup;	/* enables index vacuuming and cleanup */
 	bool		vacuum_truncate;	/* enables vacuum to truncate a relation */
-} StdRdOptions;
+} HeapOptions;
 
 #define HEAP_MIN_FILLFACTOR			10
 #define HEAP_DEFAULT_FILLFACTOR		100
 
 /*
+ * ToastOptions
+ *		Binary representation of relation options for Toast relations.
+ */
+typedef struct ToastOptions
+{
+	int32		vl_len_;		/* varlena header (do not touch directly!) */
+	AutoVacOpts autovacuum;		/* autovacuum-related options */
+	bool		vacuum_index_cleanup; /* enables index vacuuming and cleanup */
+	bool		vacuum_truncate;	/* enables vacuum to truncate a relation */
+} ToastOptions;
+
+#define TOAST_DEFAULT_FILLFACTOR	100 /* Only default is actually used */
+
+/*
  * RelationGetToastTupleTarget
- *		Returns the relation's toast_tuple_target.  Note multiple eval of argument!
+ *		Returns the heap's toast_tuple_target.  Note multiple eval of argument!
  */
-#define RelationGetToastTupleTarget(relation, defaulttarg) \
-	((relation)->rd_options ? \
-	 ((StdRdOptions *) (relation)->rd_options)->toast_tuple_target : (defaulttarg))
+#define RelationGetToastTupleTarget(relation, defaulttarg)				\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION ||		\
+				 relation->rd_rel->relkind == RELKIND_MATVIEW),			\
+	 (relation)->rd_options ? 											\
+	  ((HeapOptions *) (relation)->rd_options)->toast_tuple_target : \
+			(defaulttarg))
 
 /*
- * RelationGetFillFactor
- *		Returns the relation's fillfactor.  Note multiple eval of argument!
+ * HeapGetFillFactor
+ *		Returns the heap's fillfactor.  Note multiple eval of argument!
  */
-#define RelationGetFillFactor(relation, defaultff) \
-	((relation)->rd_options ? \
-	 ((StdRdOptions *) (relation)->rd_options)->fillfactor : (defaultff))
+#define HeapGetFillFactor(relation)									\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION ||		\
+				 relation->rd_rel->relkind == RELKIND_MATVIEW),			\
+	 (relation)->rd_options ? 											\
+	  ((HeapOptions *) (relation)->rd_options)->fillfactor : 		\
+									(HEAP_DEFAULT_FILLFACTOR))
 
 /*
- * RelationGetTargetPageUsage
+ * HeapGetTargetPageUsage
  *		Returns the relation's desired space usage per page in bytes.
  */
-#define RelationGetTargetPageUsage(relation, defaultff) \
-	(BLCKSZ * RelationGetFillFactor(relation, defaultff) / 100)
+#define HeapGetTargetPageUsage(relation) \
+	(BLCKSZ * HeapGetFillFactor(relation) / 100)
 
 /*
- * RelationGetTargetPageFreeSpace
+ * HeapGetTargetPageFreeSpace
  *		Returns the relation's desired freespace per page in bytes.
  */
-#define RelationGetTargetPageFreeSpace(relation, defaultff) \
-	(BLCKSZ * (100 - RelationGetFillFactor(relation, defaultff)) / 100)
+#define HeapGetTargetPageFreeSpace(relation) \
+	(BLCKSZ * (100 - HeapGetFillFactor(relation)) / 100)
 
 /*
  * RelationIsUsedAsCatalogTable
@@ -328,16 +347,27 @@ typedef struct StdRdOptions
 	((relation)->rd_options && \
 	 ((relation)->rd_rel->relkind == RELKIND_RELATION || \
 	  (relation)->rd_rel->relkind == RELKIND_MATVIEW) ? \
-	 ((StdRdOptions *) (relation)->rd_options)->user_catalog_table : false)
+	 ((HeapOptions *) (relation)->rd_options)->user_catalog_table : false)
 
 /*
  * RelationGetParallelWorkers
- *		Returns the relation's parallel_workers reloption setting.
+ *		Returns the heap's parallel_workers reloption setting.
  *		Note multiple eval of argument!
  */
-#define RelationGetParallelWorkers(relation, defaultpw) \
-	((relation)->rd_options ? \
-	 ((StdRdOptions *) (relation)->rd_options)->parallel_workers : (defaultpw))
+#define RelationGetParallelWorkers(relation, defaultpw) 				\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION ||		\
+				 relation->rd_rel->relkind == RELKIND_MATVIEW),			\
+	 (relation)->rd_options ? 											\
+	  ((HeapOptions *) (relation)->rd_options)->parallel_workers : 	\
+			(defaultpw))
+
+/*
+ * ToastGetTargetPageFreeSpace
+ *		Returns the TOAST relation's desired freespace per page in bytes.
+ *		Always calculated using default fillfactor value.
+ */
+#define ToastGetTargetPageFreeSpace() \
+	(BLCKSZ * (100 - TOAST_DEFAULT_FILLFACTOR) / 100)
 
 /* ViewOptions->check_option values */
 typedef enum ViewOptCheckOption

Daniel Gustafsson

daniel@yesql.se

over 5 years ago

In reply to: Nikolay Shaplov (#6)

Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions

On 28 Mar 2020, at 19:57, Nikolay Shaplov <dhyan@nataraj.su> wrote:

A new version of the patch.
Autovacuum options were extended in b07642db

So I added that options to the current patch.

The heapam.c hunk in this version fails to apply to HEAD, can you please submit
a rebased version? Marking the CF entry as Waiting on Author in the meantime.

cheers ./daniel

Nikolay Shaplov

dhyan@nataraj.su

over 5 years ago

In reply to: Daniel Gustafsson (#7)

1 attachment(s)

Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions

В письме от четверг, 2 июля 2020 г. 17:15:13 MSK пользователь Daniel
Gustafsson написал:

A new version of the patch.
Autovacuum options were extended in b07642db

So I added that options to the current patch.

The heapam.c hunk in this version fails to apply to HEAD, can you please
submit a rebased version?

Thanks for reminding about it.

Here goes a rebased version.

--
Software Developer: https://www.upwork.com/freelancers/~014a87e140ff02c0da
Body-oriented Therapist: https://vk.com/nataraj_rebalancing (Russian)

Attachments:

StdRdOptions_to_HeapOptions_and_ToastOptions_v1c.difftext/x-patch; charset=UTF-8; name=StdRdOptions_to_HeapOptions_and_ToastOptions_v1c.diffDownload

diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 8ccc228..6118b55 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -46,9 +46,8 @@
  * upper and lower bounds (if applicable); for strings, consider a validation
  * routine.
  * (ii) add a record below (or use add_<type>_reloption).
- * (iii) add it to the appropriate options struct (perhaps StdRdOptions)
- * (iv) add it to the appropriate handling routine (perhaps
- * default_reloptions)
+ * (iii) add it to the appropriate options struct
+ * (iv) add it to the appropriate handling routine
  * (v) make sure the lock level is set correctly for that operation
  * (vi) don't forget to document the option
  *
@@ -1375,9 +1374,11 @@ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
 	switch (classForm->relkind)
 	{
 		case RELKIND_RELATION:
-		case RELKIND_TOASTVALUE:
 		case RELKIND_MATVIEW:
-			options = heap_reloptions(classForm->relkind, datum, false);
+			options = heap_reloptions(datum, false);
+			break;
+		case RELKIND_TOASTVALUE:
+			options = toast_reloptions(datum, false);
 			break;
 		case RELKIND_PARTITIONED_TABLE:
 			options = partitioned_table_reloptions(datum, false);
@@ -1811,59 +1812,66 @@ fillRelOptions(void *rdopts, Size basesize,
 
 
 /*
- * Option parser for anything that uses StdRdOptions.
+ * Option parsing definition for autovacuum. Used in toast and heap options.
+ */
+
+#define AUTOVACUUM_RELOPTIONS(OFFSET)                                \
+		{"autovacuum_enabled", RELOPT_TYPE_BOOL,                     \
+		OFFSET + offsetof(AutoVacOpts, enabled)},                    \
+		{"autovacuum_vacuum_threshold", RELOPT_TYPE_INT,             \
+		OFFSET + offsetof(AutoVacOpts, vacuum_threshold)},           \
+		{"autovacuum_vacuum_insert_threshold", RELOPT_TYPE_INT,      \
+		OFFSET + offsetof(AutoVacOpts, vacuum_ins_threshold)},       \
+		{"autovacuum_analyze_threshold", RELOPT_TYPE_INT,            \
+		OFFSET + offsetof(AutoVacOpts, analyze_threshold)},          \
+		{"autovacuum_vacuum_cost_delay", RELOPT_TYPE_REAL,           \
+		OFFSET + offsetof(AutoVacOpts, vacuum_cost_delay)},          \
+		{"autovacuum_vacuum_cost_limit", RELOPT_TYPE_INT,            \
+		OFFSET + offsetof(AutoVacOpts, vacuum_cost_limit)},          \
+		{"autovacuum_freeze_min_age", RELOPT_TYPE_INT,               \
+		OFFSET + offsetof(AutoVacOpts, freeze_min_age)},             \
+		{"autovacuum_freeze_max_age", RELOPT_TYPE_INT,               \
+		OFFSET + offsetof(AutoVacOpts, freeze_max_age)},             \
+		{"autovacuum_freeze_table_age", RELOPT_TYPE_INT,             \
+		OFFSET + offsetof(AutoVacOpts, freeze_table_age)},           \
+		{"autovacuum_multixact_freeze_min_age", RELOPT_TYPE_INT,     \
+		OFFSET + offsetof(AutoVacOpts, multixact_freeze_min_age)},   \
+		{"autovacuum_multixact_freeze_max_age", RELOPT_TYPE_INT,     \
+		OFFSET + offsetof(AutoVacOpts, multixact_freeze_max_age)},   \
+		{"autovacuum_multixact_freeze_table_age", RELOPT_TYPE_INT,   \
+		OFFSET + offsetof(AutoVacOpts, multixact_freeze_table_age)}, \
+		{"log_autovacuum_min_duration", RELOPT_TYPE_INT,             \
+		OFFSET + offsetof(AutoVacOpts, log_min_duration)},           \
+		{"autovacuum_vacuum_scale_factor", RELOPT_TYPE_REAL,         \
+		OFFSET + offsetof(AutoVacOpts, vacuum_scale_factor)},        \
+		{"autovacuum_vacuum_insert_scale_factor", RELOPT_TYPE_REAL,  \
+		OFFSET  + offsetof(AutoVacOpts, vacuum_ins_scale_factor)},   \
+		{"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL,        \
+		OFFSET + offsetof(AutoVacOpts, analyze_scale_factor)}
+/*
+ * Option parser for heap
  */
 bytea *
-default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
+heap_reloptions(Datum reloptions, bool validate)
 {
 	static const relopt_parse_elt tab[] = {
-		{"fillfactor", RELOPT_TYPE_INT, offsetof(StdRdOptions, fillfactor)},
-		{"autovacuum_enabled", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, enabled)},
-		{"autovacuum_vacuum_threshold", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_threshold)},
-		{"autovacuum_vacuum_insert_threshold", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_ins_threshold)},
-		{"autovacuum_analyze_threshold", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, analyze_threshold)},
-		{"autovacuum_vacuum_cost_limit", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_limit)},
-		{"autovacuum_freeze_min_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_min_age)},
-		{"autovacuum_freeze_max_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_max_age)},
-		{"autovacuum_freeze_table_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_table_age)},
-		{"autovacuum_multixact_freeze_min_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_min_age)},
-		{"autovacuum_multixact_freeze_max_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_max_age)},
-		{"autovacuum_multixact_freeze_table_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_table_age)},
-		{"log_autovacuum_min_duration", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, log_min_duration)},
+		{"fillfactor", RELOPT_TYPE_INT, offsetof(HeapOptions, fillfactor)},
+		AUTOVACUUM_RELOPTIONS(offsetof(HeapOptions, autovacuum)),
 		{"toast_tuple_target", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, toast_tuple_target)},
-		{"autovacuum_vacuum_cost_delay", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_delay)},
-		{"autovacuum_vacuum_scale_factor", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_scale_factor)},
-		{"autovacuum_vacuum_insert_scale_factor", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_ins_scale_factor)},
-		{"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, analyze_scale_factor)},
+		offsetof(HeapOptions, toast_tuple_target)},
 		{"user_catalog_table", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, user_catalog_table)},
+		offsetof(HeapOptions, user_catalog_table)},
 		{"parallel_workers", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, parallel_workers)},
+		offsetof(HeapOptions, parallel_workers)},
 		{"vacuum_index_cleanup", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, vacuum_index_cleanup)},
+		offsetof(HeapOptions, vacuum_index_cleanup)},
 		{"vacuum_truncate", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, vacuum_truncate)}
+		offsetof(HeapOptions, vacuum_truncate)}
 	};
 
-	return (bytea *) build_reloptions(reloptions, validate, kind,
-									  sizeof(StdRdOptions),
+	return (bytea *) build_reloptions(reloptions, validate,
+									  RELOPT_KIND_HEAP,
+									  sizeof(HeapOptions),
 									  tab, lengthof(tab));
 }
 
@@ -1971,6 +1979,32 @@ partitioned_table_reloptions(Datum reloptions, bool validate)
 }
 
 /*
+ * Option parser for toast
+ */
+bytea *
+toast_reloptions(Datum reloptions, bool validate)
+{
+	ToastOptions *rdopts;
+	static const relopt_parse_elt tab[] = {
+		AUTOVACUUM_RELOPTIONS(offsetof(ToastOptions, autovacuum)),
+		{"vacuum_index_cleanup", RELOPT_TYPE_BOOL,
+		offsetof(ToastOptions, vacuum_index_cleanup)},
+		{"vacuum_truncate", RELOPT_TYPE_BOOL,
+		offsetof(ToastOptions, vacuum_truncate)}
+	};
+
+	rdopts = build_reloptions(reloptions, validate,
+									  RELOPT_KIND_TOAST,
+									  sizeof(ToastOptions),
+									  tab, lengthof(tab));
+	/* adjust default-only parameters for TOAST relations */
+	rdopts->autovacuum.analyze_threshold = -1;
+	rdopts->autovacuum.analyze_scale_factor = -1;
+
+	return (bytea *) rdopts;
+}
+
+/*
  * Option parser for views
  */
 bytea *
@@ -1990,37 +2024,6 @@ view_reloptions(Datum reloptions, bool validate)
 }
 
 /*
- * Parse options for heaps, views and toast tables.
- */
-bytea *
-heap_reloptions(char relkind, Datum reloptions, bool validate)
-{
-	StdRdOptions *rdopts;
-
-	switch (relkind)
-	{
-		case RELKIND_TOASTVALUE:
-			rdopts = (StdRdOptions *)
-				default_reloptions(reloptions, validate, RELOPT_KIND_TOAST);
-			if (rdopts != NULL)
-			{
-				/* adjust default-only parameters for TOAST relations */
-				rdopts->fillfactor = 100;
-				rdopts->autovacuum.analyze_threshold = -1;
-				rdopts->autovacuum.analyze_scale_factor = -1;
-			}
-			return (bytea *) rdopts;
-		case RELKIND_RELATION:
-		case RELKIND_MATVIEW:
-			return default_reloptions(reloptions, validate, RELOPT_KIND_HEAP);
-		default:
-			/* other relkinds are not supported */
-			return NULL;
-	}
-}
-
-
-/*
  * Parse options for indexes.
  *
  *	amoptions	index AM's option parser function
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 537913d..2f759bc 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2083,8 +2083,10 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
 	AssertArg(!(options & HEAP_INSERT_NO_LOGICAL));
 
 	needwal = RelationNeedsWAL(relation);
-	saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
-												   HEAP_DEFAULT_FILLFACTOR);
+	if (IsToastRelation(relation))
+		saveFreeSpace = ToastGetTargetPageFreeSpace();
+	else
+		saveFreeSpace = HeapGetTargetPageFreeSpace(relation);
 
 	/* Toast and set header data in all the slots */
 	heaptuples = palloc(ntuples * sizeof(HeapTuple));
diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index aa3f14c..7b0cb75 100644
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -19,6 +19,7 @@
 #include "access/hio.h"
 #include "access/htup_details.h"
 #include "access/visibilitymap.h"
+#include "catalog/catalog.h"
 #include "storage/bufmgr.h"
 #include "storage/freespace.h"
 #include "storage/lmgr.h"
@@ -346,8 +347,10 @@ RelationGetBufferForTuple(Relation relation, Size len,
 						len, MaxHeapTupleSize)));
 
 	/* Compute desired extra freespace due to fillfactor option */
-	saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
-												   HEAP_DEFAULT_FILLFACTOR);
+	if (IsToastRelation(relation))
+		saveFreeSpace = ToastGetTargetPageFreeSpace();
+	else
+		saveFreeSpace = HeapGetTargetPageFreeSpace(relation);
 
 	if (otherBuffer != InvalidBuffer)
 		otherBlock = BufferGetBlockNumber(otherBuffer);
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index 1794cfd..cf020e2 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -129,8 +129,11 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
 	 * important than sometimes getting a wrong answer in what is after all
 	 * just a heuristic estimate.
 	 */
-	minfree = RelationGetTargetPageFreeSpace(relation,
-											 HEAP_DEFAULT_FILLFACTOR);
+
+	if (IsToastRelation(relation))
+		minfree = ToastGetTargetPageFreeSpace();
+	else
+		minfree = HeapGetTargetPageFreeSpace(relation);
 	minfree = Max(minfree, BLCKSZ / 10);
 
 	if (PageIsFull(page) || PageGetHeapFreeSpace(page) < minfree)
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 39e3376..86dd43f 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -663,8 +663,10 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
 						len, MaxHeapTupleSize)));
 
 	/* Compute desired extra freespace due to fillfactor option */
-	saveFreeSpace = RelationGetTargetPageFreeSpace(state->rs_new_rel,
-												   HEAP_DEFAULT_FILLFACTOR);
+	if (IsToastRelation(state->rs_new_rel))
+		saveFreeSpace = ToastGetTargetPageFreeSpace();
+	else
+		saveFreeSpace = HeapGetTargetPageFreeSpace(state->rs_new_rel);
 
 	/* Now we can check to see if there's enough free space already. */
 	if (state->rs_buffer_valid)
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index d53ec95..c3942ea 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -130,7 +130,7 @@ create_ctas_internal(List *attrList, IntoClause *into)
 										validnsps,
 										true, false);
 
-	(void) heap_reloptions(RELKIND_TOASTVALUE, toast_options, true);
+	(void) toast_reloptions(toast_options, true);
 
 	NewRelationCreateToastTable(intoRelationAddr.objectId, toast_options);
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index f79044f..ed6b520 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -754,8 +754,12 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 		case RELKIND_PARTITIONED_TABLE:
 			(void) partitioned_table_reloptions(reloptions, true);
 			break;
-		default:
-			(void) heap_reloptions(relkind, reloptions, true);
+		case RELKIND_TOASTVALUE:
+			(void) toast_reloptions(reloptions, true);
+			break;
+		case RELKIND_RELATION:
+		case RELKIND_MATVIEW:
+			(void) heap_reloptions(reloptions, true);
 	}
 
 	if (stmt->ofTypename)
@@ -12870,9 +12874,11 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 	switch (rel->rd_rel->relkind)
 	{
 		case RELKIND_RELATION:
-		case RELKIND_TOASTVALUE:
 		case RELKIND_MATVIEW:
-			(void) heap_reloptions(rel->rd_rel->relkind, newOptions, true);
+			(void) heap_reloptions( newOptions, true);
+			break;
+		case RELKIND_TOASTVALUE:
+			(void) toast_reloptions(newOptions, true);
 			break;
 		case RELKIND_PARTITIONED_TABLE:
 			(void) partitioned_table_reloptions(newOptions, true);
@@ -12984,7 +12990,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 										 defList, "toast", validnsps, false,
 										 operation == AT_ResetRelOptions);
 
-		(void) heap_reloptions(RELKIND_TOASTVALUE, newOptions, true);
+		(void) toast_reloptions(newOptions, true);
 
 		memset(repl_val, 0, sizeof(repl_val));
 		memset(repl_null, false, sizeof(repl_null));
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index d32de23..3ccc07c 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -31,6 +31,7 @@
 #include "access/tableam.h"
 #include "access/transam.h"
 #include "access/xact.h"
+#include "catalog/catalog.h"
 #include "catalog/namespace.h"
 #include "catalog/pg_database.h"
 #include "catalog/pg_inherits.h"
@@ -1822,7 +1823,10 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	if (params->index_cleanup == VACOPT_TERNARY_DEFAULT)
 	{
 		if (onerel->rd_options == NULL ||
-			((StdRdOptions *) onerel->rd_options)->vacuum_index_cleanup)
+			(!IsToastRelation(onerel) &&
+			 ((HeapOptions *) onerel->rd_options)->vacuum_index_cleanup) ||
+			(IsToastRelation(onerel) &&
+			 ((ToastOptions *) onerel->rd_options)->vacuum_index_cleanup))
 			params->index_cleanup = VACOPT_TERNARY_ENABLED;
 		else
 			params->index_cleanup = VACOPT_TERNARY_DISABLED;
@@ -1832,7 +1836,10 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	if (params->truncate == VACOPT_TERNARY_DEFAULT)
 	{
 		if (onerel->rd_options == NULL ||
-			((StdRdOptions *) onerel->rd_options)->vacuum_truncate)
+			(!IsToastRelation(onerel) &&
+			 ((HeapOptions *) onerel->rd_options)->vacuum_truncate) ||
+			(IsToastRelation(onerel) &&
+			 ((ToastOptions *) onerel->rd_options)->vacuum_truncate))
 			params->truncate = VACOPT_TERNARY_ENABLED;
 		else
 			params->truncate = VACOPT_TERNARY_DISABLED;
diff --git a/src/backend/optimizer/util/plancat.c b/src/backend/optimizer/util/plancat.c
index 2554502..c17d3ed 100644
--- a/src/backend/optimizer/util/plancat.c
+++ b/src/backend/optimizer/util/plancat.c
@@ -150,8 +150,15 @@ get_relation_info(PlannerInfo *root, Oid relationObjectId, bool inhparent,
 		estimate_rel_size(relation, rel->attr_widths - rel->min_attr,
 						  &rel->pages, &rel->tuples, &rel->allvisfrac);
 
-	/* Retrieve the parallel_workers reloption, or -1 if not set. */
-	rel->rel_parallel_workers = RelationGetParallelWorkers(relation, -1);
+	/*
+	 * Retrieve the parallel_workers for heap and mat.view relations.
+	 * Use -1 if not set, or if we are dealing with other relation kinds
+	 */
+	if (relation->rd_rel->relkind == RELKIND_RELATION ||
+		relation->rd_rel->relkind == RELKIND_MATVIEW)
+		rel->rel_parallel_workers = RelationGetParallelWorkers(relation, -1);
+	else
+		rel->rel_parallel_workers = -1;
 
 	/*
 	 * Make list of indexes.  Ignore indexes on system catalogs if told to.
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 9c7d4b0..5882e94 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2719,6 +2719,7 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
 {
 	bytea	   *relopts;
 	AutoVacOpts *av;
+	AutoVacOpts *src;
 
 	Assert(((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_RELATION ||
 		   ((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_MATVIEW ||
@@ -2728,8 +2729,13 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
 	if (relopts == NULL)
 		return NULL;
 
+	if (((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_TOASTVALUE)
+		src = &(((ToastOptions *) relopts)->autovacuum);
+	else
+		src = &(((HeapOptions *) relopts)->autovacuum);
+
 	av = palloc(sizeof(AutoVacOpts));
-	memcpy(av, &(((StdRdOptions *) relopts)->autovacuum), sizeof(AutoVacOpts));
+	memcpy(av, src, sizeof(AutoVacOpts));
 	pfree(relopts);
 
 	return av;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 97cbaa3..6a80445 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1177,9 +1177,7 @@ ProcessUtilitySlow(ParseState *pstate,
 																validnsps,
 																true,
 																false);
-							(void) heap_reloptions(RELKIND_TOASTVALUE,
-												   toast_options,
-												   true);
+							(void) toast_reloptions(toast_options, true);
 
 							NewRelationCreateToastTable(address.objectId,
 														toast_options);
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index 5964438..45cce3c 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -233,9 +233,8 @@ extern void *build_reloptions(Datum reloptions, bool validate,
 extern void *build_local_reloptions(local_relopts *relopts, Datum options,
 									bool validate);
 
-extern bytea *default_reloptions(Datum reloptions, bool validate,
-								 relopt_kind kind);
-extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
+extern bytea *toast_reloptions(Datum reloptions, bool validate);
+extern bytea *heap_reloptions(Datum reloptions, bool validate);
 extern bytea *view_reloptions(Datum reloptions, bool validate);
 extern bytea *partitioned_table_reloptions(Datum reloptions, bool validate);
 extern bytea *index_reloptions(amoptions_function amoptions, Datum reloptions,
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 0b5957b..a29f3f2 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -266,14 +266,9 @@ typedef struct ForeignKeyCacheInfo
 
 
 /*
- * StdRdOptions
- *		Standard contents of rd_options for heaps.
- *
- * RelationGetFillFactor() and RelationGetTargetPageFreeSpace() can only
- * be applied to relations that use this format or a superset for
- * private options data.
+ * AutoVacOpts
+ *		Auto Vacuum options used both in Heap and Toast relations
  */
- /* autovacuum-related reloptions. */
 typedef struct AutoVacOpts
 {
 	bool		enabled;
@@ -294,7 +289,11 @@ typedef struct AutoVacOpts
 	float8		analyze_scale_factor;
 } AutoVacOpts;
 
-typedef struct StdRdOptions
+/*
+ * HeapOptions
+ *		Binary representation of relation options for Heap relations.
+ */
+typedef struct HeapOptions
 {
 	int32		vl_len_;		/* varlena header (do not touch directly!) */
 	int			fillfactor;		/* page fill factor in percent (0..100) */
@@ -305,40 +304,60 @@ typedef struct StdRdOptions
 	int			parallel_workers;	/* max number of parallel workers */
 	bool		vacuum_index_cleanup;	/* enables index vacuuming and cleanup */
 	bool		vacuum_truncate;	/* enables vacuum to truncate a relation */
-} StdRdOptions;
+} HeapOptions;
 
 #define HEAP_MIN_FILLFACTOR			10
 #define HEAP_DEFAULT_FILLFACTOR		100
 
 /*
+ * ToastOptions
+ *		Binary representation of relation options for Toast relations.
+ */
+typedef struct ToastOptions
+{
+	int32		vl_len_;		/* varlena header (do not touch directly!) */
+	AutoVacOpts autovacuum;		/* autovacuum-related options */
+	bool		vacuum_index_cleanup; /* enables index vacuuming and cleanup */
+	bool		vacuum_truncate;	/* enables vacuum to truncate a relation */
+} ToastOptions;
+
+#define TOAST_DEFAULT_FILLFACTOR	100 /* Only default is actually used */
+
+/*
  * RelationGetToastTupleTarget
- *		Returns the relation's toast_tuple_target.  Note multiple eval of argument!
+ *		Returns the heap's toast_tuple_target.  Note multiple eval of argument!
  */
-#define RelationGetToastTupleTarget(relation, defaulttarg) \
-	((relation)->rd_options ? \
-	 ((StdRdOptions *) (relation)->rd_options)->toast_tuple_target : (defaulttarg))
+#define RelationGetToastTupleTarget(relation, defaulttarg)				\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION ||		\
+				 relation->rd_rel->relkind == RELKIND_MATVIEW),			\
+	 (relation)->rd_options ? 											\
+	  ((HeapOptions *) (relation)->rd_options)->toast_tuple_target : \
+			(defaulttarg))
 
 /*
- * RelationGetFillFactor
- *		Returns the relation's fillfactor.  Note multiple eval of argument!
+ * HeapGetFillFactor
+ *		Returns the heap's fillfactor.  Note multiple eval of argument!
  */
-#define RelationGetFillFactor(relation, defaultff) \
-	((relation)->rd_options ? \
-	 ((StdRdOptions *) (relation)->rd_options)->fillfactor : (defaultff))
+#define HeapGetFillFactor(relation)									\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION ||		\
+				 relation->rd_rel->relkind == RELKIND_MATVIEW),			\
+	 (relation)->rd_options ? 											\
+	  ((HeapOptions *) (relation)->rd_options)->fillfactor : 		\
+									(HEAP_DEFAULT_FILLFACTOR))
 
 /*
- * RelationGetTargetPageUsage
+ * HeapGetTargetPageUsage
  *		Returns the relation's desired space usage per page in bytes.
  */
-#define RelationGetTargetPageUsage(relation, defaultff) \
-	(BLCKSZ * RelationGetFillFactor(relation, defaultff) / 100)
+#define HeapGetTargetPageUsage(relation) \
+	(BLCKSZ * HeapGetFillFactor(relation) / 100)
 
 /*
- * RelationGetTargetPageFreeSpace
+ * HeapGetTargetPageFreeSpace
  *		Returns the relation's desired freespace per page in bytes.
  */
-#define RelationGetTargetPageFreeSpace(relation, defaultff) \
-	(BLCKSZ * (100 - RelationGetFillFactor(relation, defaultff)) / 100)
+#define HeapGetTargetPageFreeSpace(relation) \
+	(BLCKSZ * (100 - HeapGetFillFactor(relation)) / 100)
 
 /*
  * RelationIsUsedAsCatalogTable
@@ -349,16 +368,27 @@ typedef struct StdRdOptions
 	((relation)->rd_options && \
 	 ((relation)->rd_rel->relkind == RELKIND_RELATION || \
 	  (relation)->rd_rel->relkind == RELKIND_MATVIEW) ? \
-	 ((StdRdOptions *) (relation)->rd_options)->user_catalog_table : false)
+	 ((HeapOptions *) (relation)->rd_options)->user_catalog_table : false)
 
 /*
  * RelationGetParallelWorkers
- *		Returns the relation's parallel_workers reloption setting.
+ *		Returns the heap's parallel_workers reloption setting.
  *		Note multiple eval of argument!
  */
-#define RelationGetParallelWorkers(relation, defaultpw) \
-	((relation)->rd_options ? \
-	 ((StdRdOptions *) (relation)->rd_options)->parallel_workers : (defaultpw))
+#define RelationGetParallelWorkers(relation, defaultpw) 				\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION ||		\
+				 relation->rd_rel->relkind == RELKIND_MATVIEW),			\
+	 (relation)->rd_options ? 											\
+	  ((HeapOptions *) (relation)->rd_options)->parallel_workers : 	\
+			(defaultpw))
+
+/*
+ * ToastGetTargetPageFreeSpace
+ *		Returns the TOAST relation's desired freespace per page in bytes.
+ *		Always calculated using default fillfactor value.
+ */
+#define ToastGetTargetPageFreeSpace() \
+	(BLCKSZ * (100 - TOAST_DEFAULT_FILLFACTOR) / 100)
 
 /* ViewOptions->check_option values */
 typedef enum ViewOptCheckOption

Georgios Kokolatos

gkokolatos@protonmail.com

over 5 years ago

In reply to: Nikolay Shaplov (#8)

Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions

The following review has been posted through the commitfest application:
make installcheck-world: not tested
Implements feature: not tested
Spec compliant: not tested
Documentation: not tested

Hi,

thank you for the patch. It applies cleanly, compiles and passes check, check-world.

I feel as per the discussion, this is a step to the right direction yet it does not get far enough. From experience, I can confirm that dealing with reloptions in a new table AM is somewhat of a pain. Ultimately, reloptions should be handled by the table AM specific code. The current patch does not address the issue. Yet it does make the issue easier to address by clearing up the current state.

If you allow me, I have a couple of comments.

-	saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
-												   HEAP_DEFAULT_FILLFACTOR);
+	if (IsToastRelation(relation))
+		saveFreeSpace = ToastGetTargetPageFreeSpace();
+	else
+		saveFreeSpace = HeapGetTargetPageFreeSpace(relation);

For balance, it does make some sense for ToastGetTargetPageFreeSpace() to get relation as an argument, similarly to HeapGetTargetPageFreeSpace().
Also, this pattern is repeated in four places, maybe the branch can be moved inside a macro or static inline instead?

-       /* Retrieve the parallel_workers reloption, or -1 if not set. */
-       rel->rel_parallel_workers = RelationGetParallelWorkers(relation, -1);
+       /*
+        * Retrieve the parallel_workers for heap and mat.view relations.
+        * Use -1 if not set, or if we are dealing with other relation kinds
+        */
+       if (relation->rd_rel->relkind == RELKIND_RELATION ||
+               relation->rd_rel->relkind == RELKIND_MATVIEW)
+               rel->rel_parallel_workers = RelationGetParallelWorkers(relation, -1);
+       else
+               rel->rel_parallel_workers = -1;

If the comment above is agreed upon, then it makes a bit of sense to apply the same here. The expression in the branch is already asserted for in macro, why not switch there and remove the responsibility from the caller?

Any thoughts on the above?

Cheers,
Georgios

The new status of this patch is: Waiting on Author

#10

Nikolay Shaplov

dhyan@nataraj.su

over 5 years ago

In reply to: Georgios Kokolatos (#9)

1 attachment(s)

Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions

В письме от понедельник, 20 июля 2020 г. 18:36:44 MSK пользователь Georgios
Kokolatos написал:

Hi! Sorry for really long delay, I was at my summer vacations, and then has
urgent things to finish first. :-( Now I hope we can continue...

thank you for the patch. It applies cleanly, compiles and passes check,
check-world.

Thank you for reviewing efforts.

I feel as per the discussion, this is a step to the right direction yet it
does not get far enough. From experience, I can confirm that dealing with
reloptions in a new table AM is somewhat of a pain. Ultimately, reloptions
should be handled by the table AM specific code. The current patch does not
address the issue. Yet it does make the issue easier to address by clearing
up the current state.

Moving reloptions to AM code is the goal I am slowly moving to. I've started
some time ago with big patch https://commitfest.postgresql.org/14/992/ and
have been told to split it into smaller parts. So I did, and this patch is
last step that cleans options related things up, and then actual moving can be
done.

If you allow me, I have a couple of comments.
-	saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
-												   HEAP_DEFAULT_FILLFACTOR);
+	if (IsToastRelation(relation))
+		saveFreeSpace = ToastGetTargetPageFreeSpace();
+	else
+		saveFreeSpace = HeapGetTargetPageFreeSpace(relation);
For balance, it does make some sense for ToastGetTargetPageFreeSpace() to
get relation as an argument, similarly to HeapGetTargetPageFreeSpace().

ToastGetTargetPageFreeSpace return a const value. I've change the code, so it
gets relation argument, that is not used, the way you suggested. But I am not
sure if it is good or bad idea. May be we will get some "Unused variable"
warning on some compilers. I like consistency... But not sure we need it here.

- /* Retrieve the parallel_workers reloption, or -1 if not set. */
- rel->rel_parallel_workers = RelationGetParallelWorkers(relation,
-1);

+ /*

+        * Retrieve the parallel_workers for heap and mat.view relations.
+        * Use -1 if not set, or if we are dealing with other relation
kinds

+ */

+       if (relation->rd_rel->relkind == RELKIND_RELATION ||
+               relation->rd_rel->relkind == RELKIND_MATVIEW)
+               rel->rel_parallel_workers =
RelationGetParallelWorkers(relation, -1);

+ else

+ rel->rel_parallel_workers = -1;
Also, this pattern is repeated in four places, maybe the branch can be
moved inside a macro or static inline instead?

If the comment above is agreed upon, then it makes a bit of sense to apply
the same here. The expression in the branch is already asserted for in
macro, why not switch there and remove the responsibility from the caller?

I guess here you are right, because here the logic is following: for heap
relation take option from options, for _all_ others use -1. This can be moved
to macro.

So I changed it to

/*
* HeapGetParallelWorkers
* Returns the heap's parallel_workers reloption setting.
* Note multiple eval of argument!
*/
#define HeapGetParallelWorkers(relation, defaultpw) \
(AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION || \
relation->rd_rel->relkind == RELKIND_MATVIEW), \
(relation)->rd_options ? \
((HeapOptions *) (relation)->rd_options)->parallel_workers : \
(defaultpw))

/*
* RelationGetParallelWorkers
* Returns the relation's parallel_workers reloption setting.
* Note multiple eval of argument!
*/

#define RelationGetParallelWorkers(relation, defaultpw) \
(((relation)->rd_rel->relkind == RELKIND_RELATION || \
(relation)->rd_rel->relkind == RELKIND_MATVIEW) ? \
HeapGetParallelWorkers(relation, defaultpw) : defaultpw)

But I would not like to move

if (IsToastRelation(relation))
saveFreeSpace = ToastGetTargetPageFreeSpace(relation);
else
saveFreeSpace = HeapGetTargetPageFreeSpace(relation);

into macros, as there is a choice only between heap and toast. All other
relation types are not mentioned.

So we can not call it RelationGetTargetPageFreeSpace. It would be
ToastOrHeapGetTargetPageFreeSpace actually. Better not to have such macro.

Please find new version of the patch in the attachment.

--
Software Developer: https://www.upwork.com/freelancers/~014a87e140ff02c0da
Body-oriented Therapist: https://vk.com/nataraj_rebalancing (Russian)

Attachments:

StdRdOptions_to_HeapOptions_and_ToastOptions_v2.difftext/x-patch; charset=UTF-8; name=StdRdOptions_to_HeapOptions_and_ToastOptions_v2.diffDownload

diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 8ccc228..6118b55 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -46,9 +46,8 @@
  * upper and lower bounds (if applicable); for strings, consider a validation
  * routine.
  * (ii) add a record below (or use add_<type>_reloption).
- * (iii) add it to the appropriate options struct (perhaps StdRdOptions)
- * (iv) add it to the appropriate handling routine (perhaps
- * default_reloptions)
+ * (iii) add it to the appropriate options struct
+ * (iv) add it to the appropriate handling routine
  * (v) make sure the lock level is set correctly for that operation
  * (vi) don't forget to document the option
  *
@@ -1375,9 +1374,11 @@ extractRelOptions(HeapTuple tuple, TupleDesc tupdesc,
 	switch (classForm->relkind)
 	{
 		case RELKIND_RELATION:
-		case RELKIND_TOASTVALUE:
 		case RELKIND_MATVIEW:
-			options = heap_reloptions(classForm->relkind, datum, false);
+			options = heap_reloptions(datum, false);
+			break;
+		case RELKIND_TOASTVALUE:
+			options = toast_reloptions(datum, false);
 			break;
 		case RELKIND_PARTITIONED_TABLE:
 			options = partitioned_table_reloptions(datum, false);
@@ -1811,59 +1812,66 @@ fillRelOptions(void *rdopts, Size basesize,
 
 
 /*
- * Option parser for anything that uses StdRdOptions.
+ * Option parsing definition for autovacuum. Used in toast and heap options.
+ */
+
+#define AUTOVACUUM_RELOPTIONS(OFFSET)                                \
+		{"autovacuum_enabled", RELOPT_TYPE_BOOL,                     \
+		OFFSET + offsetof(AutoVacOpts, enabled)},                    \
+		{"autovacuum_vacuum_threshold", RELOPT_TYPE_INT,             \
+		OFFSET + offsetof(AutoVacOpts, vacuum_threshold)},           \
+		{"autovacuum_vacuum_insert_threshold", RELOPT_TYPE_INT,      \
+		OFFSET + offsetof(AutoVacOpts, vacuum_ins_threshold)},       \
+		{"autovacuum_analyze_threshold", RELOPT_TYPE_INT,            \
+		OFFSET + offsetof(AutoVacOpts, analyze_threshold)},          \
+		{"autovacuum_vacuum_cost_delay", RELOPT_TYPE_REAL,           \
+		OFFSET + offsetof(AutoVacOpts, vacuum_cost_delay)},          \
+		{"autovacuum_vacuum_cost_limit", RELOPT_TYPE_INT,            \
+		OFFSET + offsetof(AutoVacOpts, vacuum_cost_limit)},          \
+		{"autovacuum_freeze_min_age", RELOPT_TYPE_INT,               \
+		OFFSET + offsetof(AutoVacOpts, freeze_min_age)},             \
+		{"autovacuum_freeze_max_age", RELOPT_TYPE_INT,               \
+		OFFSET + offsetof(AutoVacOpts, freeze_max_age)},             \
+		{"autovacuum_freeze_table_age", RELOPT_TYPE_INT,             \
+		OFFSET + offsetof(AutoVacOpts, freeze_table_age)},           \
+		{"autovacuum_multixact_freeze_min_age", RELOPT_TYPE_INT,     \
+		OFFSET + offsetof(AutoVacOpts, multixact_freeze_min_age)},   \
+		{"autovacuum_multixact_freeze_max_age", RELOPT_TYPE_INT,     \
+		OFFSET + offsetof(AutoVacOpts, multixact_freeze_max_age)},   \
+		{"autovacuum_multixact_freeze_table_age", RELOPT_TYPE_INT,   \
+		OFFSET + offsetof(AutoVacOpts, multixact_freeze_table_age)}, \
+		{"log_autovacuum_min_duration", RELOPT_TYPE_INT,             \
+		OFFSET + offsetof(AutoVacOpts, log_min_duration)},           \
+		{"autovacuum_vacuum_scale_factor", RELOPT_TYPE_REAL,         \
+		OFFSET + offsetof(AutoVacOpts, vacuum_scale_factor)},        \
+		{"autovacuum_vacuum_insert_scale_factor", RELOPT_TYPE_REAL,  \
+		OFFSET  + offsetof(AutoVacOpts, vacuum_ins_scale_factor)},   \
+		{"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL,        \
+		OFFSET + offsetof(AutoVacOpts, analyze_scale_factor)}
+/*
+ * Option parser for heap
  */
 bytea *
-default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
+heap_reloptions(Datum reloptions, bool validate)
 {
 	static const relopt_parse_elt tab[] = {
-		{"fillfactor", RELOPT_TYPE_INT, offsetof(StdRdOptions, fillfactor)},
-		{"autovacuum_enabled", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, enabled)},
-		{"autovacuum_vacuum_threshold", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_threshold)},
-		{"autovacuum_vacuum_insert_threshold", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_ins_threshold)},
-		{"autovacuum_analyze_threshold", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, analyze_threshold)},
-		{"autovacuum_vacuum_cost_limit", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_limit)},
-		{"autovacuum_freeze_min_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_min_age)},
-		{"autovacuum_freeze_max_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_max_age)},
-		{"autovacuum_freeze_table_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, freeze_table_age)},
-		{"autovacuum_multixact_freeze_min_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_min_age)},
-		{"autovacuum_multixact_freeze_max_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_max_age)},
-		{"autovacuum_multixact_freeze_table_age", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, multixact_freeze_table_age)},
-		{"log_autovacuum_min_duration", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, log_min_duration)},
+		{"fillfactor", RELOPT_TYPE_INT, offsetof(HeapOptions, fillfactor)},
+		AUTOVACUUM_RELOPTIONS(offsetof(HeapOptions, autovacuum)),
 		{"toast_tuple_target", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, toast_tuple_target)},
-		{"autovacuum_vacuum_cost_delay", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_cost_delay)},
-		{"autovacuum_vacuum_scale_factor", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_scale_factor)},
-		{"autovacuum_vacuum_insert_scale_factor", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, vacuum_ins_scale_factor)},
-		{"autovacuum_analyze_scale_factor", RELOPT_TYPE_REAL,
-		offsetof(StdRdOptions, autovacuum) + offsetof(AutoVacOpts, analyze_scale_factor)},
+		offsetof(HeapOptions, toast_tuple_target)},
 		{"user_catalog_table", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, user_catalog_table)},
+		offsetof(HeapOptions, user_catalog_table)},
 		{"parallel_workers", RELOPT_TYPE_INT,
-		offsetof(StdRdOptions, parallel_workers)},
+		offsetof(HeapOptions, parallel_workers)},
 		{"vacuum_index_cleanup", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, vacuum_index_cleanup)},
+		offsetof(HeapOptions, vacuum_index_cleanup)},
 		{"vacuum_truncate", RELOPT_TYPE_BOOL,
-		offsetof(StdRdOptions, vacuum_truncate)}
+		offsetof(HeapOptions, vacuum_truncate)}
 	};
 
-	return (bytea *) build_reloptions(reloptions, validate, kind,
-									  sizeof(StdRdOptions),
+	return (bytea *) build_reloptions(reloptions, validate,
+									  RELOPT_KIND_HEAP,
+									  sizeof(HeapOptions),
 									  tab, lengthof(tab));
 }
 
@@ -1971,6 +1979,32 @@ partitioned_table_reloptions(Datum reloptions, bool validate)
 }
 
 /*
+ * Option parser for toast
+ */
+bytea *
+toast_reloptions(Datum reloptions, bool validate)
+{
+	ToastOptions *rdopts;
+	static const relopt_parse_elt tab[] = {
+		AUTOVACUUM_RELOPTIONS(offsetof(ToastOptions, autovacuum)),
+		{"vacuum_index_cleanup", RELOPT_TYPE_BOOL,
+		offsetof(ToastOptions, vacuum_index_cleanup)},
+		{"vacuum_truncate", RELOPT_TYPE_BOOL,
+		offsetof(ToastOptions, vacuum_truncate)}
+	};
+
+	rdopts = build_reloptions(reloptions, validate,
+									  RELOPT_KIND_TOAST,
+									  sizeof(ToastOptions),
+									  tab, lengthof(tab));
+	/* adjust default-only parameters for TOAST relations */
+	rdopts->autovacuum.analyze_threshold = -1;
+	rdopts->autovacuum.analyze_scale_factor = -1;
+
+	return (bytea *) rdopts;
+}
+
+/*
  * Option parser for views
  */
 bytea *
@@ -1990,37 +2024,6 @@ view_reloptions(Datum reloptions, bool validate)
 }
 
 /*
- * Parse options for heaps, views and toast tables.
- */
-bytea *
-heap_reloptions(char relkind, Datum reloptions, bool validate)
-{
-	StdRdOptions *rdopts;
-
-	switch (relkind)
-	{
-		case RELKIND_TOASTVALUE:
-			rdopts = (StdRdOptions *)
-				default_reloptions(reloptions, validate, RELOPT_KIND_TOAST);
-			if (rdopts != NULL)
-			{
-				/* adjust default-only parameters for TOAST relations */
-				rdopts->fillfactor = 100;
-				rdopts->autovacuum.analyze_threshold = -1;
-				rdopts->autovacuum.analyze_scale_factor = -1;
-			}
-			return (bytea *) rdopts;
-		case RELKIND_RELATION:
-		case RELKIND_MATVIEW:
-			return default_reloptions(reloptions, validate, RELOPT_KIND_HEAP);
-		default:
-			/* other relkinds are not supported */
-			return NULL;
-	}
-}
-
-
-/*
  * Parse options for indexes.
  *
  *	amoptions	index AM's option parser function
diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index 9b5f417..473eb4d 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -2114,8 +2114,10 @@ heap_multi_insert(Relation relation, TupleTableSlot **slots, int ntuples,
 	AssertArg(!(options & HEAP_INSERT_NO_LOGICAL));
 
 	needwal = RelationNeedsWAL(relation);
-	saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
-												   HEAP_DEFAULT_FILLFACTOR);
+	if (IsToastRelation(relation))
+		saveFreeSpace = ToastGetTargetPageFreeSpace(relation);
+	else
+		saveFreeSpace = HeapGetTargetPageFreeSpace(relation);
 
 	/* Toast and set header data in all the slots */
 	heaptuples = palloc(ntuples * sizeof(HeapTuple));
diff --git a/src/backend/access/heap/hio.c b/src/backend/access/heap/hio.c
index aa3f14c..80e14b0 100644
--- a/src/backend/access/heap/hio.c
+++ b/src/backend/access/heap/hio.c
@@ -19,6 +19,7 @@
 #include "access/hio.h"
 #include "access/htup_details.h"
 #include "access/visibilitymap.h"
+#include "catalog/catalog.h"
 #include "storage/bufmgr.h"
 #include "storage/freespace.h"
 #include "storage/lmgr.h"
@@ -346,8 +347,10 @@ RelationGetBufferForTuple(Relation relation, Size len,
 						len, MaxHeapTupleSize)));
 
 	/* Compute desired extra freespace due to fillfactor option */
-	saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
-												   HEAP_DEFAULT_FILLFACTOR);
+	if (IsToastRelation(relation))
+		saveFreeSpace = ToastGetTargetPageFreeSpace(relation);
+	else
+		saveFreeSpace = HeapGetTargetPageFreeSpace(relation);
 
 	if (otherBuffer != InvalidBuffer)
 		otherBlock = BufferGetBlockNumber(otherBuffer);
diff --git a/src/backend/access/heap/pruneheap.c b/src/backend/access/heap/pruneheap.c
index bc510e2..f23a306 100644
--- a/src/backend/access/heap/pruneheap.c
+++ b/src/backend/access/heap/pruneheap.c
@@ -164,8 +164,11 @@ heap_page_prune_opt(Relation relation, Buffer buffer)
 	 * important than sometimes getting a wrong answer in what is after all
 	 * just a heuristic estimate.
 	 */
-	minfree = RelationGetTargetPageFreeSpace(relation,
-											 HEAP_DEFAULT_FILLFACTOR);
+
+	if (IsToastRelation(relation))
+		minfree = ToastGetTargetPageFreeSpace(relation);
+	else
+		minfree = HeapGetTargetPageFreeSpace(relation);
 	minfree = Max(minfree, BLCKSZ / 10);
 
 	if (PageIsFull(page) || PageGetHeapFreeSpace(page) < minfree)
diff --git a/src/backend/access/heap/rewriteheap.c b/src/backend/access/heap/rewriteheap.c
index 39e3376..5af0157 100644
--- a/src/backend/access/heap/rewriteheap.c
+++ b/src/backend/access/heap/rewriteheap.c
@@ -663,8 +663,10 @@ raw_heap_insert(RewriteState state, HeapTuple tup)
 						len, MaxHeapTupleSize)));
 
 	/* Compute desired extra freespace due to fillfactor option */
-	saveFreeSpace = RelationGetTargetPageFreeSpace(state->rs_new_rel,
-												   HEAP_DEFAULT_FILLFACTOR);
+	if (IsToastRelation(state->rs_new_rel))
+		saveFreeSpace = ToastGetTargetPageFreeSpace(state->rs_new_rel);
+	else
+		saveFreeSpace = HeapGetTargetPageFreeSpace(state->rs_new_rel);
 
 	/* Now we can check to see if there's enough free space already. */
 	if (state->rs_buffer_valid)
diff --git a/src/backend/commands/createas.c b/src/backend/commands/createas.c
index d53ec95..c3942ea 100644
--- a/src/backend/commands/createas.c
+++ b/src/backend/commands/createas.c
@@ -130,7 +130,7 @@ create_ctas_internal(List *attrList, IntoClause *into)
 										validnsps,
 										true, false);
 
-	(void) heap_reloptions(RELKIND_TOASTVALUE, toast_options, true);
+	(void) toast_reloptions(toast_options, true);
 
 	NewRelationCreateToastTable(intoRelationAddr.objectId, toast_options);
 
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 3e57c7f..63b0c1d 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -756,8 +756,12 @@ DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId,
 		case RELKIND_PARTITIONED_TABLE:
 			(void) partitioned_table_reloptions(reloptions, true);
 			break;
-		default:
-			(void) heap_reloptions(relkind, reloptions, true);
+		case RELKIND_TOASTVALUE:
+			(void) toast_reloptions(reloptions, true);
+			break;
+		case RELKIND_RELATION:
+		case RELKIND_MATVIEW:
+			(void) heap_reloptions(reloptions, true);
 	}
 
 	if (stmt->ofTypename)
@@ -12932,9 +12936,11 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 	switch (rel->rd_rel->relkind)
 	{
 		case RELKIND_RELATION:
-		case RELKIND_TOASTVALUE:
 		case RELKIND_MATVIEW:
-			(void) heap_reloptions(rel->rd_rel->relkind, newOptions, true);
+			(void) heap_reloptions( newOptions, true);
+			break;
+		case RELKIND_TOASTVALUE:
+			(void) toast_reloptions(newOptions, true);
 			break;
 		case RELKIND_PARTITIONED_TABLE:
 			(void) partitioned_table_reloptions(newOptions, true);
@@ -13046,7 +13052,7 @@ ATExecSetRelOptions(Relation rel, List *defList, AlterTableType operation,
 										 defList, "toast", validnsps, false,
 										 operation == AT_ResetRelOptions);
 
-		(void) heap_reloptions(RELKIND_TOASTVALUE, newOptions, true);
+		(void) toast_reloptions(newOptions, true);
 
 		memset(repl_val, 0, sizeof(repl_val));
 		memset(repl_null, false, sizeof(repl_null));
diff --git a/src/backend/commands/vacuum.c b/src/backend/commands/vacuum.c
index ddeec87..7729db9 100644
--- a/src/backend/commands/vacuum.c
+++ b/src/backend/commands/vacuum.c
@@ -31,6 +31,7 @@
 #include "access/tableam.h"
 #include "access/transam.h"
 #include "access/xact.h"
+#include "catalog/catalog.h"
 #include "catalog/namespace.h"
 #include "catalog/pg_database.h"
 #include "catalog/pg_inherits.h"
@@ -1878,7 +1879,10 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	if (params->index_cleanup == VACOPT_TERNARY_DEFAULT)
 	{
 		if (onerel->rd_options == NULL ||
-			((StdRdOptions *) onerel->rd_options)->vacuum_index_cleanup)
+			(!IsToastRelation(onerel) &&
+			 ((HeapOptions *) onerel->rd_options)->vacuum_index_cleanup) ||
+			(IsToastRelation(onerel) &&
+			 ((ToastOptions *) onerel->rd_options)->vacuum_index_cleanup))
 			params->index_cleanup = VACOPT_TERNARY_ENABLED;
 		else
 			params->index_cleanup = VACOPT_TERNARY_DISABLED;
@@ -1888,7 +1892,10 @@ vacuum_rel(Oid relid, RangeVar *relation, VacuumParams *params)
 	if (params->truncate == VACOPT_TERNARY_DEFAULT)
 	{
 		if (onerel->rd_options == NULL ||
-			((StdRdOptions *) onerel->rd_options)->vacuum_truncate)
+			(!IsToastRelation(onerel) &&
+			 ((HeapOptions *) onerel->rd_options)->vacuum_truncate) ||
+			(IsToastRelation(onerel) &&
+			 ((ToastOptions *) onerel->rd_options)->vacuum_truncate))
 			params->truncate = VACOPT_TERNARY_ENABLED;
 		else
 			params->truncate = VACOPT_TERNARY_DISABLED;
diff --git a/src/backend/postmaster/autovacuum.c b/src/backend/postmaster/autovacuum.c
index 19ba26b..6e86dbe 100644
--- a/src/backend/postmaster/autovacuum.c
+++ b/src/backend/postmaster/autovacuum.c
@@ -2737,6 +2737,7 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
 {
 	bytea	   *relopts;
 	AutoVacOpts *av;
+	AutoVacOpts *src;
 
 	Assert(((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_RELATION ||
 		   ((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_MATVIEW ||
@@ -2746,8 +2747,13 @@ extract_autovac_opts(HeapTuple tup, TupleDesc pg_class_desc)
 	if (relopts == NULL)
 		return NULL;
 
+	if (((Form_pg_class) GETSTRUCT(tup))->relkind == RELKIND_TOASTVALUE)
+		src = &(((ToastOptions *) relopts)->autovacuum);
+	else
+		src = &(((HeapOptions *) relopts)->autovacuum);
+
 	av = palloc(sizeof(AutoVacOpts));
-	memcpy(av, &(((StdRdOptions *) relopts)->autovacuum), sizeof(AutoVacOpts));
+	memcpy(av, src, sizeof(AutoVacOpts));
 	pfree(relopts);
 
 	return av;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 9713a7a..375f253 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -1179,9 +1179,7 @@ ProcessUtilitySlow(ParseState *pstate,
 																validnsps,
 																true,
 																false);
-							(void) heap_reloptions(RELKIND_TOASTVALUE,
-												   toast_options,
-												   true);
+							(void) toast_reloptions(toast_options, true);
 
 							NewRelationCreateToastTable(address.objectId,
 														toast_options);
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index 5964438..45cce3c 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -233,9 +233,8 @@ extern void *build_reloptions(Datum reloptions, bool validate,
 extern void *build_local_reloptions(local_relopts *relopts, Datum options,
 									bool validate);
 
-extern bytea *default_reloptions(Datum reloptions, bool validate,
-								 relopt_kind kind);
-extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
+extern bytea *toast_reloptions(Datum reloptions, bool validate);
+extern bytea *heap_reloptions(Datum reloptions, bool validate);
 extern bytea *view_reloptions(Datum reloptions, bool validate);
 extern bytea *partitioned_table_reloptions(Datum reloptions, bool validate);
 extern bytea *index_reloptions(amoptions_function amoptions, Datum reloptions,
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index 0b5957b..e475e87 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -266,14 +266,9 @@ typedef struct ForeignKeyCacheInfo
 
 
 /*
- * StdRdOptions
- *		Standard contents of rd_options for heaps.
- *
- * RelationGetFillFactor() and RelationGetTargetPageFreeSpace() can only
- * be applied to relations that use this format or a superset for
- * private options data.
+ * AutoVacOpts
+ *		Auto Vacuum options used both in Heap and Toast relations
  */
- /* autovacuum-related reloptions. */
 typedef struct AutoVacOpts
 {
 	bool		enabled;
@@ -294,7 +289,11 @@ typedef struct AutoVacOpts
 	float8		analyze_scale_factor;
 } AutoVacOpts;
 
-typedef struct StdRdOptions
+/*
+ * HeapOptions
+ *		Binary representation of relation options for Heap relations.
+ */
+typedef struct HeapOptions
 {
 	int32		vl_len_;		/* varlena header (do not touch directly!) */
 	int			fillfactor;		/* page fill factor in percent (0..100) */
@@ -305,40 +304,60 @@ typedef struct StdRdOptions
 	int			parallel_workers;	/* max number of parallel workers */
 	bool		vacuum_index_cleanup;	/* enables index vacuuming and cleanup */
 	bool		vacuum_truncate;	/* enables vacuum to truncate a relation */
-} StdRdOptions;
+} HeapOptions;
 
 #define HEAP_MIN_FILLFACTOR			10
 #define HEAP_DEFAULT_FILLFACTOR		100
 
 /*
+ * ToastOptions
+ *		Binary representation of relation options for Toast relations.
+ */
+typedef struct ToastOptions
+{
+	int32		vl_len_;		/* varlena header (do not touch directly!) */
+	AutoVacOpts autovacuum;		/* autovacuum-related options */
+	bool		vacuum_index_cleanup; /* enables index vacuuming and cleanup */
+	bool		vacuum_truncate;	/* enables vacuum to truncate a relation */
+} ToastOptions;
+
+#define TOAST_DEFAULT_FILLFACTOR	100 /* Only default is actually used */
+
+/*
  * RelationGetToastTupleTarget
- *		Returns the relation's toast_tuple_target.  Note multiple eval of argument!
+ *		Returns the heap's toast_tuple_target.  Note multiple eval of argument!
  */
-#define RelationGetToastTupleTarget(relation, defaulttarg) \
-	((relation)->rd_options ? \
-	 ((StdRdOptions *) (relation)->rd_options)->toast_tuple_target : (defaulttarg))
+#define RelationGetToastTupleTarget(relation, defaulttarg)				\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION ||		\
+				 relation->rd_rel->relkind == RELKIND_MATVIEW),			\
+	 (relation)->rd_options ? 											\
+	  ((HeapOptions *) (relation)->rd_options)->toast_tuple_target : \
+			(defaulttarg))
 
 /*
- * RelationGetFillFactor
- *		Returns the relation's fillfactor.  Note multiple eval of argument!
+ * HeapGetFillFactor
+ *		Returns the heap's fillfactor.  Note multiple eval of argument!
  */
-#define RelationGetFillFactor(relation, defaultff) \
-	((relation)->rd_options ? \
-	 ((StdRdOptions *) (relation)->rd_options)->fillfactor : (defaultff))
+#define HeapGetFillFactor(relation)									\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION ||		\
+				 relation->rd_rel->relkind == RELKIND_MATVIEW),			\
+	 (relation)->rd_options ? 											\
+	  ((HeapOptions *) (relation)->rd_options)->fillfactor : 		\
+									(HEAP_DEFAULT_FILLFACTOR))
 
 /*
- * RelationGetTargetPageUsage
+ * HeapGetTargetPageUsage
  *		Returns the relation's desired space usage per page in bytes.
  */
-#define RelationGetTargetPageUsage(relation, defaultff) \
-	(BLCKSZ * RelationGetFillFactor(relation, defaultff) / 100)
+#define HeapGetTargetPageUsage(relation) \
+	(BLCKSZ * HeapGetFillFactor(relation) / 100)
 
 /*
- * RelationGetTargetPageFreeSpace
+ * HeapGetTargetPageFreeSpace
  *		Returns the relation's desired freespace per page in bytes.
  */
-#define RelationGetTargetPageFreeSpace(relation, defaultff) \
-	(BLCKSZ * (100 - RelationGetFillFactor(relation, defaultff)) / 100)
+#define HeapGetTargetPageFreeSpace(relation) \
+	(BLCKSZ * (100 - HeapGetFillFactor(relation)) / 100)
 
 /*
  * RelationIsUsedAsCatalogTable
@@ -349,16 +368,38 @@ typedef struct StdRdOptions
 	((relation)->rd_options && \
 	 ((relation)->rd_rel->relkind == RELKIND_RELATION || \
 	  (relation)->rd_rel->relkind == RELKIND_MATVIEW) ? \
-	 ((StdRdOptions *) (relation)->rd_options)->user_catalog_table : false)
+	 ((HeapOptions *) (relation)->rd_options)->user_catalog_table : false)
+
+/*
+ * HeapGetParallelWorkers
+ *		Returns the heap's parallel_workers reloption setting.
+ *		Note multiple eval of argument!
+ */
+#define HeapGetParallelWorkers(relation, defaultpw) 					\
+	(AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION ||		\
+				 relation->rd_rel->relkind == RELKIND_MATVIEW),			\
+	 (relation)->rd_options ? 											\
+	  ((HeapOptions *) (relation)->rd_options)->parallel_workers : 		\
+			(defaultpw))
 
 /*
  * RelationGetParallelWorkers
  *		Returns the relation's parallel_workers reloption setting.
  *		Note multiple eval of argument!
  */
-#define RelationGetParallelWorkers(relation, defaultpw) \
-	((relation)->rd_options ? \
-	 ((StdRdOptions *) (relation)->rd_options)->parallel_workers : (defaultpw))
+
+#define RelationGetParallelWorkers(relation, defaultpw) 				\
+	(((relation)->rd_rel->relkind == RELKIND_RELATION ||				\
+	  (relation)->rd_rel->relkind == RELKIND_MATVIEW) ? 				\
+	  HeapGetParallelWorkers(relation, defaultpw) : defaultpw)
+
+/*
+ * ToastGetTargetPageFreeSpace
+ *		Returns the TOAST relation's desired freespace per page in bytes.
+ *		Always calculated using default fillfactor value.
+ */
+#define ToastGetTargetPageFreeSpace(relation) \
+	(BLCKSZ * (100 - TOAST_DEFAULT_FILLFACTOR) / 100)
 
 /* ViewOptions->check_option values */
 typedef enum ViewOptCheckOption

#11

David Steele

david@pgmasters.net

almost 5 years ago

In reply to: Nikolay Shaplov (#10)

Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions

Hi Georgios,

On 9/13/20 12:04 PM, Nikolay Shaplov wrote:

В письме от понедельник, 20 июля 2020 г. 18:36:44 MSK пользователь Georgios
Kokolatos написал:

thank you for the patch. It applies cleanly, compiles and passes check,
check-world.

Thank you for reviewing efforts.

<snip>

Please find new version of the patch in the attachment.

Any thoughts on the updated patch?

Regards,
--
-David
david@pgmasters.net

#12

Jeff Davis

pgsql@j-davis.com

over 4 years ago

In reply to: Nikolay Shaplov (#10)

Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions

On Sun, 2020-09-13 at 19:04 +0300, Nikolay Shaplov wrote:

Moving reloptions to AM code is the goal I am slowly moving to. I've
started
some time ago with big patch
https://commitfest.postgresql.org/14/992/ and
have been told to split it into smaller parts. So I did, and this
patch is
last step that cleans options related things up, and then actual
moving can be
done.

Thank you for working on this.

Can you outline the plan for moving these options to the table AM to
make sure this patch is a step in the right direction?

I was trying to work through this problem as well[1]/messages/by-id/43c6ec161f930e385dbc3169a065a917cfc60714.camel@j-davis.com, and there are a
few complications.

* Which options apply to any relation (of any table AM), and which
apply to only heaps? As far as I can tell, the only one that seems
heap-specific is "fillfactor".

* Toast tables can be any AM, as well, so if we accept new reloptions
for a custom AM, we also need to accept options for toast tables of
that AM.

* Implementation-wise, the bytea representation of the options is not
very easy to extend. Should we have a new text field in the catalog to
hold the custom options?

Regards,
Jeff Davis

[1]: /messages/by-id/43c6ec161f930e385dbc3169a065a917cfc60714.camel@j-davis.com
/messages/by-id/43c6ec161f930e385dbc3169a065a917cfc60714.camel@j-davis.com

#13

Nikolay Shaplov

dhyan@nataraj.su

over 4 years ago

In reply to: Jeff Davis (#12)

Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions

В письме от пятница, 4 июня 2021 г. 3:19:09 MSK пользователь Jeff Davis
написал:

Moving reloptions to AM code is the goal I am slowly moving to. I've
started
some time ago with big patch
https://commitfest.postgresql.org/14/992/ and
have been told to split it into smaller parts. So I did, and this
patch is
last step that cleans options related things up, and then actual
moving can be
done.

Thank you for working on this.

Welcome!
Sorry for slow reply, I am on summer vacations now, no chance for fast replies
now :-)

Can you outline the plan for moving these options to the table AM to
make sure this patch is a step in the right direction?

Yes, I can. First you can see the whole patch, the way it was several years
ago: https://commitfest.postgresql.org/15/992/, reloptions8a.diff
The things I would say is accurate for postgres ~11, it may have been changed
since I last payed attention to them.

So, there are three general places where options can be stored:
1. Global boolRelOpts, intRelOpts, realRelOpts, stringRelOpts in src/backend/
access/common/reloptions.c for in-core access methods.

2. custom_options array of accessable via add_bool_reloption,
add_int_reloption, add_real_reloption, add_string_reloption for access methods
from contribs. (See reloptions.c too)

3. And also each access method has an array of relopt_parse_elt[] that is
also about reloptions.

1 and 2 are technically arrays of relopt_get, and store information what kind
of options do we have.

3 is array of relopt_parse_elt[] that store information how options should be
stored into binary representation.

My idea was to merge relopt_get and relopt_parse_elt into one structure (in my
patch it is "option_definition_basic"), each access method, that needs options,
should have a set of option_definition_basic for their options (called
option_definition_basic in my patch, may be should be renamed)

and this set of option_definitions is the only data that is needed to parse
option into binary representation.

So in access method instead of am_option function we will have
amrelopt_catalog function that returns "options defenition set" for this am,
and this definition set is used by option parser to parse options.

So, it my explanation is not quite clear, please ask questions, I will try to
answer them.

I was trying to work through this problem as well[1], and there are a
few complications.

* Which options apply to any relation (of any table AM), and which
apply to only heaps? As far as I can tell, the only one that seems
heap-specific is "fillfactor".

From my point of view, each relation kind has it's own set of options.
The fact, that almost every kind has a fillfactor is just a coincidence.
If we try to do some optimization here, we will be buried under the complexity
of it soon. So they are _different_ options just having the same name.

* Toast tables can be any AM, as well, so if we accept new reloptions
for a custom AM, we also need to accept options for toast tables of
that AM.

When I wrote this patch, AM was introduced only to index relations.
I do not how it is implemented for heap now, but there should be some logic in
it. If toast tables has it's own AM, then option definition set should be
stored there, and we should find a way to work with it, somehow.

* Implementation-wise, the bytea representation of the options is not
very easy to extend. Should we have a new text field in the catalog to
hold the custom options?

I am not really understanding this question.

Normally all options can be well represented as binary structure stored at
bytea. I see no problem here. If we need some unusual behaviour, we can use
string option with custom validation function. This should cover almost all
needs I can imagine.

=======

So it you are interested in having better option implementation, and has no
ideas of your own, I would suggest to revive my patch and try to commit it.
What I need first of all is a reviewer. Testing and coauthoring will also be
apriciated.

My original big patch, I gave you link to, have been split into several parts.
The last minor part, that should be commit in advance, and have not been
commit yet is https://commitfest.postgresql.org/33/2370/
If you join as a reviewer this would be splendid! :-)

--
Nikolay Shaplov
dhyan@nataraj.su (e-mail, jabber)
@dhyan:nataraj.su (matrix)

#14

vignesh C

vignesh21@gmail.com

over 4 years ago

In reply to: Nikolay Shaplov (#10)

Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions

On Sun, Sep 13, 2020 at 9:34 PM Nikolay Shaplov <dhyan@nataraj.su> wrote:

В письме от понедельник, 20 июля 2020 г. 18:36:44 MSK пользователь Georgios
Kokolatos написал:

Hi! Sorry for really long delay, I was at my summer vacations, and then has
urgent things to finish first. :-( Now I hope we can continue...

thank you for the patch. It applies cleanly, compiles and passes check,
check-world.

Thank you for reviewing efforts.

I feel as per the discussion, this is a step to the right direction yet it
does not get far enough. From experience, I can confirm that dealing with
reloptions in a new table AM is somewhat of a pain. Ultimately, reloptions
should be handled by the table AM specific code. The current patch does not
address the issue. Yet it does make the issue easier to address by clearing
up the current state.

Moving reloptions to AM code is the goal I am slowly moving to. I've started
some time ago with big patch https://commitfest.postgresql.org/14/992/ and
have been told to split it into smaller parts. So I did, and this patch is
last step that cleans options related things up, and then actual moving can be
done.
If you allow me, I have a couple of comments.
-     saveFreeSpace = RelationGetTargetPageFreeSpace(relation,
-                                                                                                HEAP_DEFAULT_FILLFACTOR);
+     if (IsToastRelation(relation))
+             saveFreeSpace = ToastGetTargetPageFreeSpace();
+     else
+             saveFreeSpace = HeapGetTargetPageFreeSpace(relation);
For balance, it does make some sense for ToastGetTargetPageFreeSpace() to
get relation as an argument, similarly to HeapGetTargetPageFreeSpace().
ToastGetTargetPageFreeSpace return a const value. I've change the code, so it
gets relation argument, that is not used, the way you suggested. But I am not
sure if it is good or bad idea. May be we will get some "Unused variable"
warning on some compilers. I like consistency... But not sure we need it here.

- /* Retrieve the parallel_workers reloption, or -1 if not set. */
- rel->rel_parallel_workers = RelationGetParallelWorkers(relation,
-1);

+ /*
+        * Retrieve the parallel_workers for heap and mat.view relations.
+        * Use -1 if not set, or if we are dealing with other relation
kinds
+ */
+       if (relation->rd_rel->relkind == RELKIND_RELATION ||
+               relation->rd_rel->relkind == RELKIND_MATVIEW)
+               rel->rel_parallel_workers =
RelationGetParallelWorkers(relation, -1);
+ else

+ rel->rel_parallel_workers = -1;
Also, this pattern is repeated in four places, maybe the branch can be
moved inside a macro or static inline instead?

If the comment above is agreed upon, then it makes a bit of sense to apply
the same here. The expression in the branch is already asserted for in
macro, why not switch there and remove the responsibility from the caller?

I guess here you are right, because here the logic is following: for heap
relation take option from options, for _all_ others use -1. This can be moved
to macro.

So I changed it to

/*
* HeapGetParallelWorkers
* Returns the heap's parallel_workers reloption setting.
* Note multiple eval of argument!
*/
#define HeapGetParallelWorkers(relation, defaultpw) \
(AssertMacro(relation->rd_rel->relkind == RELKIND_RELATION || \
relation->rd_rel->relkind == RELKIND_MATVIEW), \
(relation)->rd_options ? \
((HeapOptions *) (relation)->rd_options)->parallel_workers : \
(defaultpw))

/*
* RelationGetParallelWorkers
* Returns the relation's parallel_workers reloption setting.
* Note multiple eval of argument!
*/

#define RelationGetParallelWorkers(relation, defaultpw) \
(((relation)->rd_rel->relkind == RELKIND_RELATION || \
(relation)->rd_rel->relkind == RELKIND_MATVIEW) ? \
HeapGetParallelWorkers(relation, defaultpw) : defaultpw)

But I would not like to move

if (IsToastRelation(relation))
saveFreeSpace = ToastGetTargetPageFreeSpace(relation);
else
saveFreeSpace = HeapGetTargetPageFreeSpace(relation);

into macros, as there is a choice only between heap and toast. All other
relation types are not mentioned.

So we can not call it RelationGetTargetPageFreeSpace. It would be
ToastOrHeapGetTargetPageFreeSpace actually. Better not to have such macro.

Please find new version of the patch in the attachment.

The patch does not apply on Head anymore, could you rebase and post a
patch. I'm changing the status to "Waiting for Author".

Regards,
Vignesh

#15

Nikolay Shaplov

dhyan@nataraj.su

over 4 years ago

In reply to: vignesh C (#14)

Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions

В письме от среда, 14 июля 2021 г. 15:09:12 MSK пользователь vignesh C
написал:

The patch does not apply on Head anymore, could you rebase and post a
patch. I'm changing the status to "Waiting for Author".

Thank you for notification.

I've tried to rebase it and found out that some options have been added to
partitioned table.
Handling this needs careful approach, and I will fix it two weeks later, when I
am back from vacations.

Meanwhile I would strongly suggest to change

{"vacuum_index_cleanup", RELOPT_TYPE_BOOL,

{"vacuum_index_cleanup", RELOPT_TYPE_ENUM,

in src/backend/access/common/reloptions.c

This change should be done in 3499df0d
But current implementation of reloptions is very error prone , and it is very
easy to miss this part.

--
Nikolay Shaplov
dhyan@nataraj.su (e-mail, jabber)
@dhyan:nataraj.su (matrix)