generic reloptions improvement
Started by Alvaro Herreraabout 17 years ago28 messages
Hi,
Here's a patch for improving the general reloptions mechanism. What
this patch does is add a table-based option parser. This allows adding
new options very easily, and stops the business of having to pass the
minimum and default fillfactor each time you want the reloptions
processed. (This approach would not scale very well; each new reloption
requires more parameters to default_reloptions, and at the same time we
are forcing external AMs to support fillfactor, which they may very well
do not. For example GIN was already passing useless values
pointlessly.)
I kept StdRdOptions as a fixed struct, which fixes the previous complain
about speed. The new code parses the array and stores the values into
the fixed struct. The only new thing in this area is that
default_reloptions has to walk the returned array of relopt_gen to store
the values in the struct. So in order to add a new option, it is
necessary to patch both the options table (intRelOpts, etc) *and*
default_reloptions. This is a bit ugly but it's the only way I found to
keep both generality and speed.
Right now, external AMs cannot do anything much apart from fillfactor,
but it is very simple to add a routine to register a new "reloption
kind" and another to register options in the table. That and a new
*_reloptions routine (which needs to be registered in pg_am) would allow
an AM to create whatever options it needs.
Note that the only types supported are int, bool, real. We don't
support strings. I don't see this as a problem, but shout if you
disagree. (In the current patch, the bool and real lists are empty.
The autovacuum patch adds items to both.)
The patch to add the autovacuum options on top of this is very light on
reloptions.c but heavy on lots of other places, which is why I'm
submitting this separately.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Attachments:
reloptions-3.patchtext/x-diff; charset=us-asciiDownload
Index: src/backend/access/common/reloptions.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/common/reloptions.c,v
retrieving revision 1.11
diff -c -p -r1.11 reloptions.c
*** src/backend/access/common/reloptions.c 23 Jul 2008 17:29:53 -0000 1.11
--- src/backend/access/common/reloptions.c 19 Dec 2008 21:54:32 -0000
***************
*** 15,20 ****
--- 15,23 ----
#include "postgres.h"
+ #include "access/gist_private.h"
+ #include "access/hash.h"
+ #include "access/nbtree.h"
#include "access/reloptions.h"
#include "catalog/pg_type.h"
#include "commands/defrem.h"
***************
*** 22,29 ****
--- 25,157 ----
#include "utils/array.h"
#include "utils/builtins.h"
#include "utils/guc.h"
+ #include "utils/memutils.h"
#include "utils/rel.h"
+ /*
+ * Contents of pg_class.reloptions
+ *
+ * To add an option:
+ *
+ * (i) decide on a class (integer, real, bool), 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
+ *
+ * Note that we don't handle "oids" in relOpts because it is handled by
+ * interpretOidsOption().
+ */
+
+ static const relopt_bool boolRelOpts[] =
+ {
+ /* list terminator */
+ { { NULL } }
+ };
+
+ static const relopt_int intRelOpts[] =
+ {
+ {
+ {
+ "fillfactor",
+ "Packs table pages only to this percentage",
+ RELOPT_KIND_HEAP
+ },
+ HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100
+ },
+ {
+ {
+ "fillfactor",
+ "Packs btree index pages only to this percentage",
+ RELOPT_KIND_BTREE
+ },
+ BTREE_DEFAULT_FILLFACTOR, BTREE_MIN_FILLFACTOR, 100
+ },
+ {
+ {
+ "fillfactor",
+ "Packs hash index pages only to this percentage",
+ RELOPT_KIND_HASH
+ },
+ HASH_DEFAULT_FILLFACTOR, HASH_MIN_FILLFACTOR, 100
+ },
+ {
+ {
+ "fillfactor",
+ "Packs gist index pages only to this percentage",
+ RELOPT_KIND_GIST
+ },
+ GIST_DEFAULT_FILLFACTOR, GIST_MIN_FILLFACTOR, 100
+ },
+ /* list terminator */
+ { { NULL } }
+ };
+
+ static const relopt_real realRelOpts[] =
+ {
+ /* list terminator */
+ { { NULL } }
+ };
+
+ static relopt_gen *relOpts = NULL;
+
+ static void parse_one_reloption(relopt_gen *option, char *text_str,
+ int text_len, bool validate);
+
+ /*
+ * initialize_reloptions
+ * initialization routine, must be called at backend start
+ *
+ * Initialize the relOpts array and fill each variable's type and name length.
+ */
+ void
+ initialize_reloptions(void)
+ {
+ int i;
+ int j = 0;
+
+ for (i = 0; boolRelOpts[i].gen.name; i++)
+ j++;
+ for (i = 0; intRelOpts[i].gen.name; i++)
+ j++;
+ for (i = 0; realRelOpts[i].gen.name; i++)
+ j++;
+
+ if (relOpts)
+ pfree(relOpts);
+ relOpts = MemoryContextAlloc(TopMemoryContext,
+ (j + 1) * sizeof(relopt_gen));
+
+ j = 0;
+ for (i = 0; boolRelOpts[i].gen.name; i++)
+ {
+ relOpts[j] = boolRelOpts[i].gen;
+ relOpts[j].type = RELOPT_TYPE_BOOL;
+ relOpts[j].namelen = strlen(relOpts[j].name);
+ j++;
+ }
+
+ for (i = 0; intRelOpts[i].gen.name; i++)
+ {
+ relOpts[j] = intRelOpts[i].gen;
+ relOpts[j].type = RELOPT_TYPE_INT;
+ relOpts[j].namelen = strlen(relOpts[j].name);
+ j++;
+ }
+
+ for (i = 0; realRelOpts[i].gen.name; i++)
+ {
+ relOpts[j] = realRelOpts[i].gen;
+ relOpts[j].type = RELOPT_TYPE_REAL;
+ relOpts[j].namelen = strlen(relOpts[j].name);
+ j++;
+ }
+
+ /* add a list terminator */
+ relOpts[j].name = NULL;
+ }
/*
* Transform a relation options list (list of DefElem) into the text array
*************** transformRelOptions(Datum oldOptions, Li
*** 73,81 ****
for (i = 0; i < noldoptions; i++)
{
! text *oldoption = DatumGetTextP(oldoptions[i]);
! char *text_str = VARDATA(oldoption);
! int text_len = VARSIZE(oldoption) - VARHDRSZ;
/* Search for a match in defList */
foreach(cell, defList)
--- 201,208 ----
for (i = 0; i < noldoptions; i++)
{
! char *text_str = TextDatumGetCString(oldoptions[i]);
! int text_len = strlen(text_str);
/* Search for a match in defList */
foreach(cell, defList)
*************** untransformRelOptions(Datum options)
*** 198,332 ****
/*
* Interpret reloptions that are given in text-array format.
*
! * options: array of "keyword=value" strings, as built by transformRelOptions
! * numkeywords: number of legal keywords
! * keywords: the allowed keywords
! * values: output area
! * validate: if true, throw error for unrecognized keywords.
! *
! * The keywords and values arrays must both be of length numkeywords.
! * The values entry corresponding to a keyword is set to a palloc'd string
! * containing the corresponding value, or NULL if the keyword does not appear.
*/
! void
! parseRelOptions(Datum options, int numkeywords, const char *const * keywords,
! char **values, bool validate)
{
! ArrayType *array;
! Datum *optiondatums;
! int noptions;
int i;
! /* Initialize to "all defaulted" */
! MemSet(values, 0, numkeywords * sizeof(char *));
! /* Done if no options */
! if (!PointerIsValid(DatumGetPointer(options)))
! return;
! array = DatumGetArrayTypeP(options);
! Assert(ARR_ELEMTYPE(array) == TEXTOID);
! deconstruct_array(array, TEXTOID, -1, false, 'i',
! &optiondatums, NULL, &noptions);
! for (i = 0; i < noptions; i++)
! {
! text *optiontext = DatumGetTextP(optiondatums[i]);
! char *text_str = VARDATA(optiontext);
! int text_len = VARSIZE(optiontext) - VARHDRSZ;
! int j;
! /* Search for a match in keywords */
! for (j = 0; j < numkeywords; j++)
{
! int kw_len = strlen(keywords[j]);
! if (text_len > kw_len && text_str[kw_len] == '=' &&
! pg_strncasecmp(text_str, keywords[j], kw_len) == 0)
{
! char *value;
! int value_len;
! if (values[j] && validate)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" specified more than once",
! keywords[j])));
! value_len = text_len - kw_len - 1;
! value = (char *) palloc(value_len + 1);
! memcpy(value, text_str + kw_len + 1, value_len);
! value[value_len] = '\0';
! values[j] = value;
! break;
}
- }
- if (j >= numkeywords && validate)
- {
- char *s;
- char *p;
! s = TextDatumGetCString(optiondatums[i]);
! p = strchr(s, '=');
! if (p)
! *p = '\0';
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("unrecognized parameter \"%s\"", s)));
}
}
}
/*
! * Parse reloptions for anything using StdRdOptions (ie, fillfactor only)
*/
bytea *
! default_reloptions(Datum reloptions, bool validate,
! int minFillfactor, int defaultFillfactor)
{
! static const char *const default_keywords[1] = {"fillfactor"};
! char *values[1];
! int fillfactor;
! StdRdOptions *result;
! parseRelOptions(reloptions, 1, default_keywords, values, validate);
! /*
! * If no options, we can just return NULL rather than doing anything.
! * (defaultFillfactor is thus not used, but we require callers to pass it
! * anyway since we would need it if more options were added.)
! */
! if (values[0] == NULL)
return NULL;
! if (!parse_int(values[0], &fillfactor, 0, NULL))
! {
! if (validate)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("fillfactor must be an integer: \"%s\"",
! values[0])));
! return NULL;
! }
! if (fillfactor < minFillfactor || fillfactor > 100)
{
! if (validate)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("fillfactor=%d is out of range (should be between %d and 100)",
! fillfactor, minFillfactor)));
! return NULL;
}
! result = (StdRdOptions *) palloc(sizeof(StdRdOptions));
! SET_VARSIZE(result, sizeof(StdRdOptions));
! result->fillfactor = fillfactor;
! return (bytea *) result;
}
--- 325,553 ----
/*
* Interpret reloptions that are given in text-array format.
*
! * The return value is a relopt_gen * array on which the options actually
! * set in the options array have been marked with isset=true. The
! * length of this array is returned in *numrelopts.
! *
! * options is the array as constructed by transformRelOptions.
! * kind specifies the family of options to be processed.
! *
! * XXX Note that this function is not reentrant -- the return array contains
! * pointers to the global relOpts array which have been modified in place.
! * (It is possible to fix this by copying the entries, but right now there
! * is no point in doing so.)
*/
! relopt_gen **
! parseRelOptions(Datum options, bool validate, relopt_kind kind,
! int *numrelopts)
{
! relopt_gen **reloptions = NULL;
! int maxopts = 0;
! int numoptions = 0;
int i;
! /* Build a list of expected options, based on kind */
! for (i = 0; relOpts[i].name; i++)
! {
! if (relOpts[i].kind == kind)
! {
! /* add this option to the list */
! if (numoptions >= maxopts)
! {
! if (maxopts == 0)
! maxopts = 16;
! else
! maxopts *= 2;
! if (reloptions == NULL)
! reloptions = palloc(maxopts * sizeof(relopt_gen *));
! else
! reloptions = repalloc(reloptions, maxopts * sizeof(relopt_gen *));
! }
! reloptions[numoptions] = &relOpts[i];
! numoptions++;
! }
! }
! /* initialize to all unset */
! for (i = 0; i < numoptions; i++)
! reloptions[i]->isset = false;
! /* Done if no options */
! if (PointerIsValid(DatumGetPointer(options)))
! {
! ArrayType *array;
! Datum *optiondatums;
! int noptions;
! array = DatumGetArrayTypeP(options);
! Assert(ARR_ELEMTYPE(array) == TEXTOID);
! deconstruct_array(array, TEXTOID, -1, false, 'i',
! &optiondatums, NULL, &noptions);
!
! for (i = 0; i < noptions; i++)
{
! text *optiontext = DatumGetTextP(optiondatums[i]);
! char *text_str = VARDATA(optiontext);
! int text_len = VARSIZE(optiontext) - VARHDRSZ;
! int j;
! /* Search for a match in reloptions */
! for (j = 0; j < numoptions; j++)
{
! int kw_len = reloptions[j]->namelen;
! if (text_len > kw_len && text_str[kw_len] == '=' &&
! pg_strncasecmp(text_str, reloptions[j]->name, kw_len) == 0)
! {
! parse_one_reloption(reloptions[j], text_str, text_len, validate);
! break;
! }
}
! if (j >= numoptions && validate)
! {
! char *s;
! char *p;
!
! s = TextDatumGetCString(optiondatums[i]);
! p = strchr(s, '=');
! if (p)
! *p = '\0';
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("unrecognized parameter \"%s\"", s)));
! }
}
}
+
+ *numrelopts = numoptions;
+ return reloptions;
}
+ /*
+ * Subroutine for parseRelOptions, to parse and validate a single option's
+ * value
+ */
+ static void
+ parse_one_reloption(relopt_gen *option, char *text_str, int text_len,
+ bool validate)
+ {
+ char *value;
+ int value_len;
+ bool parsed;
+
+ if (option->isset && validate)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("parameter \"%s\" specified more than once",
+ option->name)));
+
+ value_len = text_len - option->namelen - 1;
+ value = (char *) palloc(value_len + 1);
+ memcpy(value, text_str + option->namelen + 1, value_len);
+ value[value_len] = '\0';
+
+ switch (option->type)
+ {
+ case RELOPT_TYPE_BOOL:
+ {
+ parsed = parse_bool(value,
+ &((relopt_bool *) option)->parsed_val);
+ if (validate && !parsed)
+ ereport(ERROR,
+ (errmsg("invalid value for boolean option \"%s\": %s",
+ option->name, value)));
+ }
+ break;
+ case RELOPT_TYPE_INT:
+ {
+ relopt_int *optint = (relopt_int *) option;
+
+ parsed = parse_int(value, &optint->parsed_val,
+ 0, NULL);
+ if (validate && !parsed)
+ ereport(ERROR,
+ (errmsg("invalid value for integer option \"%s\": %s",
+ option->name, value)));
+ if (validate && (optint->parsed_val < optint->min ||
+ optint->parsed_val > optint->max))
+ ereport(ERROR,
+ (errmsg("value %s out of bounds for option \"%s\"",
+ value, option->name),
+ errdetail("Valid values are between \"%d\" and \"%d\".",
+ optint->min, optint->max)));
+ }
+ break;
+ case RELOPT_TYPE_REAL:
+ {
+ relopt_real *optreal = (relopt_real *) option;
+
+ parsed = parse_real(value,
+ &optreal->parsed_val);
+ if (validate && !parsed)
+ ereport(ERROR,
+ (errmsg("invalid value for floating point option \"%s\": %s",
+ option->name, value)));
+ if (validate && (optreal->parsed_val < optreal->min ||
+ optreal->parsed_val > optreal->max))
+ ereport(ERROR,
+ (errmsg("value %s out of bounds for option \"%s\"",
+ value, option->name),
+ errdetail("Valid values are between \"%f\" and \"%f\".",
+ optreal->min, optreal->max)));
+ }
+ break;
+ }
+
+ if (parsed)
+ option->isset = true;
+ pfree(value);
+ }
/*
! * Option parser for anything that uses StdRdOptions (i.e. fillfactor only)
*/
bytea *
! default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
{
! relopt_gen **options;
! StdRdOptions *rdopts;
! StdRdOptions lopts;
! int numoptions;
! int len;
! int i;
! options = parseRelOptions(reloptions, validate, kind, &numoptions);
! /* if none set, we're done */
! if (numoptions == 0)
return NULL;
! MemSet(&lopts, 0, sizeof(StdRdOptions));
! for (i = 0; i < numoptions; i++)
{
! if (!options[i]->isset)
! continue;
!
! if (pg_strncasecmp(options[i]->name, "fillfactor", options[i]->namelen) == 0)
! {
! lopts.fillfactor = ((relopt_int *) options[i])->parsed_val;
! break;
! }
}
! pfree(options);
! len = offsetof(StdRdOptions, fillfactor) + sizeof(int);
! rdopts = palloc(len);
! memcpy(rdopts, &lopts, len);
! SET_VARSIZE(rdopts, len);
! return (bytea *) rdopts;
}
*************** default_reloptions(Datum reloptions, boo
*** 336,344 ****
bytea *
heap_reloptions(char relkind, Datum reloptions, bool validate)
{
! return default_reloptions(reloptions, validate,
! HEAP_MIN_FILLFACTOR,
! HEAP_DEFAULT_FILLFACTOR);
}
--- 557,563 ----
bytea *
heap_reloptions(char relkind, Datum reloptions, bool validate)
{
! return default_reloptions(reloptions, validate, RELOPT_KIND_HEAP);
}
Index: src/backend/access/gin/ginutil.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/gin/ginutil.c,v
retrieving revision 1.18
diff -c -p -r1.18 ginutil.c
*** src/backend/access/gin/ginutil.c 3 Nov 2008 20:47:48 -0000 1.18
--- src/backend/access/gin/ginutil.c 19 Dec 2008 17:20:40 -0000
*************** extractEntriesSU(GinState *ginstate, Off
*** 313,332 ****
Datum
ginoptions(PG_FUNCTION_ARGS)
{
! Datum reloptions = PG_GETARG_DATUM(0);
! bool validate = PG_GETARG_BOOL(1);
! bytea *result;
! /*
! * It's not clear that fillfactor is useful for GIN, but for the moment
! * we'll accept it anyway. (It won't do anything...)
! */
! #define GIN_MIN_FILLFACTOR 10
! #define GIN_DEFAULT_FILLFACTOR 100
- result = default_reloptions(reloptions, validate,
- GIN_MIN_FILLFACTOR,
- GIN_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 313,324 ----
Datum
ginoptions(PG_FUNCTION_ARGS)
{
! Datum reloptions = PG_GETARG_DATUM(0);
! bool validate = PG_GETARG_BOOL(1);
! bytea *result;
! result = default_reloptions(reloptions, validate, RELOPT_KIND_GIN);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
Index: src/backend/access/gist/gistutil.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/gist/gistutil.c,v
retrieving revision 1.31
diff -c -p -r1.31 gistutil.c
*** src/backend/access/gist/gistutil.c 30 Sep 2008 10:52:10 -0000 1.31
--- src/backend/access/gist/gistutil.c 19 Dec 2008 17:20:40 -0000
*************** gistoptions(PG_FUNCTION_ARGS)
*** 670,678 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate,
! GIST_MIN_FILLFACTOR,
! GIST_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 670,677 ----
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate, RELOPT_KIND_GIST);
!
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
Index: src/backend/access/hash/hashutil.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/hash/hashutil.c,v
retrieving revision 1.57
diff -c -p -r1.57 hashutil.c
*** src/backend/access/hash/hashutil.c 15 Sep 2008 18:43:41 -0000 1.57
--- src/backend/access/hash/hashutil.c 19 Dec 2008 17:20:40 -0000
*************** hashoptions(PG_FUNCTION_ARGS)
*** 224,232 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate,
! HASH_MIN_FILLFACTOR,
! HASH_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 224,231 ----
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate, RELOPT_KIND_HASH);
!
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
Index: src/backend/access/nbtree/nbtsort.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/nbtree/nbtsort.c,v
retrieving revision 1.118
diff -c -p -r1.118 nbtsort.c
*** src/backend/access/nbtree/nbtsort.c 30 Sep 2008 10:52:10 -0000 1.118
--- src/backend/access/nbtree/nbtsort.c 19 Dec 2008 17:20:40 -0000
*************** _bt_pagestate(BTWriteState *wstate, uint
*** 340,346 ****
state->btps_full = (BLCKSZ * (100 - BTREE_NONLEAF_FILLFACTOR) / 100);
else
state->btps_full = RelationGetTargetPageFreeSpace(wstate->index,
! BTREE_DEFAULT_FILLFACTOR);
/* no parent level, yet */
state->btps_next = NULL;
--- 340,346 ----
state->btps_full = (BLCKSZ * (100 - BTREE_NONLEAF_FILLFACTOR) / 100);
else
state->btps_full = RelationGetTargetPageFreeSpace(wstate->index,
! BTREE_DEFAULT_FILLFACTOR);
/* no parent level, yet */
state->btps_next = NULL;
Index: src/backend/access/nbtree/nbtutils.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/nbtree/nbtutils.c,v
retrieving revision 1.91
diff -c -p -r1.91 nbtutils.c
*** src/backend/access/nbtree/nbtutils.c 19 Jun 2008 00:46:03 -0000 1.91
--- src/backend/access/nbtree/nbtutils.c 19 Dec 2008 17:20:40 -0000
*************** btoptions(PG_FUNCTION_ARGS)
*** 1402,1410 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate,
! BTREE_MIN_FILLFACTOR,
! BTREE_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 1402,1408 ----
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();
Index: src/backend/utils/init/postinit.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/init/postinit.c,v
retrieving revision 1.186
diff -c -p -r1.186 postinit.c
*** src/backend/utils/init/postinit.c 23 Sep 2008 09:20:36 -0000 1.186
--- src/backend/utils/init/postinit.c 19 Dec 2008 17:20:40 -0000
***************
*** 19,24 ****
--- 19,25 ----
#include <unistd.h>
#include "access/heapam.h"
+ #include "access/reloptions.h"
#include "access/xact.h"
#include "catalog/catalog.h"
#include "catalog/namespace.h"
*************** InitPostgres(const char *in_dbname, Oid
*** 645,650 ****
--- 646,655 ----
if (!bootstrap)
pgstat_bestart();
+ /* initialize the relation options subsystem */
+ if (!bootstrap)
+ initialize_reloptions();
+
/* close the transaction we started above */
if (!bootstrap)
CommitTransactionCommand();
Index: src/include/access/reloptions.h
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/include/access/reloptions.h,v
retrieving revision 1.5
diff -c -p -r1.5 reloptions.h
*** src/include/access/reloptions.h 1 Jan 2008 19:45:56 -0000 1.5
--- src/include/access/reloptions.h 19 Dec 2008 17:20:40 -0000
***************
*** 20,40 ****
#include "nodes/pg_list.h"
! extern Datum transformRelOptions(Datum oldOptions, List *defList,
! bool ignoreOids, bool isReset);
extern List *untransformRelOptions(Datum options);
!
! extern void parseRelOptions(Datum options, int numkeywords,
! const char *const * keywords,
! char **values, bool validate);
extern bytea *default_reloptions(Datum reloptions, bool validate,
! int minFillfactor, int defaultFillfactor);
!
extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
-
extern bytea *index_reloptions(RegProcedure amoptions, Datum reloptions,
! bool validate);
#endif /* RELOPTIONS_H */
--- 20,92 ----
#include "nodes/pg_list.h"
! /* types supported by reloptions */
! typedef enum relopt_type
! {
! RELOPT_TYPE_BOOL,
! RELOPT_TYPE_INT,
! RELOPT_TYPE_REAL
! } relopt_type;
!
! /* kinds supported by reloptions */
! typedef enum relopt_kind
! {
! RELOPT_KIND_HEAP,
! /* XXX do we need a separate kind for TOAST tables? */
! RELOPT_KIND_BTREE,
! RELOPT_KIND_HASH,
! RELOPT_KIND_GIN,
! RELOPT_KIND_GIST
! } relopt_kind;
!
! /* generic struct to hold shared data */
! typedef struct relopt_gen
! {
! const char *name; /* must be first (used as list termination marker) */
! const char *desc;
! relopt_kind kind;
! int namelen;
! relopt_type type;
! bool isset;
! } relopt_gen;
!
! /* reloptions records for specific variable types */
! typedef struct relopt_bool
! {
! relopt_gen gen;
! bool default_val;
! bool parsed_val;
! } relopt_bool;
!
! typedef struct relopt_int
! {
! relopt_gen gen;
! int default_val;
! int min;
! int max;
! int parsed_val;
! } relopt_int;
!
! typedef struct relopt_real
! {
! relopt_gen gen;
! double default_val;
! double min;
! double max;
! double parsed_val;
! } relopt_real;
+ extern void initialize_reloptions(void);
+ extern Datum transformRelOptions(Datum oldOptions, List *defList,
+ bool ignoreOids, bool isReset);
extern List *untransformRelOptions(Datum options);
! extern relopt_gen **parseRelOptions(Datum options, bool validate,
! relopt_kind kind, int *numrelopts);
extern bytea *default_reloptions(Datum reloptions, bool validate,
! relopt_kind kind);
extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
extern bytea *index_reloptions(RegProcedure amoptions, Datum reloptions,
! bool validate);
#endif /* RELOPTIONS_H */
Re: generic reloptions improvement
A small correction to this patch: this is needed because otherwise the
autovac code to parse the option becomes all tangled; this avoids having
to invent special values for "use the default value", and it also avoid
having the default value stored elsewhere in the code than in the
reloptions table.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Attachments:
reloptions-set.patchtext/x-diff; charset=us-asciiDownload
diff -u src/backend/access/common/reloptions.c src/backend/access/common/reloptions.c
--- src/backend/access/common/reloptions.c 19 Dec 2008 21:54:32 -0000
+++ src/backend/access/common/reloptions.c 20 Dec 2008 03:37:36 -0000
@@ -530,12 +530,12 @@
for (i = 0; i < numoptions; i++)
{
- if (!options[i]->isset)
- continue;
-
if (pg_strncasecmp(options[i]->name, "fillfactor", options[i]->namelen) == 0)
{
- lopts.fillfactor = ((relopt_int *) options[i])->parsed_val;
+ if (options[i]->isset)
+ lopts.fillfactor = ((relopt_int *) options[i])->parsed_val;
+ else
+ lopts.fillfactor = ((relopt_int *) options[i])->default_val;
break;
}
}
Re: generic reloptions improvement
Hi, I have a comment about the generic-reloptions patch.
Alvaro Herrera <alvherre@commandprompt.com> wrote:
Here's a patch for improving the general reloptions mechanism. What
this patch does is add a table-based option parser. This allows adding
new options very easily, and stops the business of having to pass the
minimum and default fillfactor each time you want the reloptions
processed.
You use struct relopt_gen (and its subclasses) for the purpose of
both "definition of options" and "parsed result". But I think
it is cleaner to separete parsed results into another struct
something like:
struct relopt_value
{
const relopt_gen *which;
bool isset;
union
{
bool val_bool;
int val_int;
double val_real;
} types;
};
the ariables 'isset' and 'parsed_val' are not used in definition, AFAICS.
Regards,
---
ITAGAKI Takahiro
NTT Open Source Software Center
Re: generic reloptions improvement
ITAGAKI Takahiro wrote:
Alvaro Herrera <alvherre@commandprompt.com> wrote:
Here's a patch for improving the general reloptions mechanism. What
this patch does is add a table-based option parser. This allows adding
new options very easily, and stops the business of having to pass the
minimum and default fillfactor each time you want the reloptions
processed.You use struct relopt_gen (and its subclasses) for the purpose of
both "definition of options" and "parsed result". But I think
it is cleaner to separete parsed results into another struct
something like:
Thanks for the suggestion -- yes, it is better as you suggest. I think
putting the default on the same struct was just out of laziness at
first, and inertia later.
Here's the next version, which also fixes some particularly embarrasing
bugs.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Attachments:
reloptions-4.patchtext/x-diff; charset=us-asciiDownload
Index: src/backend/access/common/reloptions.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/common/reloptions.c,v
retrieving revision 1.11
diff -c -p -r1.11 reloptions.c
*** src/backend/access/common/reloptions.c 23 Jul 2008 17:29:53 -0000 1.11
--- src/backend/access/common/reloptions.c 22 Dec 2008 16:14:40 -0000
***************
*** 15,20 ****
--- 15,23 ----
#include "postgres.h"
+ #include "access/gist_private.h"
+ #include "access/hash.h"
+ #include "access/nbtree.h"
#include "access/reloptions.h"
#include "catalog/pg_type.h"
#include "commands/defrem.h"
***************
*** 22,29 ****
--- 25,157 ----
#include "utils/array.h"
#include "utils/builtins.h"
#include "utils/guc.h"
+ #include "utils/memutils.h"
#include "utils/rel.h"
+ /*
+ * Contents of pg_class.reloptions
+ *
+ * To add an option:
+ *
+ * (i) decide on a class (integer, real, bool), 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
+ *
+ * Note that we don't handle "oids" in relOpts because it is handled by
+ * interpretOidsOption().
+ */
+
+ static relopt_bool boolRelOpts[] =
+ {
+ /* list terminator */
+ { { NULL } }
+ };
+
+ static relopt_int intRelOpts[] =
+ {
+ {
+ {
+ "fillfactor",
+ "Packs table pages only to this percentage",
+ RELOPT_KIND_HEAP
+ },
+ HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100
+ },
+ {
+ {
+ "fillfactor",
+ "Packs btree index pages only to this percentage",
+ RELOPT_KIND_BTREE
+ },
+ BTREE_DEFAULT_FILLFACTOR, BTREE_MIN_FILLFACTOR, 100
+ },
+ {
+ {
+ "fillfactor",
+ "Packs hash index pages only to this percentage",
+ RELOPT_KIND_HASH
+ },
+ HASH_DEFAULT_FILLFACTOR, HASH_MIN_FILLFACTOR, 100
+ },
+ {
+ {
+ "fillfactor",
+ "Packs gist index pages only to this percentage",
+ RELOPT_KIND_GIST
+ },
+ GIST_DEFAULT_FILLFACTOR, GIST_MIN_FILLFACTOR, 100
+ },
+ /* list terminator */
+ { { NULL } }
+ };
+
+ static relopt_real realRelOpts[] =
+ {
+ /* list terminator */
+ { { NULL } }
+ };
+
+ static relopt_gen **relOpts = NULL;
+
+ static void parse_one_reloption(relopt_value *option, char *text_str,
+ int text_len, bool validate);
+
+ /*
+ * initialize_reloptions
+ * initialization routine, must be called at backend start
+ *
+ * Initialize the relOpts array and fill each variable's type and name length.
+ */
+ void
+ initialize_reloptions(void)
+ {
+ int i;
+ int j = 0;
+
+ for (i = 0; boolRelOpts[i].gen.name; i++)
+ j++;
+ for (i = 0; intRelOpts[i].gen.name; i++)
+ j++;
+ for (i = 0; realRelOpts[i].gen.name; i++)
+ j++;
+
+ if (relOpts)
+ pfree(relOpts);
+ relOpts = MemoryContextAlloc(TopMemoryContext,
+ (j + 1) * sizeof(relopt_gen *));
+
+ j = 0;
+ for (i = 0; boolRelOpts[i].gen.name; i++)
+ {
+ relOpts[j] = &boolRelOpts[i].gen;
+ relOpts[j]->type = RELOPT_TYPE_BOOL;
+ relOpts[j]->namelen = strlen(relOpts[j]->name);
+ j++;
+ }
+
+ for (i = 0; intRelOpts[i].gen.name; i++)
+ {
+ relOpts[j] = &intRelOpts[i].gen;
+ relOpts[j]->type = RELOPT_TYPE_INT;
+ relOpts[j]->namelen = strlen(relOpts[j]->name);
+ j++;
+ }
+
+ for (i = 0; realRelOpts[i].gen.name; i++)
+ {
+ relOpts[j] = &realRelOpts[i].gen;
+ relOpts[j]->type = RELOPT_TYPE_REAL;
+ relOpts[j]->namelen = strlen(relOpts[j]->name);
+ j++;
+ }
+
+ /* add a list terminator */
+ relOpts[j] = NULL;
+ }
/*
* Transform a relation options list (list of DefElem) into the text array
*************** transformRelOptions(Datum oldOptions, Li
*** 73,81 ****
for (i = 0; i < noldoptions; i++)
{
! text *oldoption = DatumGetTextP(oldoptions[i]);
! char *text_str = VARDATA(oldoption);
! int text_len = VARSIZE(oldoption) - VARHDRSZ;
/* Search for a match in defList */
foreach(cell, defList)
--- 201,208 ----
for (i = 0; i < noldoptions; i++)
{
! char *text_str = TextDatumGetCString(oldoptions[i]);
! int text_len = strlen(text_str);
/* Search for a match in defList */
foreach(cell, defList)
*************** untransformRelOptions(Datum options)
*** 198,344 ****
/*
* Interpret reloptions that are given in text-array format.
*
! * options: array of "keyword=value" strings, as built by transformRelOptions
! * numkeywords: number of legal keywords
! * keywords: the allowed keywords
! * values: output area
! * validate: if true, throw error for unrecognized keywords.
! *
! * The keywords and values arrays must both be of length numkeywords.
! * The values entry corresponding to a keyword is set to a palloc'd string
! * containing the corresponding value, or NULL if the keyword does not appear.
*/
! void
! parseRelOptions(Datum options, int numkeywords, const char *const * keywords,
! char **values, bool validate)
{
! ArrayType *array;
! Datum *optiondatums;
! int noptions;
int i;
! /* Initialize to "all defaulted" */
! MemSet(values, 0, numkeywords * sizeof(char *));
! /* Done if no options */
! if (!PointerIsValid(DatumGetPointer(options)))
! return;
! array = DatumGetArrayTypeP(options);
!
! Assert(ARR_ELEMTYPE(array) == TEXTOID);
! deconstruct_array(array, TEXTOID, -1, false, 'i',
! &optiondatums, NULL, &noptions);
! for (i = 0; i < noptions; i++)
{
! text *optiontext = DatumGetTextP(optiondatums[i]);
! char *text_str = VARDATA(optiontext);
! int text_len = VARSIZE(optiontext) - VARHDRSZ;
! int j;
! /* Search for a match in keywords */
! for (j = 0; j < numkeywords; j++)
{
! int kw_len = strlen(keywords[j]);
! if (text_len > kw_len && text_str[kw_len] == '=' &&
! pg_strncasecmp(text_str, keywords[j], kw_len) == 0)
{
! char *value;
! int value_len;
! if (values[j] && validate)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" specified more than once",
! keywords[j])));
! value_len = text_len - kw_len - 1;
! value = (char *) palloc(value_len + 1);
! memcpy(value, text_str + kw_len + 1, value_len);
! value[value_len] = '\0';
! values[j] = value;
! break;
}
- }
- if (j >= numkeywords && validate)
- {
- char *s;
- char *p;
! s = TextDatumGetCString(optiondatums[i]);
! p = strchr(s, '=');
! if (p)
! *p = '\0';
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("unrecognized parameter \"%s\"", s)));
}
}
}
/*
! * Parse reloptions for anything using StdRdOptions (ie, fillfactor only)
*/
bytea *
! default_reloptions(Datum reloptions, bool validate,
! int minFillfactor, int defaultFillfactor)
{
! static const char *const default_keywords[1] = {"fillfactor"};
! char *values[1];
! int fillfactor;
! StdRdOptions *result;
! parseRelOptions(reloptions, 1, default_keywords, values, validate);
! /*
! * If no options, we can just return NULL rather than doing anything.
! * (defaultFillfactor is thus not used, but we require callers to pass it
! * anyway since we would need it if more options were added.)
! */
! if (values[0] == NULL)
return NULL;
! if (!parse_int(values[0], &fillfactor, 0, NULL))
! {
! if (validate)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("fillfactor must be an integer: \"%s\"",
! values[0])));
! return NULL;
! }
! if (fillfactor < minFillfactor || fillfactor > 100)
{
! if (validate)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("fillfactor=%d is out of range (should be between %d and 100)",
! fillfactor, minFillfactor)));
! return NULL;
}
! result = (StdRdOptions *) palloc(sizeof(StdRdOptions));
! SET_VARSIZE(result, sizeof(StdRdOptions));
! result->fillfactor = fillfactor;
! return (bytea *) result;
}
-
/*
* Parse options for heaps (and perhaps someday toast tables).
*/
bytea *
heap_reloptions(char relkind, Datum reloptions, bool validate)
{
! return default_reloptions(reloptions, validate,
! HEAP_MIN_FILLFACTOR,
! HEAP_DEFAULT_FILLFACTOR);
}
--- 325,548 ----
/*
* Interpret reloptions that are given in text-array format.
*
! * The return value is a relopt_value * array on which the options actually
! * set in the options array are marked with isset=true. The length of this
! * array is returned in *numrelopts.
! *
! * options is the array as constructed by transformRelOptions.
! * kind specifies the family of options to be processed.
*/
! relopt_value *
! parseRelOptions(Datum options, bool validate, relopt_kind kind,
! int *numrelopts)
{
! relopt_value *reloptions;
! int numoptions = 0;
int i;
+ int j;
! /* Build a list of expected options, based on kind */
! for (i = 0; relOpts[i]; i++)
! if (relOpts[i]->kind == kind)
! numoptions++;
! reloptions = palloc(numoptions * sizeof(relopt_value));
! for (i = 0, j = 0; relOpts[i]; i++)
! {
! if (relOpts[i]->kind == kind)
! {
! reloptions[j].gen = relOpts[i];
! reloptions[j].isset = false;
! j++;
! }
! }
! /* Done if no options */
! if (PointerIsValid(DatumGetPointer(options)))
{
! ArrayType *array;
! Datum *optiondatums;
! int noptions;
!
! array = DatumGetArrayTypeP(options);
! Assert(ARR_ELEMTYPE(array) == TEXTOID);
!
! deconstruct_array(array, TEXTOID, -1, false, 'i',
! &optiondatums, NULL, &noptions);
!
! for (i = 0; i < noptions; i++)
{
! text *optiontext = DatumGetTextP(optiondatums[i]);
! char *text_str = VARDATA(optiontext);
! int text_len = VARSIZE(optiontext) - VARHDRSZ;
! int j;
! /* Search for a match in reloptions */
! for (j = 0; j < numoptions; j++)
{
! int kw_len = reloptions[j].gen->namelen;
! if (text_len > kw_len && text_str[kw_len] == '=' &&
! pg_strncasecmp(text_str, reloptions[j].gen->name,
! kw_len) == 0)
! {
! parse_one_reloption(&reloptions[j], text_str, text_len,
! validate);
! break;
! }
}
! if (j >= numoptions && validate)
! {
! char *s;
! char *p;
!
! s = TextDatumGetCString(optiondatums[i]);
! p = strchr(s, '=');
! if (p)
! *p = '\0';
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("unrecognized parameter \"%s\"", s)));
! }
}
}
+
+ *numrelopts = numoptions;
+ return reloptions;
}
+ /*
+ * Subroutine for parseRelOptions, to parse and validate a single option's
+ * value
+ */
+ static void
+ parse_one_reloption(relopt_value *option, char *text_str, int text_len,
+ bool validate)
+ {
+ char *value;
+ int value_len;
+ bool parsed;
+
+ if (option->isset && validate)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("parameter \"%s\" specified more than once",
+ option->gen->name)));
+
+ value_len = text_len - option->gen->namelen - 1;
+ value = (char *) palloc(value_len + 1);
+ memcpy(value, text_str + option->gen->namelen + 1, value_len);
+ value[value_len] = '\0';
+
+ switch (option->gen->type)
+ {
+ case RELOPT_TYPE_BOOL:
+ {
+ parsed = parse_bool(value, &option->values.bool_val);
+ if (validate && !parsed)
+ ereport(ERROR,
+ (errmsg("invalid value for boolean option \"%s\": %s",
+ option->gen->name, value)));
+ }
+ break;
+ case RELOPT_TYPE_INT:
+ {
+ relopt_int *optint = (relopt_int *) option->gen;
+
+ parsed = parse_int(value, &option->values.int_val, 0, NULL);
+ if (validate && !parsed)
+ ereport(ERROR,
+ (errmsg("invalid value for integer option \"%s\": %s",
+ option->gen->name, value)));
+ if (validate && (option->values.int_val < optint->min ||
+ option->values.int_val > optint->max))
+ ereport(ERROR,
+ (errmsg("value %s out of bounds for option \"%s\"",
+ value, option->gen->name),
+ errdetail("Valid values are between \"%d\" and \"%d\".",
+ optint->min, optint->max)));
+ }
+ break;
+ case RELOPT_TYPE_REAL:
+ {
+ relopt_real *optreal = (relopt_real *) option->gen;
+
+ parsed = parse_real(value, &option->values.real_val);
+ if (validate && !parsed)
+ ereport(ERROR,
+ (errmsg("invalid value for floating point option \"%s\": %s",
+ option->gen->name, value)));
+ if (validate && (option->values.real_val < optreal->min ||
+ option->values.real_val > optreal->max))
+ ereport(ERROR,
+ (errmsg("value %s out of bounds for option \"%s\"",
+ value, option->gen->name),
+ errdetail("Valid values are between \"%f\" and \"%f\".",
+ optreal->min, optreal->max)));
+ }
+ break;
+ }
+
+ if (parsed)
+ option->isset = true;
+ pfree(value);
+ }
/*
! * Option parser for anything that uses StdRdOptions (i.e. fillfactor only)
*/
bytea *
! default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
{
! relopt_value *options;
! StdRdOptions *rdopts;
! StdRdOptions lopts;
! int numoptions;
! int len;
! int i;
! options = parseRelOptions(reloptions, validate, kind, &numoptions);
! /* if none set, we're done */
! if (numoptions == 0)
return NULL;
! MemSet(&lopts, 0, sizeof(StdRdOptions));
! for (i = 0; i < numoptions; i++)
{
! if (pg_strncasecmp(options[i].gen->name, "fillfactor",
! options[i].gen->namelen) == 0)
! {
! if (options[i].isset)
! lopts.fillfactor = options[i].values.int_val;
! else
! lopts.fillfactor = ((relopt_int *) options[i].gen)->default_val;
! break;
! }
}
! pfree(options);
! len = offsetof(StdRdOptions, fillfactor) + sizeof(int);
! rdopts = palloc(len);
! memcpy(rdopts, &lopts, len);
! SET_VARSIZE(rdopts, len);
! return (bytea *) rdopts;
}
/*
* Parse options for heaps (and perhaps someday toast tables).
*/
bytea *
heap_reloptions(char relkind, Datum reloptions, bool validate)
{
! return default_reloptions(reloptions, validate, RELOPT_KIND_HEAP);
}
Index: src/backend/access/gin/ginutil.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/gin/ginutil.c,v
retrieving revision 1.18
diff -c -p -r1.18 ginutil.c
*** src/backend/access/gin/ginutil.c 3 Nov 2008 20:47:48 -0000 1.18
--- src/backend/access/gin/ginutil.c 22 Dec 2008 16:23:59 -0000
*************** ginoptions(PG_FUNCTION_ARGS)
*** 317,332 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! /*
! * It's not clear that fillfactor is useful for GIN, but for the moment
! * we'll accept it anyway. (It won't do anything...)
! */
! #define GIN_MIN_FILLFACTOR 10
! #define GIN_DEFAULT_FILLFACTOR 100
!
! result = default_reloptions(reloptions, validate,
! GIN_MIN_FILLFACTOR,
! GIN_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 317,323 ----
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate, RELOPT_KIND_GIN);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
Index: src/backend/access/gist/gistutil.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/gist/gistutil.c,v
retrieving revision 1.31
diff -c -p -r1.31 gistutil.c
*** src/backend/access/gist/gistutil.c 30 Sep 2008 10:52:10 -0000 1.31
--- src/backend/access/gist/gistutil.c 19 Dec 2008 23:39:48 -0000
*************** gistoptions(PG_FUNCTION_ARGS)
*** 670,678 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate,
! GIST_MIN_FILLFACTOR,
! GIST_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 670,677 ----
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate, RELOPT_KIND_GIST);
!
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
Index: src/backend/access/hash/hashutil.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/hash/hashutil.c,v
retrieving revision 1.57
diff -c -p -r1.57 hashutil.c
*** src/backend/access/hash/hashutil.c 15 Sep 2008 18:43:41 -0000 1.57
--- src/backend/access/hash/hashutil.c 19 Dec 2008 23:39:48 -0000
*************** hashoptions(PG_FUNCTION_ARGS)
*** 224,232 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate,
! HASH_MIN_FILLFACTOR,
! HASH_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 224,231 ----
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate, RELOPT_KIND_HASH);
!
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
Index: src/backend/access/nbtree/nbtutils.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/nbtree/nbtutils.c,v
retrieving revision 1.91
diff -c -p -r1.91 nbtutils.c
*** src/backend/access/nbtree/nbtutils.c 19 Jun 2008 00:46:03 -0000 1.91
--- src/backend/access/nbtree/nbtutils.c 19 Dec 2008 23:39:48 -0000
*************** btoptions(PG_FUNCTION_ARGS)
*** 1402,1410 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate,
! BTREE_MIN_FILLFACTOR,
! BTREE_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 1402,1408 ----
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();
Index: src/backend/utils/init/postinit.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/init/postinit.c,v
retrieving revision 1.186
diff -c -p -r1.186 postinit.c
*** src/backend/utils/init/postinit.c 23 Sep 2008 09:20:36 -0000 1.186
--- src/backend/utils/init/postinit.c 19 Dec 2008 23:39:48 -0000
***************
*** 19,24 ****
--- 19,25 ----
#include <unistd.h>
#include "access/heapam.h"
+ #include "access/reloptions.h"
#include "access/xact.h"
#include "catalog/catalog.h"
#include "catalog/namespace.h"
*************** InitPostgres(const char *in_dbname, Oid
*** 645,650 ****
--- 646,655 ----
if (!bootstrap)
pgstat_bestart();
+ /* initialize the relation options subsystem */
+ if (!bootstrap)
+ initialize_reloptions();
+
/* close the transaction we started above */
if (!bootstrap)
CommitTransactionCommand();
Index: src/include/access/reloptions.h
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/include/access/reloptions.h,v
retrieving revision 1.5
diff -c -p -r1.5 reloptions.h
*** src/include/access/reloptions.h 1 Jan 2008 19:45:56 -0000 1.5
--- src/include/access/reloptions.h 22 Dec 2008 16:05:16 -0000
***************
*** 20,40 ****
#include "nodes/pg_list.h"
! extern Datum transformRelOptions(Datum oldOptions, List *defList,
! bool ignoreOids, bool isReset);
extern List *untransformRelOptions(Datum options);
!
! extern void parseRelOptions(Datum options, int numkeywords,
! const char *const * keywords,
! char **values, bool validate);
extern bytea *default_reloptions(Datum reloptions, bool validate,
! int minFillfactor, int defaultFillfactor);
!
extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
-
extern bytea *index_reloptions(RegProcedure amoptions, Datum reloptions,
! bool validate);
#endif /* RELOPTIONS_H */
--- 20,101 ----
#include "nodes/pg_list.h"
! /* types supported by reloptions */
! typedef enum relopt_type
! {
! RELOPT_TYPE_BOOL,
! RELOPT_TYPE_INT,
! RELOPT_TYPE_REAL
! } relopt_type;
!
! /* kinds supported by reloptions */
! typedef enum relopt_kind
! {
! RELOPT_KIND_HEAP,
! /* XXX do we need a separate kind for TOAST tables? */
! RELOPT_KIND_BTREE,
! RELOPT_KIND_HASH,
! RELOPT_KIND_GIN,
! RELOPT_KIND_GIST
! } relopt_kind;
!
! /* generic struct to hold shared data */
! typedef struct relopt_gen
! {
! const char *name; /* must be first (used as list termination marker) */
! const char *desc;
! relopt_kind kind;
! int namelen;
! relopt_type type;
! } relopt_gen;
!
! /* holds a parsed value */
! typedef struct relopt_value
! {
! relopt_gen *gen;
! bool isset;
! union
! {
! bool bool_val;
! int int_val;
! double real_val;
! } values;
! } relopt_value;
!
! /* reloptions records for specific variable types */
! typedef struct relopt_bool
! {
! relopt_gen gen;
! bool default_val;
! } relopt_bool;
!
! typedef struct relopt_int
! {
! relopt_gen gen;
! int default_val;
! int min;
! int max;
! } relopt_int;
!
! typedef struct relopt_real
! {
! relopt_gen gen;
! double default_val;
! double min;
! double max;
! } relopt_real;
+ extern void initialize_reloptions(void);
+ 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);
extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
extern bytea *index_reloptions(RegProcedure amoptions, Datum reloptions,
! bool validate);
#endif /* RELOPTIONS_H */
Re: generic reloptions improvement
On Monday 22 December 2008 18:24:53 Alvaro Herrera wrote:
Alvaro Herrera <alvherre@commandprompt.com> wrote:
Here's a patch for improving the general reloptions mechanism. What
this patch does is add a table-based option parser. This allows adding
new options very easily, and stops the business of having to pass the
minimum and default fillfactor each time you want the reloptions
processed.
Here's the next version, which also fixes some particularly embarrasing
bugs.
I'm not sure how important this is, but if you are enumerating the access
methods (RELOPT_KIND_BTREE, etc.), how will this work with user-defined
access methods?
Re: generic reloptions improvement
Peter Eisentraut wrote:
I'm not sure how important this is, but if you are enumerating the access
methods (RELOPT_KIND_BTREE, etc.), how will this work with user-defined
access methods?
It is important.
I'm intending to have a new routine which would reserve a value at
runtime. This value would be later be passed by the AM to create new
options on the table.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Re: generic reloptions improvement
Alvaro Herrera <alvherre@commandprompt.com> writes:
Peter Eisentraut wrote:
I'm not sure how important this is, but if you are enumerating the access
methods (RELOPT_KIND_BTREE, etc.), how will this work with user-defined
access methods?
It is important.
I'm intending to have a new routine which would reserve a value at
runtime. This value would be later be passed by the AM to create new
options on the table.
What do you mean by "at runtime"? Surely the value would have to remain
stable across database restarts, since it's going to relate to stuff
that is in catalog entries.
I'd feel more comfortable with a scheme that used the AM's pg_am OID.
regards, tom lane
Re: generic reloptions improvement
Tom Lane wrote:
Alvaro Herrera <alvherre@commandprompt.com> writes:
I'm intending to have a new routine which would reserve a value at
runtime. This value would be later be passed by the AM to create new
options on the table.What do you mean by "at runtime"? Surely the value would have to remain
stable across database restarts, since it's going to relate to stuff
that is in catalog entries.
No, there's no need for the value to be stable across restart; what's
stored in catalogs is the option name, which is linked to the kind
number only in the parser table.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Re: generic reloptions improvement
Alvaro Herrera wrote:
Tom Lane wrote:
Alvaro Herrera <alvherre@commandprompt.com> writes:
I'm intending to have a new routine which would reserve a value at
runtime. This value would be later be passed by the AM to create new
options on the table.What do you mean by "at runtime"? Surely the value would have to remain
stable across database restarts, since it's going to relate to stuff
that is in catalog entries.No, there's no need for the value to be stable across restart; what's
stored in catalogs is the option name, which is linked to the kind
number only in the parser table.
So this is an updated patch. This now allows a user-defined AM to
create new reloptions and pass them down to the parser for parsing and
checking. I also attach a proof-of-concept patch that adds three new
options to btree (which do nothing apart from logging a message at
insert time). This patch demonstrates the coding pattern that a
user-defined AM should follow to add and use new storage options.
The main thing I find slightly hateful about this patch is that the code
to translate from the returned relopt_value array and the fixed struct
is rather verbose; and that the AM needs to duplicate the code in
default_reloptions. I don't find it ugly enough to warrant objecting to
the patch as a whole however.
The neat thing about this code is that the parsing and searching is done
only once, when the relcache entry is loaded. Later accesses to the
option values themselves is just a struct access, and thus plenty quick.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Attachments:
reloptions-6.patchtext/x-diff; charset=us-asciiDownload
Index: src/backend/access/common/reloptions.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/common/reloptions.c,v
retrieving revision 1.11
diff -c -p -r1.11 reloptions.c
*** src/backend/access/common/reloptions.c 23 Jul 2008 17:29:53 -0000 1.11
--- src/backend/access/common/reloptions.c 30 Dec 2008 14:33:15 -0000
***************
*** 15,20 ****
--- 15,23 ----
#include "postgres.h"
+ #include "access/gist_private.h"
+ #include "access/hash.h"
+ #include "access/nbtree.h"
#include "access/reloptions.h"
#include "catalog/pg_type.h"
#include "commands/defrem.h"
***************
*** 22,29 ****
--- 25,307 ----
#include "utils/array.h"
#include "utils/builtins.h"
#include "utils/guc.h"
+ #include "utils/memutils.h"
#include "utils/rel.h"
+ /*
+ * Contents of pg_class.reloptions
+ *
+ * To add an option:
+ *
+ * (i) decide on a class (integer, real, bool), 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
+ *
+ * Note that we don't handle "oids" in relOpts because it is handled by
+ * interpretOidsOption().
+ */
+
+ static relopt_bool boolRelOpts[] =
+ {
+ /* list terminator */
+ { { NULL } }
+ };
+
+ static relopt_int intRelOpts[] =
+ {
+ {
+ {
+ "fillfactor",
+ "Packs table pages only to this percentage",
+ RELOPT_KIND_HEAP
+ },
+ HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100
+ },
+ {
+ {
+ "fillfactor",
+ "Packs btree index pages only to this percentage",
+ RELOPT_KIND_BTREE
+ },
+ BTREE_DEFAULT_FILLFACTOR, BTREE_MIN_FILLFACTOR, 100
+ },
+ {
+ {
+ "fillfactor",
+ "Packs hash index pages only to this percentage",
+ RELOPT_KIND_HASH
+ },
+ HASH_DEFAULT_FILLFACTOR, HASH_MIN_FILLFACTOR, 100
+ },
+ {
+ {
+ "fillfactor",
+ "Packs gist index pages only to this percentage",
+ RELOPT_KIND_GIST
+ },
+ GIST_DEFAULT_FILLFACTOR, GIST_MIN_FILLFACTOR, 100
+ },
+ /* list terminator */
+ { { NULL } }
+ };
+
+ static relopt_real realRelOpts[] =
+ {
+ /* list terminator */
+ { { NULL } }
+ };
+
+ static relopt_gen **relOpts = NULL;
+ static int last_assigned_kind = RELOPT_KIND_LAST_DEFAULT + 1;
+
+ static int num_custom_options = 0;
+ static relopt_gen **custom_options = NULL;
+
+ static void parse_one_reloption(relopt_value *option, char *text_str,
+ int text_len, bool validate);
+
+ /*
+ * initialize_reloptions
+ * initialization routine, must be called at backend start
+ *
+ * Initialize the relOpts array and fill each variable's type and name length.
+ */
+ void
+ initialize_reloptions(void)
+ {
+ int i;
+ int j = 0;
+
+ for (i = 0; boolRelOpts[i].gen.name; i++)
+ j++;
+ for (i = 0; intRelOpts[i].gen.name; i++)
+ j++;
+ for (i = 0; realRelOpts[i].gen.name; i++)
+ j++;
+ j += num_custom_options;
+
+ if (relOpts)
+ pfree(relOpts);
+ relOpts = MemoryContextAlloc(TopMemoryContext,
+ (j + 1) * sizeof(relopt_gen *));
+
+ j = 0;
+ for (i = 0; boolRelOpts[i].gen.name; i++)
+ {
+ relOpts[j] = &boolRelOpts[i].gen;
+ relOpts[j]->type = RELOPT_TYPE_BOOL;
+ relOpts[j]->namelen = strlen(relOpts[j]->name);
+ j++;
+ }
+
+ for (i = 0; intRelOpts[i].gen.name; i++)
+ {
+ relOpts[j] = &intRelOpts[i].gen;
+ relOpts[j]->type = RELOPT_TYPE_INT;
+ relOpts[j]->namelen = strlen(relOpts[j]->name);
+ j++;
+ }
+
+ for (i = 0; realRelOpts[i].gen.name; i++)
+ {
+ relOpts[j] = &realRelOpts[i].gen;
+ relOpts[j]->type = RELOPT_TYPE_REAL;
+ relOpts[j]->namelen = strlen(relOpts[j]->name);
+ j++;
+ }
+
+ for (i = 0; i < num_custom_options; i++)
+ {
+ relOpts[j] = custom_options[i];
+ j++;
+ }
+
+ /* add a list terminator */
+ relOpts[j] = NULL;
+ }
+
+ /*
+ * add_reloption_kind
+ * Create a new relopt_kind value, to be used in custom reloptions by
+ * user-defined AMs.
+ */
+ int
+ add_reloption_kind(void)
+ {
+ return last_assigned_kind++;
+ }
+
+ /*
+ * add_reloption
+ * Add an already-created custom reloption to the list, and recompute the
+ * main parser table.
+ */
+ static void
+ add_reloption(relopt_gen *newoption)
+ {
+ static int max_custom_options = 0;
+
+ if (num_custom_options >= max_custom_options)
+ {
+ MemoryContext oldcxt;
+
+ oldcxt = MemoryContextSwitchTo(TopMemoryContext);
+
+ if (max_custom_options == 0)
+ {
+ max_custom_options = 8;
+ custom_options = palloc(max_custom_options * sizeof(relopt_gen *));
+ }
+ else
+ {
+ max_custom_options *= 2;
+ custom_options = repalloc(custom_options, max_custom_options * sizeof(relopt_gen *));
+ }
+ MemoryContextSwitchTo(oldcxt);
+ }
+ custom_options[num_custom_options++] = newoption;
+
+ initialize_reloptions();
+ }
+
+ /*
+ * allocate_reloption
+ * Allocate a new reloption and initialize the type-agnostic fields
+ */
+ static relopt_gen *
+ allocate_reloption(int kind, int type, char *name, char *desc)
+ {
+ MemoryContext oldcxt;
+ size_t size;
+ relopt_gen *newoption;
+
+ oldcxt = MemoryContextSwitchTo(TopMemoryContext);
+
+ switch (type)
+ {
+ case RELOPT_TYPE_BOOL:
+ size = sizeof(relopt_bool);
+ break;
+ case RELOPT_TYPE_INT:
+ size = sizeof(relopt_int);
+ break;
+ case RELOPT_TYPE_REAL:
+ size = sizeof(relopt_real);
+ break;
+ default:
+ elog(ERROR, "unsupported option type");
+ }
+
+ newoption = palloc(size);
+
+ newoption->name = pstrdup(name);
+ if (desc)
+ newoption->desc = pstrdup(desc);
+ else
+ newoption->desc = NULL;
+ newoption->kind = kind;
+ newoption->namelen = strlen(name);
+ newoption->type = type;
+
+ MemoryContextSwitchTo(oldcxt);
+
+ return newoption;
+ }
+
+ /*
+ * add_bool_reloption
+ * Add a new boolean reloption
+ */
+ void
+ add_bool_reloption(int kind, char *name, char *desc, bool default_val)
+ {
+ relopt_bool *newoption;
+
+ newoption = (relopt_bool *) allocate_reloption(kind, RELOPT_TYPE_BOOL, name, desc);
+ newoption->default_val = default_val;
+
+ add_reloption((relopt_gen *) newoption);
+ }
+
+ /*
+ * add_int_reloption
+ * Add a new integer reloption
+ */
+ void
+ add_int_reloption(int kind, char *name, char *desc, int default_val,
+ int min_val, int max_val)
+ {
+ relopt_int *newoption;
+
+ newoption = (relopt_int *) allocate_reloption(kind, RELOPT_TYPE_INT, name, desc);
+ newoption->default_val = default_val;
+ newoption->min = min_val;
+ newoption->max = max_val;
+
+ add_reloption((relopt_gen *) newoption);
+ }
+
+ /*
+ * add_real_reloption
+ * Add a new float reloption
+ */
+ void
+ add_real_reloption(int kind, char *name, char *desc, double default_val,
+ double min_val, double max_val)
+ {
+ relopt_real *newoption;
+
+ newoption = (relopt_real *) allocate_reloption(kind, RELOPT_TYPE_REAL, name, desc);
+ newoption->default_val = default_val;
+ newoption->min = min_val;
+ newoption->max = max_val;
+
+ add_reloption((relopt_gen *) newoption);
+ }
/*
* Transform a relation options list (list of DefElem) into the text array
*************** untransformRelOptions(Datum options)
*** 198,344 ****
/*
* Interpret reloptions that are given in text-array format.
*
! * options: array of "keyword=value" strings, as built by transformRelOptions
! * numkeywords: number of legal keywords
! * keywords: the allowed keywords
! * values: output area
! * validate: if true, throw error for unrecognized keywords.
! *
! * The keywords and values arrays must both be of length numkeywords.
! * The values entry corresponding to a keyword is set to a palloc'd string
! * containing the corresponding value, or NULL if the keyword does not appear.
*/
! void
! parseRelOptions(Datum options, int numkeywords, const char *const * keywords,
! char **values, bool validate)
{
! ArrayType *array;
! Datum *optiondatums;
! int noptions;
int i;
! /* Initialize to "all defaulted" */
! MemSet(values, 0, numkeywords * sizeof(char *));
! /* Done if no options */
! if (!PointerIsValid(DatumGetPointer(options)))
! return;
! array = DatumGetArrayTypeP(options);
! Assert(ARR_ELEMTYPE(array) == TEXTOID);
!
! deconstruct_array(array, TEXTOID, -1, false, 'i',
! &optiondatums, NULL, &noptions);
! for (i = 0; i < noptions; i++)
{
! text *optiontext = DatumGetTextP(optiondatums[i]);
! char *text_str = VARDATA(optiontext);
! int text_len = VARSIZE(optiontext) - VARHDRSZ;
! int j;
! /* Search for a match in keywords */
! for (j = 0; j < numkeywords; j++)
{
! int kw_len = strlen(keywords[j]);
! if (text_len > kw_len && text_str[kw_len] == '=' &&
! pg_strncasecmp(text_str, keywords[j], kw_len) == 0)
{
! char *value;
! int value_len;
! if (values[j] && validate)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" specified more than once",
! keywords[j])));
! value_len = text_len - kw_len - 1;
! value = (char *) palloc(value_len + 1);
! memcpy(value, text_str + kw_len + 1, value_len);
! value[value_len] = '\0';
! values[j] = value;
! break;
}
- }
- if (j >= numkeywords && validate)
- {
- char *s;
- char *p;
! s = TextDatumGetCString(optiondatums[i]);
! p = strchr(s, '=');
! if (p)
! *p = '\0';
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("unrecognized parameter \"%s\"", s)));
}
}
}
/*
! * Parse reloptions for anything using StdRdOptions (ie, fillfactor only)
*/
bytea *
! default_reloptions(Datum reloptions, bool validate,
! int minFillfactor, int defaultFillfactor)
{
! static const char *const default_keywords[1] = {"fillfactor"};
! char *values[1];
! int fillfactor;
! StdRdOptions *result;
! parseRelOptions(reloptions, 1, default_keywords, values, validate);
! /*
! * If no options, we can just return NULL rather than doing anything.
! * (defaultFillfactor is thus not used, but we require callers to pass it
! * anyway since we would need it if more options were added.)
! */
! if (values[0] == NULL)
return NULL;
! if (!parse_int(values[0], &fillfactor, 0, NULL))
! {
! if (validate)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("fillfactor must be an integer: \"%s\"",
! values[0])));
! return NULL;
! }
! if (fillfactor < minFillfactor || fillfactor > 100)
{
! if (validate)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("fillfactor=%d is out of range (should be between %d and 100)",
! fillfactor, minFillfactor)));
! return NULL;
}
! result = (StdRdOptions *) palloc(sizeof(StdRdOptions));
! SET_VARSIZE(result, sizeof(StdRdOptions));
! result->fillfactor = fillfactor;
! return (bytea *) result;
}
-
/*
* Parse options for heaps (and perhaps someday toast tables).
*/
bytea *
heap_reloptions(char relkind, Datum reloptions, bool validate)
{
! return default_reloptions(reloptions, validate,
! HEAP_MIN_FILLFACTOR,
! HEAP_DEFAULT_FILLFACTOR);
}
--- 476,700 ----
/*
* Interpret reloptions that are given in text-array format.
*
! * The return value is a relopt_value * array on which the options actually
! * set in the options array are marked with isset=true. The length of this
! * array is returned in *numrelopts. Options not set are also present in the
! * array; this is so that the caller can easily locate the default values.
! *
! * options is the array as constructed by transformRelOptions.
! * kind specifies the family of options to be processed.
*/
! relopt_value *
! parseRelOptions(Datum options, bool validate, relopt_kind kind,
! int *numrelopts)
{
! relopt_value *reloptions;
! int numoptions = 0;
int i;
+ int j;
! /* Build a list of expected options, based on kind */
! for (i = 0; relOpts[i]; i++)
! if (relOpts[i]->kind == kind)
! numoptions++;
! reloptions = palloc(numoptions * sizeof(relopt_value));
! for (i = 0, j = 0; relOpts[i]; i++)
! {
! if (relOpts[i]->kind == kind)
! {
! reloptions[j].gen = relOpts[i];
! reloptions[j].isset = false;
! j++;
! }
! }
! /* Done if no options */
! if (PointerIsValid(DatumGetPointer(options)))
{
! ArrayType *array;
! Datum *optiondatums;
! int noptions;
!
! array = DatumGetArrayTypeP(options);
!
! Assert(ARR_ELEMTYPE(array) == TEXTOID);
!
! deconstruct_array(array, TEXTOID, -1, false, 'i',
! &optiondatums, NULL, &noptions);
! for (i = 0; i < noptions; i++)
{
! text *optiontext = DatumGetTextP(optiondatums[i]);
! char *text_str = VARDATA(optiontext);
! int text_len = VARSIZE(optiontext) - VARHDRSZ;
! int j;
! /* Search for a match in reloptions */
! for (j = 0; j < numoptions; j++)
{
! int kw_len = reloptions[j].gen->namelen;
! if (text_len > kw_len && text_str[kw_len] == '=' &&
! pg_strncasecmp(text_str, reloptions[j].gen->name,
! kw_len) == 0)
! {
! parse_one_reloption(&reloptions[j], text_str, text_len,
! validate);
! break;
! }
}
! if (j >= numoptions && validate)
! {
! char *s;
! char *p;
!
! s = TextDatumGetCString(optiondatums[i]);
! p = strchr(s, '=');
! if (p)
! *p = '\0';
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("unrecognized parameter \"%s\"", s)));
! }
}
}
+
+ *numrelopts = numoptions;
+ return reloptions;
}
+ /*
+ * Subroutine for parseRelOptions, to parse and validate a single option's
+ * value
+ */
+ static void
+ parse_one_reloption(relopt_value *option, char *text_str, int text_len,
+ bool validate)
+ {
+ char *value;
+ int value_len;
+ bool parsed;
+
+ if (option->isset && validate)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("parameter \"%s\" specified more than once",
+ option->gen->name)));
+
+ value_len = text_len - option->gen->namelen - 1;
+ value = (char *) palloc(value_len + 1);
+ memcpy(value, text_str + option->gen->namelen + 1, value_len);
+ value[value_len] = '\0';
+
+ switch (option->gen->type)
+ {
+ case RELOPT_TYPE_BOOL:
+ {
+ parsed = parse_bool(value, &option->values.bool_val);
+ if (validate && !parsed)
+ ereport(ERROR,
+ (errmsg("invalid value for boolean option \"%s\": %s",
+ option->gen->name, value)));
+ }
+ break;
+ case RELOPT_TYPE_INT:
+ {
+ relopt_int *optint = (relopt_int *) option->gen;
+
+ parsed = parse_int(value, &option->values.int_val, 0, NULL);
+ if (validate && !parsed)
+ ereport(ERROR,
+ (errmsg("invalid value for integer option \"%s\": %s",
+ option->gen->name, value)));
+ if (validate && (option->values.int_val < optint->min ||
+ option->values.int_val > optint->max))
+ ereport(ERROR,
+ (errmsg("value %s out of bounds for option \"%s\"",
+ value, option->gen->name),
+ errdetail("Valid values are between \"%d\" and \"%d\".",
+ optint->min, optint->max)));
+ }
+ break;
+ case RELOPT_TYPE_REAL:
+ {
+ relopt_real *optreal = (relopt_real *) option->gen;
+
+ parsed = parse_real(value, &option->values.real_val);
+ if (validate && !parsed)
+ ereport(ERROR,
+ (errmsg("invalid value for floating point option \"%s\": %s",
+ option->gen->name, value)));
+ if (validate && (option->values.real_val < optreal->min ||
+ option->values.real_val > optreal->max))
+ ereport(ERROR,
+ (errmsg("value %s out of bounds for option \"%s\"",
+ value, option->gen->name),
+ errdetail("Valid values are between \"%f\" and \"%f\".",
+ optreal->min, optreal->max)));
+ }
+ break;
+ }
+
+ if (parsed)
+ option->isset = true;
+ pfree(value);
+ }
/*
! * Option parser for anything that uses StdRdOptions (i.e. fillfactor only)
*/
bytea *
! default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
{
! relopt_value *options;
! StdRdOptions *rdopts;
! StdRdOptions lopts;
! int numoptions;
! int len;
! int i;
! options = parseRelOptions(reloptions, validate, kind, &numoptions);
! /* if none set, we're done */
! if (numoptions == 0)
return NULL;
! MemSet(&lopts, 0, sizeof(StdRdOptions));
! for (i = 0; i < numoptions; i++)
{
! if (pg_strncasecmp(options[i].gen->name, "fillfactor",
! options[i].gen->namelen) == 0)
! {
! if (options[i].isset)
! lopts.fillfactor = options[i].values.int_val;
! else
! lopts.fillfactor = ((relopt_int *) options[i].gen)->default_val;
! continue;
! }
}
! pfree(options);
! len = offsetof(StdRdOptions, fillfactor) + sizeof(int);
! rdopts = palloc(len);
! memcpy(rdopts, &lopts, len);
! SET_VARSIZE(rdopts, len);
! return (bytea *) rdopts;
}
/*
* Parse options for heaps (and perhaps someday toast tables).
*/
bytea *
heap_reloptions(char relkind, Datum reloptions, bool validate)
{
! return default_reloptions(reloptions, validate, RELOPT_KIND_HEAP);
}
Index: src/backend/access/gin/ginutil.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/gin/ginutil.c,v
retrieving revision 1.18
diff -c -p -r1.18 ginutil.c
*** src/backend/access/gin/ginutil.c 3 Nov 2008 20:47:48 -0000 1.18
--- src/backend/access/gin/ginutil.c 22 Dec 2008 16:23:59 -0000
*************** ginoptions(PG_FUNCTION_ARGS)
*** 317,332 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! /*
! * It's not clear that fillfactor is useful for GIN, but for the moment
! * we'll accept it anyway. (It won't do anything...)
! */
! #define GIN_MIN_FILLFACTOR 10
! #define GIN_DEFAULT_FILLFACTOR 100
!
! result = default_reloptions(reloptions, validate,
! GIN_MIN_FILLFACTOR,
! GIN_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 317,323 ----
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate, RELOPT_KIND_GIN);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
Index: src/backend/access/gist/gistutil.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/gist/gistutil.c,v
retrieving revision 1.31
diff -c -p -r1.31 gistutil.c
*** src/backend/access/gist/gistutil.c 30 Sep 2008 10:52:10 -0000 1.31
--- src/backend/access/gist/gistutil.c 19 Dec 2008 23:39:48 -0000
*************** gistoptions(PG_FUNCTION_ARGS)
*** 670,678 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate,
! GIST_MIN_FILLFACTOR,
! GIST_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 670,677 ----
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate, RELOPT_KIND_GIST);
!
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
Index: src/backend/access/hash/hashutil.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/hash/hashutil.c,v
retrieving revision 1.57
diff -c -p -r1.57 hashutil.c
*** src/backend/access/hash/hashutil.c 15 Sep 2008 18:43:41 -0000 1.57
--- src/backend/access/hash/hashutil.c 19 Dec 2008 23:39:48 -0000
*************** hashoptions(PG_FUNCTION_ARGS)
*** 224,232 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate,
! HASH_MIN_FILLFACTOR,
! HASH_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 224,231 ----
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate, RELOPT_KIND_HASH);
!
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
Index: src/backend/access/nbtree/nbtutils.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/access/nbtree/nbtutils.c,v
retrieving revision 1.91
diff -c -p -r1.91 nbtutils.c
*** src/backend/access/nbtree/nbtutils.c 19 Jun 2008 00:46:03 -0000 1.91
--- src/backend/access/nbtree/nbtutils.c 30 Dec 2008 14:45:56 -0000
*************** btoptions(PG_FUNCTION_ARGS)
*** 1402,1410 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate,
! BTREE_MIN_FILLFACTOR,
! BTREE_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 1402,1409 ----
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();
Index: src/backend/utils/init/postinit.c
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/backend/utils/init/postinit.c,v
retrieving revision 1.186
diff -c -p -r1.186 postinit.c
*** src/backend/utils/init/postinit.c 23 Sep 2008 09:20:36 -0000 1.186
--- src/backend/utils/init/postinit.c 19 Dec 2008 23:39:48 -0000
***************
*** 19,24 ****
--- 19,25 ----
#include <unistd.h>
#include "access/heapam.h"
+ #include "access/reloptions.h"
#include "access/xact.h"
#include "catalog/catalog.h"
#include "catalog/namespace.h"
*************** InitPostgres(const char *in_dbname, Oid
*** 645,650 ****
--- 646,655 ----
if (!bootstrap)
pgstat_bestart();
+ /* initialize the relation options subsystem */
+ if (!bootstrap)
+ initialize_reloptions();
+
/* close the transaction we started above */
if (!bootstrap)
CommitTransactionCommand();
Index: src/include/access/reloptions.h
===================================================================
RCS file: /home/alvherre/Code/cvs/pgsql/src/include/access/reloptions.h,v
retrieving revision 1.5
diff -c -p -r1.5 reloptions.h
*** src/include/access/reloptions.h 1 Jan 2008 19:45:56 -0000 1.5
--- src/include/access/reloptions.h 29 Dec 2008 23:13:40 -0000
***************
*** 20,40 ****
#include "nodes/pg_list.h"
! extern Datum transformRelOptions(Datum oldOptions, List *defList,
! bool ignoreOids, bool isReset);
extern List *untransformRelOptions(Datum options);
!
! extern void parseRelOptions(Datum options, int numkeywords,
! const char *const * keywords,
! char **values, bool validate);
extern bytea *default_reloptions(Datum reloptions, bool validate,
! int minFillfactor, int defaultFillfactor);
!
extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
-
extern bytea *index_reloptions(RegProcedure amoptions, Datum reloptions,
! bool validate);
#endif /* RELOPTIONS_H */
--- 20,113 ----
#include "nodes/pg_list.h"
! /* types supported by reloptions */
! typedef enum relopt_type
! {
! RELOPT_TYPE_BOOL,
! RELOPT_TYPE_INT,
! RELOPT_TYPE_REAL
! } relopt_type;
!
! /* kinds supported by reloptions */
! typedef enum relopt_kind
! {
! RELOPT_KIND_HEAP,
! /* XXX do we need a separate kind for TOAST tables? */
! RELOPT_KIND_BTREE,
! RELOPT_KIND_HASH,
! RELOPT_KIND_GIN,
! RELOPT_KIND_GIST,
! /* if you add a new kind, make sure you update "last_default" too */
! RELOPT_KIND_LAST_DEFAULT = RELOPT_KIND_GIST,
! RELOPT_KIND_MAX = 255
! } relopt_kind;
!
! /* generic struct to hold shared data */
! typedef struct relopt_gen
! {
! const char *name; /* must be first (used as list termination marker) */
! const char *desc;
! relopt_kind kind;
! int namelen;
! relopt_type type;
! } relopt_gen;
!
! /* holds a parsed value */
! typedef struct relopt_value
! {
! relopt_gen *gen;
! bool isset;
! union
! {
! bool bool_val;
! int int_val;
! double real_val;
! } values;
! } relopt_value;
!
! /* reloptions records for specific variable types */
! typedef struct relopt_bool
! {
! relopt_gen gen;
! bool default_val;
! } relopt_bool;
!
! typedef struct relopt_int
! {
! relopt_gen gen;
! int default_val;
! int min;
! int max;
! } relopt_int;
!
! typedef struct relopt_real
! {
! relopt_gen gen;
! double default_val;
! double min;
! double max;
! } relopt_real;
!
! extern void initialize_reloptions(void);
!
! extern int add_reloption_kind(void);
! extern void add_bool_reloption(int kind, char *name, char *desc,
! bool default_val);
! extern void add_int_reloption(int kind, char *name, char *desc,
! int default_val, int min_val, int max_val);
! extern void add_real_reloption(int kind, char *name, char *desc,
! double default_val, double min_val, double max_val);
+ 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);
extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
extern bytea *index_reloptions(RegProcedure amoptions, Datum reloptions,
! bool validate);
#endif /* RELOPTIONS_H */
reloptions-extra-btree.patchtext/x-diff; charset=us-asciiDownload
*** src/backend/access/nbtree/nbtinsert.c 3 Nov 2008 20:47:48 -0000 1.168
--- src/backend/access/nbtree/nbtinsert.c 30 Dec 2008 14:29:21 -0000
***************
*** 565,570 ****
--- 565,581 ----
OffsetNumber firstright = InvalidOffsetNumber;
Size itemsz;
+ {
+ BtOptions *options = (BtOptions *) rel->rd_options;
+
+ if (options)
+ {
+ elog(LOG, "the testbool option is %s", options->testbool ? "true" : "false");
+ elog(LOG, "the testint option is %d", options->testint);
+ elog(LOG, "the testreal option is %f", options->testreal);
+ }
+ }
+
page = BufferGetPage(buf);
lpageop = (BTPageOpaque) PageGetSpecialPointer(page);
*** src/backend/access/nbtree/nbtutils.c 19 Jun 2008 00:46:03 -0000 1.91
--- src/backend/access/nbtree/nbtutils.c 30 Dec 2008 14:26:17 -0000
***************
*** 1401,1409 ****
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();
--- 1401,1481 ----
Datum reloptions = PG_GETARG_DATUM(0);
bool validate = PG_GETARG_BOOL(1);
bytea *result;
+ relopt_value *options;
+ BtOptions *rdopts;
+ BtOptions lopts;
+ int numoptions;
+ int len;
+ int i;
+ static bool initialized = false;
+
+ if (!initialized)
+ {
+ add_bool_reloption(RELOPT_KIND_BTREE, "testbool", NULL, false);
+ add_int_reloption(RELOPT_KIND_BTREE, "testint", NULL, 25, 10, 100);
+ add_real_reloption(RELOPT_KIND_BTREE, "testreal", NULL, 3.14, 2.78, 42.0);
+ initialized = true;
+ }
+
+ options = parseRelOptions(reloptions, validate, RELOPT_KIND_BTREE, &numoptions);
+
+ /* if none set, we're done */
+ if (numoptions == 0)
+ result = NULL;
+ else
+ {
+ MemSet(&lopts, 0, sizeof(BtOptions));
+
+ for (i = 0; i < numoptions; i++)
+ {
+ if (pg_strncasecmp(options[i].gen->name, "fillfactor",
+ options[i].gen->namelen) == 0)
+ {
+ if (options[i].isset)
+ lopts.fillfactor = options[i].values.int_val;
+ else
+ lopts.fillfactor = ((relopt_int *) options[i].gen)->default_val;
+ continue;
+ }
+ if (pg_strncasecmp(options[i].gen->name, "testbool",
+ options[i].gen->namelen) == 0)
+ {
+ if (options[i].isset)
+ lopts.testbool = options[i].values.bool_val;
+ else
+ lopts.testbool = ((relopt_bool *) options[i].gen)->default_val;
+ continue;
+ }
+ if (pg_strncasecmp(options[i].gen->name, "testint",
+ options[i].gen->namelen) == 0)
+ {
+ if (options[i].isset)
+ lopts.testint = options[i].values.int_val;
+ else
+ lopts.testint = ((relopt_int *) options[i].gen)->default_val;
+ continue;
+ }
+ if (pg_strncasecmp(options[i].gen->name, "testreal",
+ options[i].gen->namelen) == 0)
+ {
+ if (options[i].isset)
+ lopts.testreal = options[i].values.real_val;
+ else
+ lopts.testreal = ((relopt_real *) options[i].gen)->default_val;
+ continue;
+ }
+ }
+
+ pfree(options);
+
+ len = sizeof(BtOptions);
+ rdopts = palloc(len);
+ memcpy(rdopts, &lopts, len);
+ SET_VARSIZE(rdopts, len);
+
+ result = (bytea *) rdopts;
+ }
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
*** src/include/access/nbtree.h 13 Jul 2008 20:45:47 -0000 1.121
--- src/include/access/nbtree.h 30 Dec 2008 14:26:28 -0000
***************
*** 595,598 ****
--- 595,608 ----
extern void btree_xlog_cleanup(void);
extern bool btree_safe_restartpoint(void);
+ /* must follow StdRdOptions conventions */
+ typedef struct BtOptions
+ {
+ int32 vl_len_;
+ int fillfactor;
+ bool testbool;
+ int testint;
+ double testreal;
+ } BtOptions;
+
#endif /* NBTREE_H */
Re: generic reloptions improvement
Alvaro Herrera wrote:
Alvaro Herrera wrote:
Tom Lane wrote:
Alvaro Herrera <alvherre@commandprompt.com> writes:
I'm intending to have a new routine which would reserve a value at
runtime. This value would be later be passed by the AM to create new
options on the table.What do you mean by "at runtime"? Surely the value would have to remain
stable across database restarts, since it's going to relate to stuff
that is in catalog entries.No, there's no need for the value to be stable across restart; what's
stored in catalogs is the option name, which is linked to the kind
number only in the parser table.So this is an updated patch. This now allows a user-defined AM to
create new reloptions and pass them down to the parser for parsing and
checking. I also attach a proof-of-concept patch that adds three new
options to btree (which do nothing apart from logging a message at
insert time). This patch demonstrates the coding pattern that a
user-defined AM should follow to add and use new storage options.The main thing I find slightly hateful about this patch is that the code
to translate from the returned relopt_value array and the fixed struct
is rather verbose; and that the AM needs to duplicate the code in
default_reloptions. I don't find it ugly enough to warrant objecting to
the patch as a whole however.The neat thing about this code is that the parsing and searching is done
only once, when the relcache entry is loaded. Later accesses to the
option values themselves is just a struct access, and thus plenty quick.
This patch does not support reloptions in string expression, like:
CREATE TABLE t1 (
a int,
b text
) WITH (default_row_acl='{yamada=r/kaigai}');
Do you have any plan to support reloptions in string?
Thanks,
--
KaiGai Kohei <kaigai@kaigai.gr.jp>
Re: generic reloptions improvement
On Tue, 2008-12-30 at 12:31 -0300, Alvaro Herrera wrote:
Alvaro Herrera wrote:
Tom Lane wrote:
Alvaro Herrera <alvherre@commandprompt.com> writes:
I'm intending to have a new routine which would reserve a value at
runtime. This value would be later be passed by the AM to create new
options on the table.What do you mean by "at runtime"? Surely the value would have to remain
stable across database restarts, since it's going to relate to stuff
that is in catalog entries.No, there's no need for the value to be stable across restart; what's
stored in catalogs is the option name, which is linked to the kind
number only in the parser table.So this is an updated patch. This now allows a user-defined AM to
create new reloptions and pass them down to the parser for parsing and
checking. I also attach a proof-of-concept patch that adds three new
options to btree (which do nothing apart from logging a message at
insert time). This patch demonstrates the coding pattern that a
user-defined AM should follow to add and use new storage options.The main thing I find slightly hateful about this patch is that the code
to translate from the returned relopt_value array and the fixed struct
is rather verbose; and that the AM needs to duplicate the code in
default_reloptions. I don't find it ugly enough to warrant objecting to
the patch as a whole however.The neat thing about this code is that the parsing and searching is done
only once, when the relcache entry is loaded. Later accesses to the
option values themselves is just a struct access, and thus plenty quick.
I very much like the idea of adding new/custom options to tables. There
are many uses for that.
What you have here looks fairly hard to program for normal users. I
don't want to reject the feature, but the proposal you have here isn't
the best it could be...
Can we have something like customer variable classes, but just for
reloptions?
e.g. WITH (mymodule.my_option_name = X)
e.g. WITH (funky_trigger.coolness = 25)
We can then create new custom reloptions in roughly the same way we can
create custom variable classes, or ignore them if module not loaded.
--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support
Re: generic reloptions improvement
KaiGai Kohei wrote:
Alvaro Herrera wrote:
So this is an updated patch. This now allows a user-defined AM to
create new reloptions and pass them down to the parser for parsing and
checking.This patch does not support reloptions in string expression, like:
No, it doesn't. I asked about it some time ago and nobody answered. If
people feel it is important, I can implement it.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Re: generic reloptions improvement
Simon Riggs wrote:
I very much like the idea of adding new/custom options to tables. There
are many uses for that.
Hmm, like what? I'm not sure how would it work for tables; you'd have
to add the options during _PG_init or something like that, and I haven't
tried it. It's mainly for user-defined AMs that this was done.
What you have here looks fairly hard to program for normal users. I
don't want to reject the feature, but the proposal you have here isn't
the best it could be...
Agreed ...
Can we have something like customer variable classes, but just for
reloptions?e.g. WITH (mymodule.my_option_name = X)
e.g. WITH (funky_trigger.coolness = 25)We can then create new custom reloptions in roughly the same way we can
create custom variable classes, or ignore them if module not loaded.
I'm now playing with adding "namespaces" to the options, but that's for
handling options for toast tables. I'm not really sure how would it
work for regular options.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Re: generic reloptions improvement
On Sat, 2009-01-03 at 12:20 -0300, Alvaro Herrera wrote:
Simon Riggs wrote:
I very much like the idea of adding new/custom options to tables. There
are many uses for that.Hmm, like what? I'm not sure how would it work for tables; you'd have
to add the options during _PG_init or something like that, and I haven't
tried it. It's mainly for user-defined AMs that this was done.
I understand and agree with your intended use. I see others as well and
would like to cater for them all in a generic way that will have many
uses over next 10-20 years, many of which I haven't thought of yet.
Custom variable classes are often useful, but they are system wide. It
would be good to be able to use table-level options and have them work
very similarly to something we already have. Table-level options are
just an obvious "normalisation" of how we handle parameters.
If you really can't see a use for this, OK, then: Please can you put in
a plugin API for user defined reloptions as well as what you are
proposing. We discussed this before in late July/early Aug on thread
"Uncopied parameters..."
Can we have something like customer variable classes, but just for
reloptions?e.g. WITH (mymodule.my_option_name = X)
e.g. WITH (funky_trigger.coolness = 25)We can then create new custom reloptions in roughly the same way we can
create custom variable classes, or ignore them if module not loaded.I'm now playing with adding "namespaces" to the options, but that's for
handling options for toast tables. I'm not really sure how would it
work for regular options.
toast.option_x
btree.option_y
autovacuum.option_z
--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support
Re: generic reloptions improvement
Alvaro Herrera wrote:
KaiGai Kohei wrote:
Alvaro Herrera wrote:
So this is an updated patch. This now allows a user-defined AM to
create new reloptions and pass them down to the parser for parsing and
checking.This patch does not support reloptions in string expression, like:
No, it doesn't. I asked about it some time ago and nobody answered. If
people feel it is important, I can implement it.
Oh, I missed to see the message.
If it is provided for v8.4, I'm happy at least.
The Row-level ACLs need its reloption to specify default ACLs in string
expression. Currently, it modifies "reloptions.c", but using it on common
framework will be more appropriate implementation.
Thanks,
--
KaiGai Kohei <kaigai@kaigai.gr.jp>
Re: generic reloptions improvement
KaiGai Kohei wrote:
Alvaro Herrera wrote:
No, it doesn't. I asked about it some time ago and nobody answered. If
people feel it is important, I can implement it.Oh, I missed to see the message.
If it is provided for v8.4, I'm happy at least.
The Row-level ACLs need its reloption to specify default ACLs in string
expression. Currently, it modifies "reloptions.c", but using it on common
framework will be more appropriate implementation.
Ok, I see it now. I will have to implement string reloptions then.
Thanks for the notice.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Re: generic reloptions improvement
Simon Riggs wrote:
If you really can't see a use for this, OK, then: Please can you put in
a plugin API for user defined reloptions as well as what you are
proposing. We discussed this before in late July/early Aug on thread
"Uncopied parameters..."
Hmm, I was just looking at the CREATE TABLE LIKE code yesterday; I
didn't remember that discussion. I'll have a more detailed look.
I'm now playing with adding "namespaces" to the options, but that's for
handling options for toast tables. I'm not really sure how would it
work for regular options.toast.option_x
btree.option_y
autovacuum.option_z
autovacuum as a namespace doesn't work, because we need to have
autovacuum options for toast too. If we went down this route we would
need to have two name levels.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Re: generic reloptions improvement
KaiGai Kohei wrote:
If it is provided for v8.4, I'm happy at least.
The Row-level ACLs need its reloption to specify default ACLs in string
expression. Currently, it modifies "reloptions.c", but using it on common
framework will be more appropriate implementation.
Modified to add a string type.
Note that the real difficulty is what to do with the string in
default_reloptions (or the amoptions routine). I see that your patch
has already dealt with that, so it should be pretty easy for you; for
any reloption that wants to be stored in rel->rd_options, it will be
considerably more difficult (due to memory allocation).
Some notes about this patch:
- the string type handling (basically all the new code) is untested.
I'll have a look tomorrow at the btree test code I sent the other day to
add a string option and see how it goes.
- I have added some macros to deal with options in the most common
scenario, which is that they get stored in a predefined struct. This
hides part of the complexity in writing an amoptions routine.
- there's no way to define custom reloptions as requested by Simon. I
don't have any ideas on how to do that at this time.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Attachments:
reloptions-7.patchtext/x-diff; charset=us-asciiDownload
Index: src/backend/access/common/reloptions.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/access/common/reloptions.c,v
retrieving revision 1.12
diff -c -p -r1.12 reloptions.c
*** src/backend/access/common/reloptions.c 1 Jan 2009 17:23:34 -0000 1.12
--- src/backend/access/common/reloptions.c 4 Jan 2009 03:07:38 -0000
***************
*** 15,20 ****
--- 15,23 ----
#include "postgres.h"
+ #include "access/gist_private.h"
+ #include "access/hash.h"
+ #include "access/nbtree.h"
#include "access/reloptions.h"
#include "catalog/pg_type.h"
#include "commands/defrem.h"
***************
*** 22,29 ****
--- 25,357 ----
#include "utils/array.h"
#include "utils/builtins.h"
#include "utils/guc.h"
+ #include "utils/memutils.h"
#include "utils/rel.h"
+ /*
+ * Contents of pg_class.reloptions
+ *
+ * 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
+ *
+ * Note that we don't handle "oids" in relOpts because it is handled by
+ * interpretOidsOption().
+ */
+
+ static relopt_bool boolRelOpts[] =
+ {
+ /* list terminator */
+ { { NULL } }
+ };
+
+ static relopt_int intRelOpts[] =
+ {
+ {
+ {
+ "fillfactor",
+ "Packs table pages only to this percentage",
+ RELOPT_KIND_HEAP
+ },
+ HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100
+ },
+ {
+ {
+ "fillfactor",
+ "Packs btree index pages only to this percentage",
+ RELOPT_KIND_BTREE
+ },
+ BTREE_DEFAULT_FILLFACTOR, BTREE_MIN_FILLFACTOR, 100
+ },
+ {
+ {
+ "fillfactor",
+ "Packs hash index pages only to this percentage",
+ RELOPT_KIND_HASH
+ },
+ HASH_DEFAULT_FILLFACTOR, HASH_MIN_FILLFACTOR, 100
+ },
+ {
+ {
+ "fillfactor",
+ "Packs gist index pages only to this percentage",
+ RELOPT_KIND_GIST
+ },
+ GIST_DEFAULT_FILLFACTOR, GIST_MIN_FILLFACTOR, 100
+ },
+ /* list terminator */
+ { { NULL } }
+ };
+
+ static relopt_real realRelOpts[] =
+ {
+ /* list terminator */
+ { { NULL } }
+ };
+
+ static relopt_string stringRelOpts[] =
+ {
+ /* list terminator */
+ { { NULL } }
+ };
+
+ static relopt_gen **relOpts = NULL;
+ static int last_assigned_kind = RELOPT_KIND_LAST_DEFAULT + 1;
+
+ static int num_custom_options = 0;
+ static relopt_gen **custom_options = NULL;
+
+ static void parse_one_reloption(relopt_value *option, char *text_str,
+ int text_len, bool validate);
+
+ /*
+ * initialize_reloptions
+ * initialization routine, must be called at backend start
+ *
+ * Initialize the relOpts array and fill each variable's type and name length.
+ */
+ void
+ initialize_reloptions(void)
+ {
+ int i;
+ int j = 0;
+
+ for (i = 0; boolRelOpts[i].gen.name; i++)
+ j++;
+ for (i = 0; intRelOpts[i].gen.name; i++)
+ j++;
+ for (i = 0; realRelOpts[i].gen.name; i++)
+ j++;
+ for (i = 0; stringRelOpts[i].gen.name; i++)
+ j++;
+ j += num_custom_options;
+
+ if (relOpts)
+ pfree(relOpts);
+ relOpts = MemoryContextAlloc(TopMemoryContext,
+ (j + 1) * sizeof(relopt_gen *));
+
+ j = 0;
+ for (i = 0; boolRelOpts[i].gen.name; i++)
+ {
+ relOpts[j] = &boolRelOpts[i].gen;
+ relOpts[j]->type = RELOPT_TYPE_BOOL;
+ relOpts[j]->namelen = strlen(relOpts[j]->name);
+ j++;
+ }
+
+ for (i = 0; intRelOpts[i].gen.name; i++)
+ {
+ relOpts[j] = &intRelOpts[i].gen;
+ relOpts[j]->type = RELOPT_TYPE_INT;
+ relOpts[j]->namelen = strlen(relOpts[j]->name);
+ j++;
+ }
+
+ for (i = 0; realRelOpts[i].gen.name; i++)
+ {
+ relOpts[j] = &realRelOpts[i].gen;
+ relOpts[j]->type = RELOPT_TYPE_REAL;
+ relOpts[j]->namelen = strlen(relOpts[j]->name);
+ j++;
+ }
+
+ for (i = 0; stringRelOpts[i].gen.name; i++)
+ {
+ relOpts[j] = &stringRelOpts[i].gen;
+ relOpts[j]->type = RELOPT_TYPE_STRING;
+ relOpts[j]->namelen = strlen(relOpts[j]->name);
+ j++;
+ }
+
+ for (i = 0; i < num_custom_options; i++)
+ {
+ relOpts[j] = custom_options[i];
+ j++;
+ }
+
+ /* add a list terminator */
+ relOpts[j] = NULL;
+ }
+
+ /*
+ * add_reloption_kind
+ * Create a new relopt_kind value, to be used in custom reloptions by
+ * user-defined AMs.
+ */
+ int
+ add_reloption_kind(void)
+ {
+ return last_assigned_kind++;
+ }
+
+ /*
+ * add_reloption
+ * Add an already-created custom reloption to the list, and recompute the
+ * main parser table.
+ */
+ static void
+ add_reloption(relopt_gen *newoption)
+ {
+ static int max_custom_options = 0;
+
+ if (num_custom_options >= max_custom_options)
+ {
+ MemoryContext oldcxt;
+
+ oldcxt = MemoryContextSwitchTo(TopMemoryContext);
+
+ if (max_custom_options == 0)
+ {
+ max_custom_options = 8;
+ custom_options = palloc(max_custom_options * sizeof(relopt_gen *));
+ }
+ else
+ {
+ max_custom_options *= 2;
+ custom_options = repalloc(custom_options,
+ max_custom_options * sizeof(relopt_gen *));
+ }
+ MemoryContextSwitchTo(oldcxt);
+ }
+ custom_options[num_custom_options++] = newoption;
+
+ initialize_reloptions();
+ }
+
+ /*
+ * allocate_reloption
+ * Allocate a new reloption and initialize the type-agnostic fields
+ * (for types other than string)
+ */
+ static relopt_gen *
+ allocate_reloption(int kind, int type, char *name, char *desc)
+ {
+ MemoryContext oldcxt;
+ size_t size;
+ relopt_gen *newoption;
+
+ Assert(type != RELOPT_TYPE_STRING);
+
+ oldcxt = MemoryContextSwitchTo(TopMemoryContext);
+
+ switch (type)
+ {
+ case RELOPT_TYPE_BOOL:
+ size = sizeof(relopt_bool);
+ break;
+ case RELOPT_TYPE_INT:
+ size = sizeof(relopt_int);
+ break;
+ case RELOPT_TYPE_REAL:
+ size = sizeof(relopt_real);
+ break;
+ default:
+ elog(ERROR, "unsupported option type");
+ return NULL; /* keep compiler quiet */
+ }
+
+ newoption = palloc(size);
+
+ newoption->name = pstrdup(name);
+ if (desc)
+ newoption->desc = pstrdup(desc);
+ else
+ newoption->desc = NULL;
+ newoption->kind = kind;
+ newoption->namelen = strlen(name);
+ newoption->type = type;
+
+ MemoryContextSwitchTo(oldcxt);
+
+ return newoption;
+ }
+
+ /*
+ * add_bool_reloption
+ * Add a new boolean reloption
+ */
+ void
+ add_bool_reloption(int kind, char *name, char *desc, bool default_val)
+ {
+ relopt_bool *newoption;
+
+ newoption = (relopt_bool *) allocate_reloption(kind, RELOPT_TYPE_BOOL, name, desc);
+ newoption->default_val = default_val;
+
+ add_reloption((relopt_gen *) newoption);
+ }
+
+ /*
+ * add_int_reloption
+ * Add a new integer reloption
+ */
+ void
+ add_int_reloption(int kind, char *name, char *desc, int default_val,
+ int min_val, int max_val)
+ {
+ relopt_int *newoption;
+
+ newoption = (relopt_int *) allocate_reloption(kind, RELOPT_TYPE_INT, name, desc);
+ newoption->default_val = default_val;
+ newoption->min = min_val;
+ newoption->max = max_val;
+
+ add_reloption((relopt_gen *) newoption);
+ }
+
+ /*
+ * add_real_reloption
+ * Add a new float reloption
+ */
+ void
+ add_real_reloption(int kind, char *name, char *desc, double default_val,
+ double min_val, double max_val)
+ {
+ relopt_real *newoption;
+
+ newoption = (relopt_real *) allocate_reloption(kind, RELOPT_TYPE_REAL, name, desc);
+ newoption->default_val = default_val;
+ newoption->min = min_val;
+ newoption->max = max_val;
+
+ add_reloption((relopt_gen *) newoption);
+ }
+
+ /*
+ * add_string_reloption
+ * Add a new string reloption
+ */
+ void
+ add_string_reloption(int kind, char *name, char *desc, char *default_val)
+ {
+ MemoryContext oldcxt;
+ relopt_string *newoption;
+
+ oldcxt = MemoryContextSwitchTo(TopMemoryContext);
+
+ newoption = palloc0(sizeof(relopt_string) + strlen(default_val));
+
+ newoption->gen.name = pstrdup(name);
+ if (desc)
+ newoption->gen.desc = pstrdup(desc);
+ else
+ newoption->gen.desc = NULL;
+ newoption->gen.kind = kind;
+ newoption->gen.namelen = strlen(name);
+ newoption->gen.type = RELOPT_TYPE_STRING;
+ strcpy(newoption->default_val, default_val);
+
+ MemoryContextSwitchTo(oldcxt);
+
+ add_reloption((relopt_gen *) newoption);
+ }
/*
* Transform a relation options list (list of DefElem) into the text array
*************** untransformRelOptions(Datum options)
*** 198,344 ****
/*
* Interpret reloptions that are given in text-array format.
*
! * options: array of "keyword=value" strings, as built by transformRelOptions
! * numkeywords: number of legal keywords
! * keywords: the allowed keywords
! * values: output area
! * validate: if true, throw error for unrecognized keywords.
! *
! * The keywords and values arrays must both be of length numkeywords.
! * The values entry corresponding to a keyword is set to a palloc'd string
! * containing the corresponding value, or NULL if the keyword does not appear.
*/
! void
! parseRelOptions(Datum options, int numkeywords, const char *const * keywords,
! char **values, bool validate)
{
! ArrayType *array;
! Datum *optiondatums;
! int noptions;
int i;
! /* Initialize to "all defaulted" */
! MemSet(values, 0, numkeywords * sizeof(char *));
! /* Done if no options */
! if (!PointerIsValid(DatumGetPointer(options)))
! return;
! array = DatumGetArrayTypeP(options);
! Assert(ARR_ELEMTYPE(array) == TEXTOID);
! deconstruct_array(array, TEXTOID, -1, false, 'i',
! &optiondatums, NULL, &noptions);
! for (i = 0; i < noptions; i++)
{
! text *optiontext = DatumGetTextP(optiondatums[i]);
! char *text_str = VARDATA(optiontext);
! int text_len = VARSIZE(optiontext) - VARHDRSZ;
! int j;
! /* Search for a match in keywords */
! for (j = 0; j < numkeywords; j++)
{
! int kw_len = strlen(keywords[j]);
! if (text_len > kw_len && text_str[kw_len] == '=' &&
! pg_strncasecmp(text_str, keywords[j], kw_len) == 0)
{
! char *value;
! int value_len;
! if (values[j] && validate)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" specified more than once",
! keywords[j])));
! value_len = text_len - kw_len - 1;
! value = (char *) palloc(value_len + 1);
! memcpy(value, text_str + kw_len + 1, value_len);
! value[value_len] = '\0';
! values[j] = value;
! break;
}
- }
- if (j >= numkeywords && validate)
- {
- char *s;
- char *p;
! s = TextDatumGetCString(optiondatums[i]);
! p = strchr(s, '=');
! if (p)
! *p = '\0';
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("unrecognized parameter \"%s\"", s)));
}
}
}
/*
! * Parse reloptions for anything using StdRdOptions (ie, fillfactor only)
*/
bytea *
! default_reloptions(Datum reloptions, bool validate,
! int minFillfactor, int defaultFillfactor)
{
! static const char *const default_keywords[1] = {"fillfactor"};
! char *values[1];
! int fillfactor;
! StdRdOptions *result;
! parseRelOptions(reloptions, 1, default_keywords, values, validate);
! /*
! * If no options, we can just return NULL rather than doing anything.
! * (defaultFillfactor is thus not used, but we require callers to pass it
! * anyway since we would need it if more options were added.)
! */
! if (values[0] == NULL)
return NULL;
! if (!parse_int(values[0], &fillfactor, 0, NULL))
! {
! if (validate)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("fillfactor must be an integer: \"%s\"",
! values[0])));
! return NULL;
! }
! if (fillfactor < minFillfactor || fillfactor > 100)
{
! if (validate)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("fillfactor=%d is out of range (should be between %d and 100)",
! fillfactor, minFillfactor)));
! return NULL;
}
! result = (StdRdOptions *) palloc(sizeof(StdRdOptions));
! SET_VARSIZE(result, sizeof(StdRdOptions));
! result->fillfactor = fillfactor;
! return (bytea *) result;
}
-
/*
* Parse options for heaps (and perhaps someday toast tables).
*/
bytea *
heap_reloptions(char relkind, Datum reloptions, bool validate)
{
! return default_reloptions(reloptions, validate,
! HEAP_MIN_FILLFACTOR,
! HEAP_DEFAULT_FILLFACTOR);
}
--- 526,766 ----
/*
* Interpret reloptions that are given in text-array format.
*
! * options is a reloption text array as constructed by transformRelOptions.
! * kind specifies the family of options to be processed.
! *
! * The return value is a relopt_value * array on which the options actually
! * set in the options array are marked with isset=true. The length of this
! * array is returned in *numrelopts. Options not set are also present in the
! * array; this is so that the caller can easily locate the default values.
! *
! * If there are no options of the given kind, numrelopts is set to 0 and NULL
! * is returned.
! *
! * Note: values of type int, bool and real are allocated as part of the
! * returned array. Values of type string are allocated separately and must
! * be freed by the caller.
*/
! relopt_value *
! parseRelOptions(Datum options, bool validate, relopt_kind kind,
! int *numrelopts)
{
! relopt_value *reloptions;
! int numoptions = 0;
int i;
+ int j;
! /* Build a list of expected options, based on kind */
! for (i = 0; relOpts[i]; i++)
! if (relOpts[i]->kind == kind)
! numoptions++;
! if (numoptions == 0)
! {
! *numrelopts = 0;
! return NULL;
! }
! reloptions = palloc(numoptions * sizeof(relopt_value));
! for (i = 0, j = 0; relOpts[i]; i++)
! {
! if (relOpts[i]->kind == kind)
! {
! reloptions[j].gen = relOpts[i];
! reloptions[j].isset = false;
! j++;
! }
! }
! /* Done if no options */
! if (PointerIsValid(DatumGetPointer(options)))
{
! ArrayType *array;
! Datum *optiondatums;
! int noptions;
!
! array = DatumGetArrayTypeP(options);
!
! Assert(ARR_ELEMTYPE(array) == TEXTOID);
! deconstruct_array(array, TEXTOID, -1, false, 'i',
! &optiondatums, NULL, &noptions);
!
! for (i = 0; i < noptions; i++)
{
! text *optiontext = DatumGetTextP(optiondatums[i]);
! char *text_str = VARDATA(optiontext);
! int text_len = VARSIZE(optiontext) - VARHDRSZ;
! int j;
! /* Search for a match in reloptions */
! for (j = 0; j < numoptions; j++)
{
! int kw_len = reloptions[j].gen->namelen;
! if (text_len > kw_len && text_str[kw_len] == '=' &&
! pg_strncasecmp(text_str, reloptions[j].gen->name,
! kw_len) == 0)
! {
! parse_one_reloption(&reloptions[j], text_str, text_len,
! validate);
! break;
! }
}
! if (j >= numoptions && validate)
! {
! char *s;
! char *p;
!
! s = TextDatumGetCString(optiondatums[i]);
! p = strchr(s, '=');
! if (p)
! *p = '\0';
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("unrecognized parameter \"%s\"", s)));
! }
}
}
+
+ *numrelopts = numoptions;
+ return reloptions;
}
+ /*
+ * Subroutine for parseRelOptions, to parse and validate a single option's
+ * value
+ */
+ static void
+ parse_one_reloption(relopt_value *option, char *text_str, int text_len,
+ bool validate)
+ {
+ char *value;
+ int value_len;
+ bool parsed;
+ bool nofree = false;
+
+ if (option->isset && validate)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("parameter \"%s\" specified more than once",
+ option->gen->name)));
+
+ value_len = text_len - option->gen->namelen - 1;
+ value = (char *) palloc(value_len + 1);
+ memcpy(value, text_str + option->gen->namelen + 1, value_len);
+ value[value_len] = '\0';
+
+ switch (option->gen->type)
+ {
+ case RELOPT_TYPE_BOOL:
+ {
+ parsed = parse_bool(value, &option->values.bool_val);
+ if (validate && !parsed)
+ ereport(ERROR,
+ (errmsg("invalid value for boolean option \"%s\": %s",
+ option->gen->name, value)));
+ }
+ break;
+ case RELOPT_TYPE_INT:
+ {
+ relopt_int *optint = (relopt_int *) option->gen;
+
+ parsed = parse_int(value, &option->values.int_val, 0, NULL);
+ if (validate && !parsed)
+ ereport(ERROR,
+ (errmsg("invalid value for integer option \"%s\": %s",
+ option->gen->name, value)));
+ if (validate && (option->values.int_val < optint->min ||
+ option->values.int_val > optint->max))
+ ereport(ERROR,
+ (errmsg("value %s out of bounds for option \"%s\"",
+ value, option->gen->name),
+ errdetail("Valid values are between \"%d\" and \"%d\".",
+ optint->min, optint->max)));
+ }
+ break;
+ case RELOPT_TYPE_REAL:
+ {
+ relopt_real *optreal = (relopt_real *) option->gen;
+
+ parsed = parse_real(value, &option->values.real_val);
+ if (validate && !parsed)
+ ereport(ERROR,
+ (errmsg("invalid value for floating point option \"%s\": %s",
+ option->gen->name, value)));
+ if (validate && (option->values.real_val < optreal->min ||
+ option->values.real_val > optreal->max))
+ ereport(ERROR,
+ (errmsg("value %s out of bounds for option \"%s\"",
+ value, option->gen->name),
+ errdetail("Valid values are between \"%f\" and \"%f\".",
+ optreal->min, optreal->max)));
+ }
+ break;
+ case RELOPT_TYPE_STRING:
+ option->values.string_val = value;
+ nofree = true;
+
+ /* no validation possible */
+ break;
+ default:
+ elog(ERROR, "unsupported reloption type %d", option->gen->type);
+ break;
+ }
+
+ if (parsed)
+ option->isset = true;
+ if (!nofree)
+ pfree(value);
+ }
/*
! * Option parser for anything that uses StdRdOptions (i.e. fillfactor only)
*/
bytea *
! default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
{
! relopt_value *options;
! StdRdOptions *rdopts;
! StdRdOptions lopts;
! int numoptions;
! int len;
! int i;
! options = parseRelOptions(reloptions, validate, kind, &numoptions);
! /* if none set, we're done */
! if (numoptions == 0)
return NULL;
! MemSet(&lopts, 0, sizeof(StdRdOptions));
! for (i = 0; i < numoptions; i++)
{
! HANDLE_INT_RELOPTION("fillfactor", lopts.fillfactor, options[i]);
}
! pfree(options);
! len = sizeof(StdRdOptions);
! rdopts = palloc(len);
! memcpy(rdopts, &lopts, len);
! SET_VARSIZE(rdopts, len);
! return (bytea *) rdopts;
}
/*
* Parse options for heaps (and perhaps someday toast tables).
*/
bytea *
heap_reloptions(char relkind, Datum reloptions, bool validate)
{
! return default_reloptions(reloptions, validate, RELOPT_KIND_HEAP);
}
Index: src/backend/access/gin/ginutil.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/access/gin/ginutil.c,v
retrieving revision 1.19
diff -c -p -r1.19 ginutil.c
*** src/backend/access/gin/ginutil.c 1 Jan 2009 17:23:34 -0000 1.19
--- src/backend/access/gin/ginutil.c 3 Jan 2009 16:44:06 -0000
*************** ginoptions(PG_FUNCTION_ARGS)
*** 317,332 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! /*
! * It's not clear that fillfactor is useful for GIN, but for the moment
! * we'll accept it anyway. (It won't do anything...)
! */
! #define GIN_MIN_FILLFACTOR 10
! #define GIN_DEFAULT_FILLFACTOR 100
!
! result = default_reloptions(reloptions, validate,
! GIN_MIN_FILLFACTOR,
! GIN_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 317,323 ----
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate, RELOPT_KIND_GIN);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
Index: src/backend/access/gist/gistutil.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/access/gist/gistutil.c,v
retrieving revision 1.32
diff -c -p -r1.32 gistutil.c
*** src/backend/access/gist/gistutil.c 1 Jan 2009 17:23:35 -0000 1.32
--- src/backend/access/gist/gistutil.c 3 Jan 2009 16:44:06 -0000
*************** gistoptions(PG_FUNCTION_ARGS)
*** 670,678 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate,
! GIST_MIN_FILLFACTOR,
! GIST_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 670,677 ----
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate, RELOPT_KIND_GIST);
!
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
Index: src/backend/access/hash/hashutil.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/access/hash/hashutil.c,v
retrieving revision 1.58
diff -c -p -r1.58 hashutil.c
*** src/backend/access/hash/hashutil.c 1 Jan 2009 17:23:35 -0000 1.58
--- src/backend/access/hash/hashutil.c 3 Jan 2009 16:44:06 -0000
*************** hashoptions(PG_FUNCTION_ARGS)
*** 224,232 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate,
! HASH_MIN_FILLFACTOR,
! HASH_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 224,231 ----
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate, RELOPT_KIND_HASH);
!
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
Index: src/backend/access/nbtree/nbtutils.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/access/nbtree/nbtutils.c,v
retrieving revision 1.92
diff -c -p -r1.92 nbtutils.c
*** src/backend/access/nbtree/nbtutils.c 1 Jan 2009 17:23:36 -0000 1.92
--- src/backend/access/nbtree/nbtutils.c 3 Jan 2009 16:44:06 -0000
*************** btoptions(PG_FUNCTION_ARGS)
*** 1402,1410 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate,
! BTREE_MIN_FILLFACTOR,
! BTREE_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 1402,1409 ----
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();
Index: src/backend/utils/init/postinit.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/utils/init/postinit.c,v
retrieving revision 1.187
diff -c -p -r1.187 postinit.c
*** src/backend/utils/init/postinit.c 1 Jan 2009 17:23:51 -0000 1.187
--- src/backend/utils/init/postinit.c 3 Jan 2009 16:44:06 -0000
***************
*** 19,24 ****
--- 19,25 ----
#include <unistd.h>
#include "access/heapam.h"
+ #include "access/reloptions.h"
#include "access/xact.h"
#include "catalog/catalog.h"
#include "catalog/namespace.h"
*************** InitPostgres(const char *in_dbname, Oid
*** 645,650 ****
--- 646,655 ----
if (!bootstrap)
pgstat_bestart();
+ /* initialize the relation options subsystem */
+ if (!bootstrap)
+ initialize_reloptions();
+
/* close the transaction we started above */
if (!bootstrap)
CommitTransactionCommand();
Index: src/include/access/reloptions.h
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/include/access/reloptions.h,v
retrieving revision 1.6
diff -c -p -r1.6 reloptions.h
*** src/include/access/reloptions.h 1 Jan 2009 17:23:56 -0000 1.6
--- src/include/access/reloptions.h 4 Jan 2009 02:58:39 -0000
***************
*** 20,40 ****
#include "nodes/pg_list.h"
extern Datum transformRelOptions(Datum oldOptions, List *defList,
bool ignoreOids, bool isReset);
-
extern List *untransformRelOptions(Datum options);
!
! extern void parseRelOptions(Datum options, int numkeywords,
! const char *const * keywords,
! char **values, bool validate);
extern bytea *default_reloptions(Datum reloptions, bool validate,
! int minFillfactor, int defaultFillfactor);
!
extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
-
extern bytea *index_reloptions(RegProcedure amoptions, Datum reloptions,
! bool validate);
#endif /* RELOPTIONS_H */
--- 20,167 ----
#include "nodes/pg_list.h"
+ /* types supported by reloptions */
+ typedef enum relopt_type
+ {
+ RELOPT_TYPE_BOOL,
+ RELOPT_TYPE_INT,
+ RELOPT_TYPE_REAL,
+ RELOPT_TYPE_STRING
+ } relopt_type;
+
+ /* kinds supported by reloptions */
+ typedef enum relopt_kind
+ {
+ RELOPT_KIND_HEAP,
+ /* XXX do we need a separate kind for TOAST tables? */
+ RELOPT_KIND_BTREE,
+ RELOPT_KIND_HASH,
+ RELOPT_KIND_GIN,
+ RELOPT_KIND_GIST,
+ /* if you add a new kind, make sure you update "last_default" too */
+ RELOPT_KIND_LAST_DEFAULT = RELOPT_KIND_GIST,
+ RELOPT_KIND_MAX = 255
+ } relopt_kind;
+
+ /* generic struct to hold shared data */
+ typedef struct relopt_gen
+ {
+ const char *name; /* must be first (used as list termination marker) */
+ const char *desc;
+ relopt_kind kind;
+ int namelen;
+ relopt_type type;
+ } relopt_gen;
+
+ /* holds a parsed value */
+ typedef struct relopt_value
+ {
+ relopt_gen *gen;
+ bool isset;
+ union
+ {
+ bool bool_val;
+ int int_val;
+ double real_val;
+ char *string_val; /* allocated separately */
+ } values;
+ } relopt_value;
+
+ /* reloptions records for specific variable types */
+ typedef struct relopt_bool
+ {
+ relopt_gen gen;
+ bool default_val;
+ } relopt_bool;
+
+ typedef struct relopt_int
+ {
+ relopt_gen gen;
+ int default_val;
+ int min;
+ int max;
+ } relopt_int;
+
+ typedef struct relopt_real
+ {
+ relopt_gen gen;
+ double default_val;
+ double min;
+ double max;
+ } relopt_real;
+
+ typedef struct relopt_string
+ {
+ relopt_gen gen;
+ char default_val[1]; /* variable length */
+ } relopt_string;
+
+ #define HANDLE_INT_RELOPTION(optname, var, option) \
+ if (pg_strncasecmp(option.gen->name, optname, \
+ option.gen->namelen) == 0) \
+ { \
+ if (option.isset) \
+ var = option.values.int_val; \
+ else \
+ var = ((relopt_int *) option.gen)->default_val; \
+ continue; \
+ }
+
+ #define HANDLE_BOOL_RELOPTION(optname, var, option) \
+ if (pg_strncasecmp(option.gen->name, optname, \
+ option.gen->namelen) == 0) \
+ { \
+ if (option.isset) \
+ var = option.values.bool_val; \
+ else \
+ var = ((relopt_bool *) option.gen)->default_val; \
+ continue; \
+ }
+
+ #define HANDLE_REAL_RELOPTION(optname, var, option) \
+ if (pg_strncasecmp(option.gen->name, optname, \
+ option.gen->namelen) == 0) \
+ { \
+ if (option.isset) \
+ var = option.values.real_val; \
+ else \
+ var = ((relopt_real *) option.gen)->default_val; \
+ continue; \
+ }
+
+ #define HANDLE_STRING_RELOPTION(optname, var, option) \
+ if (pg_strncasecmp(option.gen->name, optname, \
+ option.gen->namelen) == 0) \
+ { \
+ if (option.isset) \
+ var = option.values.string_val; \
+ else \
+ var = ((relopt_string *) option.gen)->default_val; \
+ continue; \
+ }
+
+ extern void initialize_reloptions(void);
+
+ extern int add_reloption_kind(void);
+ extern void add_bool_reloption(int kind, char *name, char *desc,
+ bool default_val);
+ extern void add_int_reloption(int kind, char *name, char *desc,
+ int default_val, int min_val, int max_val);
+ extern void add_real_reloption(int kind, char *name, char *desc,
+ double default_val, double min_val, double max_val);
+ extern void add_string_reloption(int kind, char *name, char *desc,
+ char *default_val);
+
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);
extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
extern bytea *index_reloptions(RegProcedure amoptions, Datum reloptions,
! bool validate);
#endif /* RELOPTIONS_H */
Re: generic reloptions improvement
Alvaro Herrera wrote:
Some notes about this patch:
- the string type handling (basically all the new code) is untested.
I'll have a look tomorrow at the btree test code I sent the other day to
add a string option and see how it goes.
Okay, it was basically fine except for the attached minor correction.
Warning: I intend to commit this patch fairly soon!
As far as I can see, the new code can work with the options you've
defined in the SEPgsql code just fine. Handling string options in
itself is fine; the complexity (as I already said) is in allocating
memory for the string if you want to store it unchanged in the bytea
stuff in relcache. Since you're not storing the string itself but
convert it to an Oid, there's no problem.
Actually, storing the string itself works fine as long as you have a
single one, because you can define the option struct like this:
/* must follow StdRdOptions conventions */
typedef struct BtOptions
{
int32 vl_len_;
int fillfactor;
char teststring[1];
} BtOptions;
and there are no pointers involved. This doesn't work:
typedef struct BtOptions
{
int32 vl_len_;
int fillfactor;
char *teststring;
} BtOptions;
because then there's a pointer, and it fails as soon as the bytea * is
copied by the relcache code.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Attachments:
relopt-7a.patchtext/x-diff; charset=us-asciiDownload
--- src/backend/access/common/reloptions.c 4 Jan 2009 03:07:38 -0000
+++ src/backend/access/common/reloptions.c 4 Jan 2009 20:59:57 -0000
@@ -704,7 +704,7 @@
case RELOPT_TYPE_STRING:
option->values.string_val = value;
nofree = true;
-
+ parsed = true;
/* no validation possible */
break;
default:
Re: generic reloptions improvement
Simon Riggs wrote:
Custom variable classes are often useful, but they are system wide. It
would be good to be able to use table-level options and have them work
very similarly to something we already have. Table-level options are
just an obvious "normalisation" of how we handle parameters.If you really can't see a use for this, OK, then: Please can you put in
a plugin API for user defined reloptions as well as what you are
proposing. We discussed this before in late July/early Aug on thread
"Uncopied parameters..."
I've been giving this a thought and I don't see any easy way to handle
it. Since I've been threatened that this whole thing may be punted for
8.5 if I'm not quick about it, I've left this alone for now, and will
concentrate on getting the namespace thing done which will allow
specifying reloptions for the toast table by an ALTER TABLE on the main
table. It's not that I don't see a use for it; it's just that I don't
have the time for it right now.
(Note: I think there are ways to do this; they'll involve storing the
unknown options as a text array. It won't be pretty or performant, but
it seems the only way around the fact that heap_reloptions comes
hardcoded with the system.)
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Re: generic reloptions improvement
Alvaro Herrera wrote:
Okay, it was basically fine except for the attached minor correction.
Warning: I intend to commit this patch fairly soon!
This is the patch in its final form. I have included a few macros to
simplify the writing of amoptions routines.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Attachments:
reloptions-8.patchtext/x-diff; charset=us-asciiDownload
Index: src/backend/access/common/reloptions.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/access/common/reloptions.c,v
retrieving revision 1.12
diff -c -p -r1.12 reloptions.c
*** src/backend/access/common/reloptions.c 1 Jan 2009 17:23:34 -0000 1.12
--- src/backend/access/common/reloptions.c 4 Jan 2009 21:59:41 -0000
***************
*** 15,20 ****
--- 15,23 ----
#include "postgres.h"
+ #include "access/gist_private.h"
+ #include "access/hash.h"
+ #include "access/nbtree.h"
#include "access/reloptions.h"
#include "catalog/pg_type.h"
#include "commands/defrem.h"
***************
*** 22,29 ****
--- 25,362 ----
#include "utils/array.h"
#include "utils/builtins.h"
#include "utils/guc.h"
+ #include "utils/memutils.h"
#include "utils/rel.h"
+ /*
+ * Contents of pg_class.reloptions
+ *
+ * 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
+ *
+ * Note that we don't handle "oids" in relOpts because it is handled by
+ * interpretOidsOption().
+ */
+
+ static relopt_bool boolRelOpts[] =
+ {
+ /* list terminator */
+ { { NULL } }
+ };
+
+ static relopt_int intRelOpts[] =
+ {
+ {
+ {
+ "fillfactor",
+ "Packs table pages only to this percentage",
+ RELOPT_KIND_HEAP
+ },
+ HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100
+ },
+ {
+ {
+ "fillfactor",
+ "Packs btree index pages only to this percentage",
+ RELOPT_KIND_BTREE
+ },
+ BTREE_DEFAULT_FILLFACTOR, BTREE_MIN_FILLFACTOR, 100
+ },
+ {
+ {
+ "fillfactor",
+ "Packs hash index pages only to this percentage",
+ RELOPT_KIND_HASH
+ },
+ HASH_DEFAULT_FILLFACTOR, HASH_MIN_FILLFACTOR, 100
+ },
+ {
+ {
+ "fillfactor",
+ "Packs gist index pages only to this percentage",
+ RELOPT_KIND_GIST
+ },
+ GIST_DEFAULT_FILLFACTOR, GIST_MIN_FILLFACTOR, 100
+ },
+ /* list terminator */
+ { { NULL } }
+ };
+
+ static relopt_real realRelOpts[] =
+ {
+ /* list terminator */
+ { { NULL } }
+ };
+
+ static relopt_string stringRelOpts[] =
+ {
+ /* list terminator */
+ { { NULL } }
+ };
+
+ static relopt_gen **relOpts = NULL;
+ static int last_assigned_kind = RELOPT_KIND_LAST_DEFAULT + 1;
+
+ static int num_custom_options = 0;
+ static relopt_gen **custom_options = NULL;
+ static bool need_initialization = true;
+
+ static void parse_one_reloption(relopt_value *option, char *text_str,
+ int text_len, bool validate);
+
+ /*
+ * initialize_reloptions
+ * initialization routine, must be called before parsing
+ *
+ * Initialize the relOpts array and fill each variable's type and name length.
+ */
+ void
+ initialize_reloptions(void)
+ {
+ int i;
+ int j = 0;
+
+ for (i = 0; boolRelOpts[i].gen.name; i++)
+ j++;
+ for (i = 0; intRelOpts[i].gen.name; i++)
+ j++;
+ for (i = 0; realRelOpts[i].gen.name; i++)
+ j++;
+ for (i = 0; stringRelOpts[i].gen.name; i++)
+ j++;
+ j += num_custom_options;
+
+ if (relOpts)
+ pfree(relOpts);
+ relOpts = MemoryContextAlloc(TopMemoryContext,
+ (j + 1) * sizeof(relopt_gen *));
+
+ j = 0;
+ for (i = 0; boolRelOpts[i].gen.name; i++)
+ {
+ relOpts[j] = &boolRelOpts[i].gen;
+ relOpts[j]->type = RELOPT_TYPE_BOOL;
+ relOpts[j]->namelen = strlen(relOpts[j]->name);
+ j++;
+ }
+
+ for (i = 0; intRelOpts[i].gen.name; i++)
+ {
+ relOpts[j] = &intRelOpts[i].gen;
+ relOpts[j]->type = RELOPT_TYPE_INT;
+ relOpts[j]->namelen = strlen(relOpts[j]->name);
+ j++;
+ }
+
+ for (i = 0; realRelOpts[i].gen.name; i++)
+ {
+ relOpts[j] = &realRelOpts[i].gen;
+ relOpts[j]->type = RELOPT_TYPE_REAL;
+ relOpts[j]->namelen = strlen(relOpts[j]->name);
+ j++;
+ }
+
+ for (i = 0; stringRelOpts[i].gen.name; i++)
+ {
+ relOpts[j] = &stringRelOpts[i].gen;
+ relOpts[j]->type = RELOPT_TYPE_STRING;
+ relOpts[j]->namelen = strlen(relOpts[j]->name);
+ j++;
+ }
+
+ for (i = 0; i < num_custom_options; i++)
+ {
+ relOpts[j] = custom_options[i];
+ j++;
+ }
+
+ /* add a list terminator */
+ relOpts[j] = NULL;
+ }
+
+ /*
+ * add_reloption_kind
+ * Create a new relopt_kind value, to be used in custom reloptions by
+ * user-defined AMs.
+ */
+ int
+ add_reloption_kind(void)
+ {
+ return last_assigned_kind++;
+ }
+
+ /*
+ * add_reloption
+ * Add an already-created custom reloption to the list, and recompute the
+ * main parser table.
+ */
+ static void
+ add_reloption(relopt_gen *newoption)
+ {
+ static int max_custom_options = 0;
+
+ if (num_custom_options >= max_custom_options)
+ {
+ MemoryContext oldcxt;
+
+ oldcxt = MemoryContextSwitchTo(TopMemoryContext);
+
+ if (max_custom_options == 0)
+ {
+ max_custom_options = 8;
+ custom_options = palloc(max_custom_options * sizeof(relopt_gen *));
+ }
+ else
+ {
+ max_custom_options *= 2;
+ custom_options = repalloc(custom_options,
+ max_custom_options * sizeof(relopt_gen *));
+ }
+ MemoryContextSwitchTo(oldcxt);
+ }
+ custom_options[num_custom_options++] = newoption;
+
+ need_initialization = true;
+ }
+
+ /*
+ * allocate_reloption
+ * Allocate a new reloption and initialize the type-agnostic fields
+ * (for types other than string)
+ */
+ static relopt_gen *
+ allocate_reloption(int kind, int type, char *name, char *desc)
+ {
+ MemoryContext oldcxt;
+ size_t size;
+ relopt_gen *newoption;
+
+ Assert(type != RELOPT_TYPE_STRING);
+
+ oldcxt = MemoryContextSwitchTo(TopMemoryContext);
+
+ switch (type)
+ {
+ case RELOPT_TYPE_BOOL:
+ size = sizeof(relopt_bool);
+ break;
+ case RELOPT_TYPE_INT:
+ size = sizeof(relopt_int);
+ break;
+ case RELOPT_TYPE_REAL:
+ size = sizeof(relopt_real);
+ break;
+ default:
+ elog(ERROR, "unsupported option type");
+ return NULL; /* keep compiler quiet */
+ }
+
+ newoption = palloc(size);
+
+ newoption->name = pstrdup(name);
+ if (desc)
+ newoption->desc = pstrdup(desc);
+ else
+ newoption->desc = NULL;
+ newoption->kind = kind;
+ newoption->namelen = strlen(name);
+ newoption->type = type;
+
+ MemoryContextSwitchTo(oldcxt);
+
+ return newoption;
+ }
+
+ /*
+ * add_bool_reloption
+ * Add a new boolean reloption
+ */
+ void
+ add_bool_reloption(int kind, char *name, char *desc, bool default_val)
+ {
+ relopt_bool *newoption;
+
+ newoption = (relopt_bool *) allocate_reloption(kind, RELOPT_TYPE_BOOL,
+ name, desc);
+ newoption->default_val = default_val;
+
+ add_reloption((relopt_gen *) newoption);
+ }
+
+ /*
+ * add_int_reloption
+ * Add a new integer reloption
+ */
+ void
+ add_int_reloption(int kind, char *name, char *desc, int default_val,
+ int min_val, int max_val)
+ {
+ relopt_int *newoption;
+
+ newoption = (relopt_int *) allocate_reloption(kind, RELOPT_TYPE_INT,
+ name, desc);
+ newoption->default_val = default_val;
+ newoption->min = min_val;
+ newoption->max = max_val;
+
+ add_reloption((relopt_gen *) newoption);
+ }
+
+ /*
+ * add_real_reloption
+ * Add a new float reloption
+ */
+ void
+ add_real_reloption(int kind, char *name, char *desc, double default_val,
+ double min_val, double max_val)
+ {
+ relopt_real *newoption;
+
+ newoption = (relopt_real *) allocate_reloption(kind, RELOPT_TYPE_REAL,
+ name, desc);
+ newoption->default_val = default_val;
+ newoption->min = min_val;
+ newoption->max = max_val;
+
+ add_reloption((relopt_gen *) newoption);
+ }
+
+ /*
+ * add_string_reloption
+ * Add a new string reloption
+ */
+ void
+ add_string_reloption(int kind, char *name, char *desc, char *default_val)
+ {
+ MemoryContext oldcxt;
+ relopt_string *newoption;
+
+ oldcxt = MemoryContextSwitchTo(TopMemoryContext);
+
+ newoption = palloc0(sizeof(relopt_string) + strlen(default_val));
+
+ newoption->gen.name = pstrdup(name);
+ if (desc)
+ newoption->gen.desc = pstrdup(desc);
+ else
+ newoption->gen.desc = NULL;
+ newoption->gen.kind = kind;
+ newoption->gen.namelen = strlen(name);
+ newoption->gen.type = RELOPT_TYPE_STRING;
+ strcpy(newoption->default_val, default_val);
+ newoption->default_len = strlen(default_val);
+
+ MemoryContextSwitchTo(oldcxt);
+
+ add_reloption((relopt_gen *) newoption);
+ }
/*
* Transform a relation options list (list of DefElem) into the text array
*************** untransformRelOptions(Datum options)
*** 198,344 ****
/*
* Interpret reloptions that are given in text-array format.
*
! * options: array of "keyword=value" strings, as built by transformRelOptions
! * numkeywords: number of legal keywords
! * keywords: the allowed keywords
! * values: output area
! * validate: if true, throw error for unrecognized keywords.
! *
! * The keywords and values arrays must both be of length numkeywords.
! * The values entry corresponding to a keyword is set to a palloc'd string
! * containing the corresponding value, or NULL if the keyword does not appear.
*/
! void
! parseRelOptions(Datum options, int numkeywords, const char *const * keywords,
! char **values, bool validate)
{
! ArrayType *array;
! Datum *optiondatums;
! int noptions;
int i;
! /* Initialize to "all defaulted" */
! MemSet(values, 0, numkeywords * sizeof(char *));
! /* Done if no options */
! if (!PointerIsValid(DatumGetPointer(options)))
! return;
! array = DatumGetArrayTypeP(options);
! Assert(ARR_ELEMTYPE(array) == TEXTOID);
! deconstruct_array(array, TEXTOID, -1, false, 'i',
! &optiondatums, NULL, &noptions);
! for (i = 0; i < noptions; i++)
{
! text *optiontext = DatumGetTextP(optiondatums[i]);
! char *text_str = VARDATA(optiontext);
! int text_len = VARSIZE(optiontext) - VARHDRSZ;
! int j;
! /* Search for a match in keywords */
! for (j = 0; j < numkeywords; j++)
{
! int kw_len = strlen(keywords[j]);
! if (text_len > kw_len && text_str[kw_len] == '=' &&
! pg_strncasecmp(text_str, keywords[j], kw_len) == 0)
{
! char *value;
! int value_len;
! if (values[j] && validate)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("parameter \"%s\" specified more than once",
! keywords[j])));
! value_len = text_len - kw_len - 1;
! value = (char *) palloc(value_len + 1);
! memcpy(value, text_str + kw_len + 1, value_len);
! value[value_len] = '\0';
! values[j] = value;
! break;
}
- }
- if (j >= numkeywords && validate)
- {
- char *s;
- char *p;
! s = TextDatumGetCString(optiondatums[i]);
! p = strchr(s, '=');
! if (p)
! *p = '\0';
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("unrecognized parameter \"%s\"", s)));
}
}
}
/*
! * Parse reloptions for anything using StdRdOptions (ie, fillfactor only)
*/
bytea *
! default_reloptions(Datum reloptions, bool validate,
! int minFillfactor, int defaultFillfactor)
{
! static const char *const default_keywords[1] = {"fillfactor"};
! char *values[1];
! int fillfactor;
! StdRdOptions *result;
! parseRelOptions(reloptions, 1, default_keywords, values, validate);
! /*
! * If no options, we can just return NULL rather than doing anything.
! * (defaultFillfactor is thus not used, but we require callers to pass it
! * anyway since we would need it if more options were added.)
! */
! if (values[0] == NULL)
return NULL;
! if (!parse_int(values[0], &fillfactor, 0, NULL))
! {
! if (validate)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("fillfactor must be an integer: \"%s\"",
! values[0])));
! return NULL;
! }
! if (fillfactor < minFillfactor || fillfactor > 100)
{
! if (validate)
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("fillfactor=%d is out of range (should be between %d and 100)",
! fillfactor, minFillfactor)));
! return NULL;
}
! result = (StdRdOptions *) palloc(sizeof(StdRdOptions));
! SET_VARSIZE(result, sizeof(StdRdOptions));
! result->fillfactor = fillfactor;
! return (bytea *) result;
}
-
/*
* Parse options for heaps (and perhaps someday toast tables).
*/
bytea *
heap_reloptions(char relkind, Datum reloptions, bool validate)
{
! return default_reloptions(reloptions, validate,
! HEAP_MIN_FILLFACTOR,
! HEAP_DEFAULT_FILLFACTOR);
}
--- 531,774 ----
/*
* Interpret reloptions that are given in text-array format.
*
! * options is a reloption text array as constructed by transformRelOptions.
! * kind specifies the family of options to be processed.
! *
! * The return value is a relopt_value * array on which the options actually
! * set in the options array are marked with isset=true. The length of this
! * array is returned in *numrelopts. Options not set are also present in the
! * array; this is so that the caller can easily locate the default values.
! *
! * If there are no options of the given kind, numrelopts is set to 0 and NULL
! * is returned.
! *
! * Note: values of type int, bool and real are allocated as part of the
! * returned array. Values of type string are allocated separately and must
! * be freed by the caller.
*/
! relopt_value *
! parseRelOptions(Datum options, bool validate, relopt_kind kind,
! int *numrelopts)
{
! relopt_value *reloptions;
! int numoptions = 0;
int i;
+ int j;
! if (need_initialization)
! initialize_reloptions();
! /* Build a list of expected options, based on kind */
! for (i = 0; relOpts[i]; i++)
! if (relOpts[i]->kind == kind)
! numoptions++;
! if (numoptions == 0)
! {
! *numrelopts = 0;
! return NULL;
! }
! reloptions = palloc(numoptions * sizeof(relopt_value));
! for (i = 0, j = 0; relOpts[i]; i++)
! {
! if (relOpts[i]->kind == kind)
! {
! reloptions[j].gen = relOpts[i];
! reloptions[j].isset = false;
! j++;
! }
! }
!
! /* Done if no options */
! if (PointerIsValid(DatumGetPointer(options)))
{
! ArrayType *array;
! Datum *optiondatums;
! int noptions;
!
! array = DatumGetArrayTypeP(options);
!
! Assert(ARR_ELEMTYPE(array) == TEXTOID);
! deconstruct_array(array, TEXTOID, -1, false, 'i',
! &optiondatums, NULL, &noptions);
!
! for (i = 0; i < noptions; i++)
{
! text *optiontext = DatumGetTextP(optiondatums[i]);
! char *text_str = VARDATA(optiontext);
! int text_len = VARSIZE(optiontext) - VARHDRSZ;
! int j;
! /* Search for a match in reloptions */
! for (j = 0; j < numoptions; j++)
{
! int kw_len = reloptions[j].gen->namelen;
! if (text_len > kw_len && text_str[kw_len] == '=' &&
! pg_strncasecmp(text_str, reloptions[j].gen->name,
! kw_len) == 0)
! {
! parse_one_reloption(&reloptions[j], text_str, text_len,
! validate);
! break;
! }
}
! if (j >= numoptions && validate)
! {
! char *s;
! char *p;
!
! s = TextDatumGetCString(optiondatums[i]);
! p = strchr(s, '=');
! if (p)
! *p = '\0';
! ereport(ERROR,
! (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
! errmsg("unrecognized parameter \"%s\"", s)));
! }
}
}
+
+ *numrelopts = numoptions;
+ return reloptions;
}
+ /*
+ * Subroutine for parseRelOptions, to parse and validate a single option's
+ * value
+ */
+ static void
+ parse_one_reloption(relopt_value *option, char *text_str, int text_len,
+ bool validate)
+ {
+ char *value;
+ int value_len;
+ bool parsed;
+ bool nofree = false;
+
+ if (option->isset && validate)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("parameter \"%s\" specified more than once",
+ option->gen->name)));
+
+ value_len = text_len - option->gen->namelen - 1;
+ value = (char *) palloc(value_len + 1);
+ memcpy(value, text_str + option->gen->namelen + 1, value_len);
+ value[value_len] = '\0';
+
+ switch (option->gen->type)
+ {
+ case RELOPT_TYPE_BOOL:
+ {
+ parsed = parse_bool(value, &option->values.bool_val);
+ if (validate && !parsed)
+ ereport(ERROR,
+ (errmsg("invalid value for boolean option \"%s\": %s",
+ option->gen->name, value)));
+ }
+ break;
+ case RELOPT_TYPE_INT:
+ {
+ relopt_int *optint = (relopt_int *) option->gen;
+
+ parsed = parse_int(value, &option->values.int_val, 0, NULL);
+ if (validate && !parsed)
+ ereport(ERROR,
+ (errmsg("invalid value for integer option \"%s\": %s",
+ option->gen->name, value)));
+ if (validate && (option->values.int_val < optint->min ||
+ option->values.int_val > optint->max))
+ ereport(ERROR,
+ (errmsg("value %s out of bounds for option \"%s\"",
+ value, option->gen->name),
+ errdetail("Valid values are between \"%d\" and \"%d\".",
+ optint->min, optint->max)));
+ }
+ break;
+ case RELOPT_TYPE_REAL:
+ {
+ relopt_real *optreal = (relopt_real *) option->gen;
+
+ parsed = parse_real(value, &option->values.real_val);
+ if (validate && !parsed)
+ ereport(ERROR,
+ (errmsg("invalid value for floating point option \"%s\": %s",
+ option->gen->name, value)));
+ if (validate && (option->values.real_val < optreal->min ||
+ option->values.real_val > optreal->max))
+ ereport(ERROR,
+ (errmsg("value %s out of bounds for option \"%s\"",
+ value, option->gen->name),
+ errdetail("Valid values are between \"%f\" and \"%f\".",
+ optreal->min, optreal->max)));
+ }
+ break;
+ case RELOPT_TYPE_STRING:
+ option->values.string_val = value;
+ nofree = true;
+ parsed = true;
+ /* no validation possible */
+ break;
+ default:
+ elog(ERROR, "unsupported reloption type %d", option->gen->type);
+ break;
+ }
+
+ if (parsed)
+ option->isset = true;
+ if (!nofree)
+ pfree(value);
+ }
/*
! * Option parser for anything that uses StdRdOptions (i.e. fillfactor only)
*/
bytea *
! default_reloptions(Datum reloptions, bool validate, relopt_kind kind)
{
! relopt_value *options;
! StdRdOptions *rdopts;
! StdRdOptions lopts;
! int numoptions;
! int len;
! int i;
! options = parseRelOptions(reloptions, validate, kind, &numoptions);
! /* if none set, we're done */
! if (numoptions == 0)
return NULL;
! MemSet(&lopts, 0, sizeof(StdRdOptions));
! for (i = 0; i < numoptions; i++)
{
! HANDLE_INT_RELOPTION("fillfactor", lopts.fillfactor, options[i]);
}
! pfree(options);
! len = sizeof(StdRdOptions);
! rdopts = palloc(len);
! memcpy(rdopts, &lopts, len);
! SET_VARSIZE(rdopts, len);
! return (bytea *) rdopts;
}
/*
* Parse options for heaps (and perhaps someday toast tables).
*/
bytea *
heap_reloptions(char relkind, Datum reloptions, bool validate)
{
! return default_reloptions(reloptions, validate, RELOPT_KIND_HEAP);
}
Index: src/backend/access/gin/ginutil.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/access/gin/ginutil.c,v
retrieving revision 1.19
diff -c -p -r1.19 ginutil.c
*** src/backend/access/gin/ginutil.c 1 Jan 2009 17:23:34 -0000 1.19
--- src/backend/access/gin/ginutil.c 3 Jan 2009 16:44:06 -0000
*************** ginoptions(PG_FUNCTION_ARGS)
*** 317,332 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! /*
! * It's not clear that fillfactor is useful for GIN, but for the moment
! * we'll accept it anyway. (It won't do anything...)
! */
! #define GIN_MIN_FILLFACTOR 10
! #define GIN_DEFAULT_FILLFACTOR 100
!
! result = default_reloptions(reloptions, validate,
! GIN_MIN_FILLFACTOR,
! GIN_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 317,323 ----
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate, RELOPT_KIND_GIN);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
Index: src/backend/access/gist/gistutil.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/access/gist/gistutil.c,v
retrieving revision 1.32
diff -c -p -r1.32 gistutil.c
*** src/backend/access/gist/gistutil.c 1 Jan 2009 17:23:35 -0000 1.32
--- src/backend/access/gist/gistutil.c 3 Jan 2009 16:44:06 -0000
*************** gistoptions(PG_FUNCTION_ARGS)
*** 670,678 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate,
! GIST_MIN_FILLFACTOR,
! GIST_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 670,677 ----
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate, RELOPT_KIND_GIST);
!
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
Index: src/backend/access/hash/hashutil.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/access/hash/hashutil.c,v
retrieving revision 1.58
diff -c -p -r1.58 hashutil.c
*** src/backend/access/hash/hashutil.c 1 Jan 2009 17:23:35 -0000 1.58
--- src/backend/access/hash/hashutil.c 3 Jan 2009 16:44:06 -0000
*************** hashoptions(PG_FUNCTION_ARGS)
*** 224,232 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate,
! HASH_MIN_FILLFACTOR,
! HASH_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 224,231 ----
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate, RELOPT_KIND_HASH);
!
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
Index: src/backend/access/nbtree/nbtutils.c
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/backend/access/nbtree/nbtutils.c,v
retrieving revision 1.92
diff -c -p -r1.92 nbtutils.c
*** src/backend/access/nbtree/nbtutils.c 1 Jan 2009 17:23:36 -0000 1.92
--- src/backend/access/nbtree/nbtutils.c 4 Jan 2009 21:42:50 -0000
*************** btoptions(PG_FUNCTION_ARGS)
*** 1402,1410 ****
bool validate = PG_GETARG_BOOL(1);
bytea *result;
! result = default_reloptions(reloptions, validate,
! BTREE_MIN_FILLFACTOR,
! BTREE_DEFAULT_FILLFACTOR);
if (result)
PG_RETURN_BYTEA_P(result);
PG_RETURN_NULL();
--- 1402,1408 ----
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();
Index: src/include/access/reloptions.h
===================================================================
RCS file: /home/alvherre/cvs/pgsql/src/include/access/reloptions.h,v
retrieving revision 1.6
diff -c -p -r1.6 reloptions.h
*** src/include/access/reloptions.h 1 Jan 2009 17:23:56 -0000 1.6
--- src/include/access/reloptions.h 4 Jan 2009 21:52:17 -0000
***************
*** 20,40 ****
#include "nodes/pg_list.h"
extern Datum transformRelOptions(Datum oldOptions, List *defList,
bool ignoreOids, bool isReset);
-
extern List *untransformRelOptions(Datum options);
!
! extern void parseRelOptions(Datum options, int numkeywords,
! const char *const * keywords,
! char **values, bool validate);
extern bytea *default_reloptions(Datum reloptions, bool validate,
! int minFillfactor, int defaultFillfactor);
!
extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
-
extern bytea *index_reloptions(RegProcedure amoptions, Datum reloptions,
! bool validate);
#endif /* RELOPTIONS_H */
--- 20,175 ----
#include "nodes/pg_list.h"
+ /* types supported by reloptions */
+ typedef enum relopt_type
+ {
+ RELOPT_TYPE_BOOL,
+ RELOPT_TYPE_INT,
+ RELOPT_TYPE_REAL,
+ RELOPT_TYPE_STRING
+ } relopt_type;
+
+ /* kinds supported by reloptions */
+ typedef enum relopt_kind
+ {
+ RELOPT_KIND_HEAP,
+ /* XXX do we need a separate kind for TOAST tables? */
+ RELOPT_KIND_BTREE,
+ RELOPT_KIND_HASH,
+ RELOPT_KIND_GIN,
+ RELOPT_KIND_GIST,
+ /* if you add a new kind, make sure you update "last_default" too */
+ RELOPT_KIND_LAST_DEFAULT = RELOPT_KIND_GIST,
+ RELOPT_KIND_MAX = 255
+ } relopt_kind;
+
+ /* generic struct to hold shared data */
+ typedef struct relopt_gen
+ {
+ const char *name; /* must be first (used as list termination marker) */
+ const char *desc;
+ relopt_kind kind;
+ int namelen;
+ relopt_type type;
+ } relopt_gen;
+
+ /* holds a parsed value */
+ typedef struct relopt_value
+ {
+ relopt_gen *gen;
+ bool isset;
+ union
+ {
+ bool bool_val;
+ int int_val;
+ double real_val;
+ char *string_val; /* allocated separately */
+ } values;
+ } relopt_value;
+
+ /* reloptions records for specific variable types */
+ typedef struct relopt_bool
+ {
+ relopt_gen gen;
+ bool default_val;
+ } relopt_bool;
+
+ typedef struct relopt_int
+ {
+ relopt_gen gen;
+ int default_val;
+ int min;
+ int max;
+ } relopt_int;
+
+ typedef struct relopt_real
+ {
+ relopt_gen gen;
+ double default_val;
+ double min;
+ double max;
+ } relopt_real;
+
+ typedef struct relopt_string
+ {
+ relopt_gen gen;
+ int default_len;
+ char default_val[1]; /* variable length */
+ } relopt_string;
+
+ /*
+ * These macros exist for the convenience of amoptions writers. See
+ * default_reloptions for an example of the intended usage.
+ *
+ * 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.)
+ */
+ #define HAVE_RELOPTION(optname, option) \
+ (pg_strncasecmp(option.gen->name, optname, option.gen->namelen) == 0)
+
+ #define HANDLE_INT_RELOPTION(optname, var, option) \
+ if (HAVE_RELOPTION(optname, option)) \
+ { \
+ if (option.isset) \
+ var = option.values.int_val; \
+ else \
+ var = ((relopt_int *) option.gen)->default_val; \
+ continue; \
+ }
+
+ #define HANDLE_BOOL_RELOPTION(optname, var, option) \
+ if (HAVE_RELOPTION(optname, option)) \
+ { \
+ if (option.isset) \
+ var = option.values.bool_val; \
+ else \
+ var = ((relopt_bool *) option.gen)->default_val; \
+ continue; \
+ }
+
+ #define HANDLE_REAL_RELOPTION(optname, var, option) \
+ if (HAVE_RELOPTION(optname, option)) \
+ { \
+ if (option.isset) \
+ var = option.values.real_val; \
+ else \
+ var = ((relopt_real *) option.gen)->default_val; \
+ continue; \
+ }
+
+ /* Note that this assumes that the variable is already allocated! */
+ #define HANDLE_STRING_RELOPTION(optname, var, option) \
+ if (HAVE_RELOPTION(optname, option)) \
+ { \
+ strcpy(var, \
+ option.isset ? option.values.string_val : \
+ ((relopt_string *) option.gen)->default_val); \
+ continue; \
+ }
+
+ extern void initialize_reloptions(void);
+
+ extern int add_reloption_kind(void);
+ extern void add_bool_reloption(int kind, char *name, char *desc,
+ bool default_val);
+ extern void add_int_reloption(int kind, char *name, char *desc,
+ int default_val, int min_val, int max_val);
+ extern void add_real_reloption(int kind, char *name, char *desc,
+ double default_val, double min_val, double max_val);
+ extern void add_string_reloption(int kind, char *name, char *desc,
+ char *default_val);
+
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);
extern bytea *heap_reloptions(char relkind, Datum reloptions, bool validate);
extern bytea *index_reloptions(RegProcedure amoptions, Datum reloptions,
! bool validate);
#endif /* RELOPTIONS_H */
Re: generic reloptions improvement
On Sun, Jan 4, 2009 at 15:01, Alvaro Herrera <alvherre@commandprompt.com> wrote:
Alvaro Herrera wrote:
Okay, it was basically fine except for the attached minor correction.
Warning: I intend to commit this patch fairly soon!This is the patch in its final form. I have included a few macros to
simplify the writing of amoptions routines.
Looks good to me, I just used it to whip up a patch to control some
toast compression knobs..
Re: generic reloptions improvement
Alvaro Herrera <alvherre@commandprompt.com> writes:
This is the patch in its final form. I have included a few macros to
simplify the writing of amoptions routines.
Minor gripes:
* Does initialize_reloptions() need to be exported? It seems to be
only called within parseRelOptions(). It's far from clear who else
should be expected to call it.
* The HANDLE_ macros are dangerous as-is (dangling if/else). Need to
use the usual do/while trick.
regards, tom lane
Re: generic reloptions improvement
Alvaro Herrera wrote:
Alvaro Herrera wrote:
Okay, it was basically fine except for the attached minor correction.
Warning: I intend to commit this patch fairly soon!This is the patch in its final form. I have included a few macros to
simplify the writing of amoptions routines.
Thanks for your efforts!
However, I could find a few issues about string reloptions.
(1) Who/Where should allocate a string area?
+ /* Note that this assumes that the variable is already allocated! */
+ #define HANDLE_STRING_RELOPTION(optname, var, option) \
+ if (HAVE_RELOPTION(optname, option)) \
+ { \
+ strcpy(var, \
+ option.isset ? option.values.string_val : \
+ ((relopt_string *) option.gen)->default_val); \
+ continue; \
+ }I think HANDLE_STRING_RELOPTION() should allocate a string area via
pstrdup(). If we have to put individual pstrdup() for each reloptions,
it will make noisy listing of codes.
How do you think:
#define HANDLE_STRING_RELOPTION(optname, var, option) \
if (HAVE_RELOPTION(optname, option)) \
{ \
char *tmp = (option.isset ? option.values.string_val : \
((relopt_string *) option.gen)->default_val); \
var = pstrdup(tmp); \
continue; \
}
(2) How does it represent NULL in string_option?
It seems to me we cannot represent a NULL string in the default.
Is it possible to add a mark to indicate NULL, like "bool default_null"
within struct relopt_string?
(3) heap_reloptions() from RelationParseRelOptions() makes a trouble.
It invokes heap_reloptions() under CurrentMemoryContext, and its result
is copied in CacheMemoryContext later, if the result is not NULL.
But it makes a matter when StdRdOptions contains a pointer.
If a string allocated under CurrentMemoryContext, target of the copied
pointer is not valid on the next query execution, even if StdRdOptions
is valid because it is copied to another memoery context.
I think RelationParseRelOptions() should be patched as follows:
/* Parse into appropriate format; don't error out here */
+ oldctx = MemoryContextSwitchTo(CacheMemoryContext);
switch (relation->rd_rel->relkind)
{
case RELKIND_RELATION:
case RELKIND_TOASTVALUE:
case RELKIND_UNCATALOGED:
options = heap_reloptions(relation->rd_rel->relkind, datum,
false);
break;
case RELKIND_INDEX:
options = index_reloptions(relation->rd_am->amoptions, datum,
false);
break;
default:
Assert(false); /* can't get here */
options = NULL; /* keep compiler quiet */
break;
}
+ MemoryContextSwitchTo(oldctx);- /* Copy parsed data into CacheMemoryContext */
- if (options)
- {
- relation->rd_options = MemoryContextAlloc(CacheMemoryContext,
- VARSIZE(options));
- memcpy(relation->rd_options, options, VARSIZE(options));
- }
Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>
Re: generic reloptions improvement
KaiGai Kohei wrote:
(1) Who/Where should allocate a string area?
+ /* Note that this assumes that the variable is already allocated! */ + #define HANDLE_STRING_RELOPTION(optname, var, option) \ + if (HAVE_RELOPTION(optname, option)) \ + { \ + strcpy(var, \ + option.isset ? option.values.string_val : \ + ((relopt_string *) option.gen)->default_val); \ + continue; \ + }I think HANDLE_STRING_RELOPTION() should allocate a string area via
pstrdup(). If we have to put individual pstrdup() for each reloptions,
it will make noisy listing of codes.How do you think:
#define HANDLE_STRING_RELOPTION(optname, var, option) \
if (HAVE_RELOPTION(optname, option)) \
{ \
char *tmp = (option.isset ? option.values.string_val : \
((relopt_string *) option.gen)->default_val); \
var = pstrdup(tmp); \
continue; \
}
Well, that's precisely the problem with string options. If we want
memory to be freed properly, we can only allocate a single chunk which
is what's going to be stored under the rd_options bytea pointer.
Allocating separately doesn't work because we need to rebuild the
relcache entry (freeing it and allocating a new one) when it is
invalidated for whatever reason. Since the relcache code cannot follow
a pointer stored in the bytea area, this would result in a permanent
memory leak.
So the rule I came up with is that the caller is responsible for
allocating it -- but it must be inside the bytea area to be returned.
Below is a sample amoptions routine to show how it works. Note that
this is exactly why I said that only a single string option can be
supported.
If you have a better idea, I'm all ears.
(2) How does it represent NULL in string_option?
It seems to me we cannot represent a NULL string in the default.
Is it possible to add a mark to indicate NULL, like "bool default_null"
within struct relopt_string?
Ah, good point. I'll have a look at this.
(3) heap_reloptions() from RelationParseRelOptions() makes a trouble.
This is the same as (1) actually.
--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.
Attachments:
Re: generic reloptions improvement
Alex Hunsaker escribi�:
On Sun, Jan 4, 2009 at 15:01, Alvaro Herrera <alvherre@commandprompt.com> wrote:
Alvaro Herrera wrote:
Okay, it was basically fine except for the attached minor correction.
Warning: I intend to commit this patch fairly soon!This is the patch in its final form. I have included a few macros to
simplify the writing of amoptions routines.Looks good to me, I just used it to whip up a patch to control some
toast compression knobs..
Excellent, thanks for testing :-)
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support
Re: generic reloptions improvement
Alvaro Herrera wrote:
KaiGai Kohei wrote:
(1) Who/Where should allocate a string area?
+ /* Note that this assumes that the variable is already allocated! */ + #define HANDLE_STRING_RELOPTION(optname, var, option) \ + if (HAVE_RELOPTION(optname, option)) \ + { \ + strcpy(var, \ + option.isset ? option.values.string_val : \ + ((relopt_string *) option.gen)->default_val); \ + continue; \ + }I think HANDLE_STRING_RELOPTION() should allocate a string area via
pstrdup(). If we have to put individual pstrdup() for each reloptions,
it will make noisy listing of codes.How do you think:
#define HANDLE_STRING_RELOPTION(optname, var, option) \
if (HAVE_RELOPTION(optname, option)) \
{ \
char *tmp = (option.isset ? option.values.string_val : \
((relopt_string *) option.gen)->default_val); \
var = pstrdup(tmp); \
continue; \
}Well, that's precisely the problem with string options. If we want
memory to be freed properly, we can only allocate a single chunk which
is what's going to be stored under the rd_options bytea pointer.
Allocating separately doesn't work because we need to rebuild the
relcache entry (freeing it and allocating a new one) when it is
invalidated for whatever reason. Since the relcache code cannot follow
a pointer stored in the bytea area, this would result in a permanent
memory leak.So the rule I came up with is that the caller is responsible for
allocating it -- but it must be inside the bytea area to be returned.
Below is a sample amoptions routine to show how it works. Note that
this is exactly why I said that only a single string option can be
supported.
If the caller allocates a surplus area to store string on tail of the
StdRdOptions (or others), the string option can be represented as an
offset value and should be accessed via macros, like:
struct StdRdOptions
{
int32 vl_len_;
int fillfactor;
int test_option_a; /* indicate offset of the surplus area from head */
int test_option_b; /* of this structure. */
/* in actually surplus area is allocated here */
};
#define GetOptionString(ptr, ofs) (ofs==0 ? NULL : ((char *)(ptr) + (ptr)->(ofs)))
We can access string options as follows:
elog(NOTICE, "test_option_a is %s", GetOptionString(RdOpts, test_option_a));
elog(NOTICE, "test_option_b is %s", GetOptionString(RdOpts, test_option_b));
It enables to store multiple string options, and represent NULL string.
If possible, HANDLE_STRING_RELOPTION() should be able to manage the head of unused
surplus area and put the given val its offset automatically.
I think using a macro to access string option is more proper restriction than
mutually exclusive string options.
Thanks,
--
OSS Platform Development Division, NEC
KaiGai Kohei <kaigai@ak.jp.nec.com>
Re: generic reloptions improvement
KaiGai Kohei wrote:
If the caller allocates a surplus area to store string on tail of the
StdRdOptions (or others), the string option can be represented as an
offset value and should be accessed via macros, like:
Excellent. I thought about storing an offset but I didn't know how to
make it work for accessors -- I was missing the macro idea :-) Thanks,
I'll see about this.
--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support