Tom Lane wrote:

> I guess I'm still wondering which part of this actually needs to be
> hand-coded so that it can be flexible.  I'm envisioning the whole
> loop replaced by something like
> 
>       FillRelOptions((void *) rdopts, options, &constanttable);
> 
> where the constant table contains entries like
> 
>       { "fillfactor", RELOPT_TYPE_INT, offsetof(StdRdOptions, fillfactor) }

I attach a patch that does things this way (it includes the btree test
code because I'm too lazy right now to strip it out).

I'm not really sure about removing the other macros completely, because
they would be useful whenever one wanted to create something
nonstandard.


> BTW, are we just assuming that there's never a possibility of no match?
> It seems like there ought to be an elog complaint if you get to the
> bottom of the loop; which again is something I don't see the point of
> writing out each time.

We need to be quiet about it when not validating, I think.

-- 
Alvaro Herrera                                http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Index: src/backend/access/common/reloptions.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/common/reloptions.c,v
retrieving revision 1.17
diff -c -p -r1.17 reloptions.c
*** src/backend/access/common/reloptions.c	8 Jan 2009 19:34:41 -0000	1.17
--- src/backend/access/common/reloptions.c	9 Jan 2009 23:40:36 -0000
***************
*** 34,43 ****
   * To add an option:
   *
   * (i) decide on a class (integer, real, bool, string), name, default value,
!  * upper and lower bounds (if applicable).
!  * (ii) add a record below.
!  * (iii) add it to StdRdOptions if appropriate
!  * (iv) add a block to the appropriate handling routine (probably
   * default_reloptions)
   * (v) don't forget to document the option
   *
--- 34,44 ----
   * To add an option:
   *
   * (i) decide on a class (integer, real, bool, string), name, default value,
!  * upper and lower bounds (if applicable); for string types, 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)
   * (v) don't forget to document the option
   *
*************** parse_one_reloption(relopt_value *option
*** 762,767 ****
--- 763,854 ----
  		pfree(value);
  }
  
+ void *
+ allocateReloptStruct(Size base, relopt_value *options, int numoptions)
+ {
+ 	Size	size = base;
+ 	int		i;
+ 
+ 	for (i = 0; i < numoptions; i++)
+ 		if (options[i].gen->type == RELOPT_TYPE_STRING)
+ 			size += GET_STRING_RELOPTION_LEN(options[i]) + 1;
+ 
+ 	return palloc0(size);
+ }
+ 
+ void
+ fillRelOptions(void *rdopts, Size basesize, relopt_value *options,
+ 			   int numoptions, bool validate, reloptParseElem *elems,
+ 			   int numelems)
+ {
+ 	int		i;
+ 	int		offset = basesize;
+ 
+ 	for (i = 0; i < numoptions; i++)
+ 	{
+ 		int		j;
+ 		bool	found = false;
+ 
+ 		for (j = 0; j < numelems; j++)
+ 		{
+ 			if (pg_strcasecmp(options[i].gen->name, elems[j].optname) == 0)
+ 			{
+ 				relopt_string *optstring;
+ 				char   *itempos = ((char *) rdopts) + elems[j].offset;
+ 				char   *string_val;
+ 
+ 				switch (options[i].gen->type)
+ 				{
+ 					case RELOPT_TYPE_BOOL:
+ 						*(bool *) itempos = options[i].isset ?
+ 							options[i].values.bool_val :
+ 							((relopt_bool *) options[i].gen)->default_val;
+ 						break;
+ 					case RELOPT_TYPE_INT:
+ 						*(int *) itempos = options[i].isset ?
+ 							options[i].values.int_val :
+ 							((relopt_int *) options[i].gen)->default_val;
+ 						break;
+ 					case RELOPT_TYPE_REAL:
+ 						*(double *) itempos = options[i].isset ?
+ 							options[i].values.real_val :
+ 							((relopt_real *) options[i].gen)->default_val;
+ 						break;
+ 					case RELOPT_TYPE_STRING:
+ 						optstring = (relopt_string *) options[i].gen;
+ 						if (options[i].isset)
+ 							string_val = options[i].values.string_val;
+ 						else if (!optstring->default_isnull)
+ 							string_val = optstring->default_val;
+ 						else
+ 							string_val = NULL;
+ 
+ 						if (string_val == NULL)
+ 							*(int *) itempos = 0;
+ 						else
+ 						{
+ 							strcpy((char *) rdopts + offset, string_val);
+ 							*(int *) itempos = offset;
+ 							offset += strlen(string_val) + 1;
+ 						}
+ 						break;
+ 					default:
+ 						elog(ERROR, "unrecognized reloption type %c",
+ 							 options[i].gen->type);
+ 						break;
+ 				}
+ 				found = true;
+ 				break;
+ 			}
+ 		}
+ 		if (validate && !found)
+ 			elog(ERROR, "storate parameter \"%s\" not found in parse table",
+ 				 options[i].gen->name);
+ 	}
+ 	SET_VARSIZE(rdopts, offset);
+ }
+ 
+ 
  /*
   * Option parser for anything that uses StdRdOptions (i.e. fillfactor only)
   */
