Code review for FILLFACTOR patch. Change WITH grammar as per earlier

discussion (including making def_arg allow reserved words), add missed
opt_definition for UNIQUE case.  Put the reloptions support code in a less
random place (I chose to make a new file access/common/reloptions.c).
Eliminate header inclusion creep.  Make the index options functions safely
user-callable (seems like client apps might like to be able to test validity
of options before trying to make an index).  Reduce overhead for normal case
with no options by allowing rd_options to be NULL.  Fix some unmaintainably
klugy code, including getting rid of Natts_pg_class_fixed at long last.
Some stylistic cleanup too, and pay attention to keeping comments in sync
with code.

Documentation still needs work, though I did fix the omissions in
catalogs.sgml and indexam.sgml.

1 files changed, 1 insertions, 61 deletions

diff --git a/src/backend/access/heap/heapam.c b/src/backend/access/heap/heapam.c
index cf3344c2009..cf4b920f7da 100644
--- a/src/backend/access/heap/heapam.c
+++ b/src/backend/access/heap/heapam.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *	  $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.214 2006/07/02 02:23:18 momjian Exp $
+ *	  $PostgreSQL: pgsql/src/backend/access/heap/heapam.c,v 1.215 2006/07/03 22:45:37 tgl Exp $
  *
  *
  * INTERFACE ROUTINES
@@ -46,13 +46,9 @@
 #include "access/xlogutils.h"
 #include "catalog/catalog.h"
 #include "catalog/namespace.h"
-#include "commands/defrem.h"
 #include "miscadmin.h"
-#include "nodes/parsenodes.h"
-#include "parser/parse_clause.h"
 #include "pgstat.h"
 #include "storage/procarray.h"
-#include "utils/catcache.h"
 #include "utils/inval.h"
 #include "utils/relcache.h"
 
@@ -3592,59 +3588,3 @@ heap_desc(StringInfo buf, uint8 xl_info, char *rec)
 	else
 		appendStringInfo(buf, "UNKNOWN");
 }
-
-/*
- * Parse options for heaps.
- *
- *	relkind		Kind of relation
- *	options		Options as text[]
- */
-bytea *
-heap_option(char relkind, ArrayType *options)
-{
-	/*
-	 * XXX: What fillfactor should be default?
-	 * overriding databases:
-	 *   - Oracle, DB2 = 90%
-	 *   - SQL Server = 100%
-	 * non-overriding database:
-	 *   - Firebird = 70%
-	 */
-#define HEAP_MIN_FILLFACTOR			50
-#define HEAP_DEFAULT_FILLFACTOR		100
-
-	int				fillfactor;
-	HeapOption	   *result;
-
-	DefElem	kwds[] =
-	{
-		{ T_DefElem, "fillfactor" },
-	};
-
-	/*
-	 * parse options
-	 */
-	OptionParse(options, lengthof(kwds), kwds, true);
-
-	/* 0: fillfactor */
-	if (kwds[0].arg)
-		fillfactor = (int) defGetInt64(&kwds[0]);
-	else
-		fillfactor = HEAP_DEFAULT_FILLFACTOR;
-	if (fillfactor < HEAP_MIN_FILLFACTOR || 100 < fillfactor)
-	{
-		ereport(ERROR,
-				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-				 errmsg("fillfactor=%d should be between %d and 100",
-                 fillfactor, HEAP_MIN_FILLFACTOR)));
-	}
-
-	/*
-	 * build option
-	 */
-	result = (HeapOption *)
-		MemoryContextAlloc(CacheMemoryContext, sizeof(HeapOption));
-	VARATT_SIZEP(result) = sizeof(HeapOption);
-	result->fillfactor = fillfactor;
-	return (bytea *) result;
-}