*************** default_reloptions(Datum reloptions, boo
*** 770,779 ****
  {
  	relopt_value   *options;
  	StdRdOptions   *rdopts;
- 	StdRdOptions	lopts;
  	int				numoptions;
! 	int				len;
! 	int				i;
  
  	options = parseRelOptions(reloptions, validate, kind, &numoptions);
  
--- 857,866 ----
  {
  	relopt_value   *options;
  	StdRdOptions   *rdopts;
  	int				numoptions;
! 	reloptParseElem tab[] = {
! 		{"fillfactor", RELOPT_TYPE_INT, offsetof(StdRdOptions, fillfactor)}
! 	};
  
  	options = parseRelOptions(reloptions, validate, kind, &numoptions);
  
*************** default_reloptions(Datum reloptions, boo
*** 781,801 ****
  	if (numoptions == 0)
  		return NULL;
  
! 	MemSet(&lopts, 0, sizeof(StdRdOptions));
  
! 	for (i = 0; i < numoptions; i++)
! 	{
! 		HANDLE_INT_RELOPTION("fillfactor", lopts.fillfactor, options[i],
! 							 (char *) NULL);
! 	}
  
  	pfree(options);
  
- 	len = sizeof(StdRdOptions);
- 	rdopts = palloc(len);
- 	memcpy(rdopts, &lopts, len);
- 	SET_VARSIZE(rdopts, len);
- 
  	return (bytea *) rdopts;
  }
  
--- 868,880 ----
  	if (numoptions == 0)
  		return NULL;
  
! 	rdopts = allocateReloptStruct(sizeof(StdRdOptions), options, numoptions);
  
! 	fillRelOptions((void *) rdopts, sizeof(StdRdOptions), options, numoptions,
! 				   validate, tab, lengthof(tab));
  
  	pfree(options);
  
  	return (bytea *) rdopts;
  }
  
Index: src/backend/access/nbtree/nbtinsert.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/nbtree/nbtinsert.c,v
retrieving revision 1.169
diff -c -p -r1.169 nbtinsert.c
*** src/backend/access/nbtree/nbtinsert.c	1 Jan 2009 17:23:35 -0000	1.169
--- src/backend/access/nbtree/nbtinsert.c	9 Jan 2009 23:41:59 -0000
***************
*** 17,22 ****
--- 17,23 ----
  
  #include "access/heapam.h"
  #include "access/nbtree.h"
+ #include "access/reloptions.h"
  #include "access/transam.h"
  #include "miscadmin.h"
  #include "storage/bufmgr.h"
*************** _bt_insertonpg(Relation rel,
*** 565,570 ****
--- 566,593 ----
  	OffsetNumber firstright = InvalidOffsetNumber;
  	Size		itemsz;
  
+ 	{
+ 		BtOptions *options = (BtOptions *) rel->rd_options;
+ 
+ 		if (options)
+ 		{
+ 			char	*str;
+ 
+ 			str = GET_STRING_RELOPTION(options, teststring);
+ 			elog(LOG, "the teststring option is %s",
+ 				 str ? str : "empty");
+ 			str = GET_STRING_RELOPTION(options, teststring2);
+ 			elog(LOG, "the teststring2 option is %s",
+ 				 str ? str : "empty");
+ 			str = GET_STRING_RELOPTION(options, teststring3);
+ 			elog(LOG, "the teststring3 option is %s",
+ 				 str ? str : "empty");
+ 
+ 			elog(LOG, "the testint option is %d", options->testint);
+ 			elog(LOG, "the testreal option is %lf", options->testreal);
+ 		}
+ 	}
+ 
  	page = BufferGetPage(buf);
  	lpageop = (BTPageOpaque) PageGetSpecialPointer(page);
  
Index: src/backend/access/nbtree/nbtutils.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/nbtree/nbtutils.c,v
retrieving revision 1.93
diff -c -p -r1.93 nbtutils.c
*** src/backend/access/nbtree/nbtutils.c	5 Jan 2009 17:14:28 -0000	1.93
--- src/backend/access/nbtree/nbtutils.c	9 Jan 2009 23:28:10 -0000
*************** BTreeShmemInit(void)
*** 1395,1408 ****
  		Assert(found);
  }
  
  Datum
  btoptions(PG_FUNCTION_ARGS)
  {
  	Datum		reloptions = PG_GETARG_DATUM(0);
  	bool		validate = PG_GETARG_BOOL(1);
  	bytea	   *result;
  
- 	result = default_reloptions(reloptions, validate, RELOPT_KIND_BTREE);
  	if (result)
  		PG_RETURN_BYTEA_P(result);
  	PG_RETURN_NULL();
--- 1395,1465 ----
  		Assert(found);
  }
  
+ static void
+ validate_teststring(char *string, bool validate)
+ {
+ 	if (!validate)
+ 		return;
+ 
+ 	if (string == NULL)
+ 		elog(ERROR, "teststring must not be null!");
+ 	if (strlen(string) != 3)
+ 		elog(ERROR, "teststring must be of length 3, but it's \"%s\"", string);
+ }
+ 
  Datum
  btoptions(PG_FUNCTION_ARGS)
  {
  	Datum		reloptions = PG_GETARG_DATUM(0);
  	bool		validate = PG_GETARG_BOOL(1);
  	bytea	   *result;
+ 	relopt_value *options;
+ 	int			numoptions;
+ 	static	bool initialized = false;
+ 
+ 	if (!initialized)
+ 	{
+ 		add_string_reloption(RELOPT_KIND_BTREE, "teststring", NULL,
+ 							 "foo", validate_teststring);
+ 		add_string_reloption(RELOPT_KIND_BTREE, "teststring2", NULL,
+ 							 "", NULL);
+ 		add_string_reloption(RELOPT_KIND_BTREE, "teststring3", NULL,
+ 							 NULL, NULL);
+ 		add_bool_reloption(RELOPT_KIND_BTREE, "testbool", NULL, false);
+ 		add_real_reloption(RELOPT_KIND_BTREE, "testreal", NULL, 2.78, 3.14, 9.99);
+ 		add_int_reloption(RELOPT_KIND_BTREE, "testint", NULL, 0, 10, 42);
+ 
+ 		initialized = true;
+ 	}
+ 
+ 	options = parseRelOptions(reloptions, validate, RELOPT_KIND_BTREE,
+ 							  &numoptions);
+ 
+ 	/* if none set, we're done */
+ 	if (numoptions == 0)
+ 		result = NULL;
+ 	else
+ 	{
+ 		BtOptions		   *rdopts;
+ 		reloptParseElem		tab[] = {
+ 			{"fillfactor", RELOPT_TYPE_INT, offsetof(BtOptions, fillfactor)},
+ 			{"teststring", RELOPT_TYPE_STRING, offsetof(BtOptions, teststring)},
+ 			{"teststring2", RELOPT_TYPE_STRING, offsetof(BtOptions, teststring2)},
+ 			{"teststring3", RELOPT_TYPE_STRING, offsetof(BtOptions, teststring3)},
+ 			{"testbool", RELOPT_TYPE_BOOL, offsetof(BtOptions, testbool)},
+ 			{"testreal", RELOPT_TYPE_REAL, offsetof(BtOptions, testreal)},
+ 			{"testint", RELOPT_TYPE_INT, offsetof(BtOptions, testint)},
+ 		};
+ 
+ 		rdopts = allocateReloptStruct(sizeof(BtOptions), options, numoptions);
+ 
+ 		fillRelOptions((void *) rdopts, sizeof(BtOptions), options, numoptions,
+ 					   validate, tab, lengthof(tab));
+ 
+ 		pfree(options);
+ 		result = (bytea *) rdopts;
+ 	}
  
  	if (result)
  		PG_RETURN_BYTEA_P(result);
  	PG_RETURN_NULL();
Index: src/include/access/nbtree.h
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/include/access/nbtree.h,v
retrieving revision 1.123
diff -c -p -r1.123 nbtree.h
*** src/include/access/nbtree.h	1 Jan 2009 17:23:56 -0000	1.123
--- src/include/access/nbtree.h	9 Jan 2009 21:48:55 -0000
*************** extern void btree_xlog_startup(void);
*** 595,598 ****
--- 595,612 ----
  extern void btree_xlog_cleanup(void);
  extern bool btree_safe_restartpoint(void);
  
+ /* must follow StdRdOptions conventions */
+ typedef struct BtOptions
+ {
+ 	int32	vl_len_;
+ 	int		fillfactor;
+ 	int		teststring;
+ 	int		teststring2;
+ 	int		teststring3;
+ 	bool	testbool;
+ 	double	testreal;
+ 	int		testint;
+ 	/* variable length struct -- string is allocated here */
+ } BtOptions;
+ 
  #endif   /* NBTREE_H */
Index: src/include/access/reloptions.h
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/include/access/reloptions.h,v
retrieving revision 1.9
diff -c -p -r1.9 reloptions.h
*** src/include/access/reloptions.h	8 Jan 2009 19:34:41 -0000	1.9
--- src/include/access/reloptions.h	9 Jan 2009 23:51:15 -0000
*************** typedef struct relopt_string
*** 106,115 ****
   * default_reloptions for an example of the intended usage.  Beware of
   * multiple evaluation of arguments!
   *
-  * Most of the time there's no need to call HAVE_RELOPTION manually, but it's
-  * possible that an amoptions routine needs to walk the array with a different
-  * purpose (say, to compute the size of a struct to allocate beforehand.)
-  *
   * The last argument in the HANDLE_*_RELOPTION macros allows the caller to
   * determine whether the option was set (true), or its value acquired from
   * defaults (false); it can be passed as (char *) NULL if the caller does not
--- 106,111 ----
*************** typedef struct relopt_string
*** 118,160 ****
  #define HAVE_RELOPTION(optname, option) \
  	(pg_strncasecmp(option.gen->name, optname, option.gen->namelen + 1) == 0)
  
! #define HANDLE_INT_RELOPTION(optname, var, option, wasset)			\
! 	do {															\
! 		if (HAVE_RELOPTION(optname, option))						\
! 		{															\
! 			if (option.isset)										\
! 				var = option.values.int_val; 						\
! 			else													\
! 				var = ((relopt_int *) option.gen)->default_val; 	\
! 			(wasset) != NULL ? *(wasset) = option.isset : (dummyret)NULL; \
! 			continue;												\
! 		}															\
  	} while (0)
  
  #define HANDLE_BOOL_RELOPTION(optname, var, option, wasset)			\
  	do {															\
! 		if (HAVE_RELOPTION(optname, option))						\
! 		{															\
! 			if (option.isset)										\
! 				var = option.values.bool_val; 						\
! 			else													\
! 				var = ((relopt_bool *) option.gen)->default_val;	\
! 			(wasset) != NULL ? *(wasset) = option.isset : (dummyret) NULL; \
! 			continue;												\
! 		}															\
  	} while (0)
  
! #define HANDLE_REAL_RELOPTION(optname, var, option, wasset) 		\
! 	do {															\
! 		if (HAVE_RELOPTION(optname, option))						\
! 		{															\
! 			if (option.isset)										\
! 				var = option.values.real_val; 						\
! 			else													\
! 				var = ((relopt_real *) option.gen)->default_val;	\
! 			(wasset) != NULL ? *(wasset) = option.isset : (dummyret) NULL; \
! 			continue;												\
! 		}															\
  	} while (0)
  
  /*
--- 114,144 ----
  #define HAVE_RELOPTION(optname, option) \
  	(pg_strncasecmp(option.gen->name, optname, option.gen->namelen + 1) == 0)
  
! #define HANDLE_INT_RELOPTION(optname, var, option, wasset)		\
! 	do {														\
! 		if (option.isset)										\
! 			var = option.values.int_val; 						\
! 		else													\
! 			var = ((relopt_int *) option.gen)->default_val; 	\
! 		(wasset) != NULL ? *(wasset) = option.isset : (dummyret)NULL; \
  	} while (0)
  
  #define HANDLE_BOOL_RELOPTION(optname, var, option, wasset)			\
  	do {															\
! 		if (option.isset)										\
! 			var = option.values.bool_val; 						\
! 		else													\
! 			var = ((relopt_bool *) option.gen)->default_val;	\
! 		(wasset) != NULL ? *(wasset) = option.isset : (dummyret) NULL; \
  	} while (0)
  
! #define HANDLE_REAL_RELOPTION(optname, var, option, wasset) 	\
! 	do {														\
! 		if (option.isset)										\
! 			var = option.values.real_val; 						\
! 		else													\
! 			var = ((relopt_real *) option.gen)->default_val;	\
! 		(wasset) != NULL ? *(wasset) = option.isset : (dummyret) NULL; \
  	} while (0)
  
  /*
*************** typedef struct relopt_string
*** 170,196 ****
   */
  #define HANDLE_STRING_RELOPTION(optname, var, option, base, offset, wasset)	\
  	do {														\
! 		if (HAVE_RELOPTION(optname, option))						\
! 		{															\
! 			relopt_string *optstring = (relopt_string *) option.gen;\
! 			char *string_val;										\
! 			if (option.isset)										\
! 				string_val = option.values.string_val;				\
! 			else if (!optstring->default_isnull)					\
! 				string_val = optstring->default_val;				\
! 			else													\
! 				string_val = NULL;									\
! 			(wasset) != NULL ? *(wasset) = option.isset : (dummyret) NULL; \
! 			if (string_val == NULL)									\
! 				var = 0;											\
! 			else													\
! 			{														\
! 				strcpy(((char *)(base)) + (offset), string_val);	\
! 				var = (offset);										\
! 				(offset) += strlen(string_val) + 1;					\
! 			}														\
! 			continue;												\
! 		}															\
  	} while (0)
  
  /*
--- 154,176 ----
   */
  #define HANDLE_STRING_RELOPTION(optname, var, option, base, offset, wasset)	\
  	do {														\
! 		relopt_string *optstring = (relopt_string *) option.gen;\
! 		char *string_val;										\
! 		if (option.isset)										\
! 			string_val = option.values.string_val;				\
! 		else if (!optstring->default_isnull)					\
! 			string_val = optstring->default_val;				\
! 		else													\
! 			string_val = NULL;									\
! 		(wasset) != NULL ? *(wasset) = option.isset : (dummyret) NULL; \
! 		if (string_val == NULL)									\
! 			var = 0;											\
! 		else													\
! 		{														\
! 			strcpy(((char *)(base)) + (offset), string_val);	\
! 			var = (offset);										\
! 			(offset) += strlen(string_val) + 1;					\
! 		}														\
  	} while (0)
  
  /*
*************** extern void add_real_reloption(int kind,
*** 220,231 ****
  				   double default_val, double min_val, double max_val);
  extern void add_string_reloption(int kind, char *name, char *desc,
  					 char *default_val, validate_string_relopt validator);
! 			
  extern Datum transformRelOptions(Datum oldOptions, List *defList,
  					bool ignoreOids, bool isReset);
  extern List *untransformRelOptions(Datum options);
  extern relopt_value *parseRelOptions(Datum options, bool validate,
  				relopt_kind kind, int *numrelopts);
  
  extern bytea *default_reloptions(Datum reloptions, bool validate,
  				   relopt_kind kind);
--- 200,223 ----
  				   double default_val, double min_val, double max_val);
  extern void add_string_reloption(int kind, char *name, char *desc,
  					 char *default_val, validate_string_relopt validator);
! 
! typedef struct
! {
! 	char   *optname;
! 	relopt_type	opttype;
! 	int		offset;
! } reloptParseElem;
! 
  extern Datum transformRelOptions(Datum oldOptions, List *defList,
  					bool ignoreOids, bool isReset);
  extern List *untransformRelOptions(Datum options);
  extern relopt_value *parseRelOptions(Datum options, bool validate,
  				relopt_kind kind, int *numrelopts);
+ extern void *allocateReloptStruct(Size base, relopt_value *options,
+ 					 int numoptions);
+ extern void fillRelOptions(void *rdopts, Size basesize, relopt_value *options,
+ 			   int numoptions, bool validate, reloptParseElem *elems,
+ 			   int nelems);
  
  extern bytea *default_reloptions(Datum reloptions, bool validate,
  				   relopt_kind kind);
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to