Hi.
I have refactored patch by introducing new struct relop_enum_element to make it
possible to use existing C-enum values in option's definition. So, additional
enum GIST_OPTION_BUFFERING_XXX was removed.
Also default option value should be placed now in the first element of
allowed_values[]. This helps not to expose default values definitions (like
GIST_BUFFERING_AUTO defined in gistbuild.c).
My github repository:
https://github.com/glukhovn/postgres/tree/enum-reloptions
--
Nikita Glukhov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 46276ce..7301ed7 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -404,7 +404,16 @@ static relopt_real realRelOpts[] =
{{NULL}}
};
-static relopt_string stringRelOpts[] =
+static relopt_enum_element
+view_check_option_enum[] =
+{
+ { RELOPT_ENUM_DEFAULT, VIEW_OPTION_CHECK_OPTION_NOT_SET },
+ { "local", VIEW_OPTION_CHECK_OPTION_LOCAL },
+ { "cascaded", VIEW_OPTION_CHECK_OPTION_CASCADED },
+ { NULL, 0 }
+};
+
+static relopt_enum enumRelOpts[] =
{
{
{
@@ -413,10 +422,7 @@ static relopt_string stringRelOpts[] =
RELOPT_KIND_GIST,
AccessExclusiveLock
},
- 4,
- false,
- gistValidateBufferingOption,
- "auto"
+ gist_buffering_option_enum
},
{
{
@@ -425,11 +431,13 @@ static relopt_string stringRelOpts[] =
RELOPT_KIND_VIEW,
AccessExclusiveLock
},
- 0,
- true,
- validateWithCheckOption,
- NULL
+ view_check_option_enum
},
+ {{NULL}}
+};
+
+static relopt_string stringRelOpts[] =
+{
/* list terminator */
{{NULL}}
};
@@ -476,6 +484,12 @@ initialize_reloptions(void)
realRelOpts[i].gen.lockmode));
j++;
}
+ for (i = 0; enumRelOpts[i].gen.name; i++)
+ {
+ Assert(DoLockModesConflict(enumRelOpts[i].gen.lockmode,
+ enumRelOpts[i].gen.lockmode));
+ j++;
+ }
for (i = 0; stringRelOpts[i].gen.name; i++)
{
Assert(DoLockModesConflict(stringRelOpts[i].gen.lockmode,
@@ -514,6 +528,14 @@ initialize_reloptions(void)
j++;
}
+ for (i = 0; enumRelOpts[i].gen.name; i++)
+ {
+ relOpts[j] = &enumRelOpts[i].gen;
+ relOpts[j]->type = RELOPT_TYPE_ENUM;
+ relOpts[j]->namelen = strlen(relOpts[j]->name);
+ j++;
+ }
+
for (i = 0; stringRelOpts[i].gen.name; i++)
{
relOpts[j] = &stringRelOpts[i].gen;
@@ -611,6 +633,9 @@ allocate_reloption(bits32 kinds, int type, const char *name, const char *desc)
case RELOPT_TYPE_REAL:
size = sizeof(relopt_real);
break;
+ case RELOPT_TYPE_ENUM:
+ size = sizeof(relopt_enum);
+ break;
case RELOPT_TYPE_STRING:
size = sizeof(relopt_string);
break;
@@ -690,6 +715,23 @@ add_real_reloption(bits32 kinds, const char *name, const char *desc, double defa
}
/*
+ * add_enuum_reloption
+ * Add a new enum reloption
+ */
+void
+add_enum_reloption(bits32 kinds, const char *name, const char *desc,
+ relopt_enum_element *allowed_values)
+{
+ relopt_enum *newoption;
+
+ newoption = (relopt_enum *) allocate_reloption(kinds, RELOPT_TYPE_ENUM,
+ name, desc);
+ newoption->allowed_values = allowed_values;
+
+ add_reloption((relopt_gen *) newoption);
+}
+
+/*
* add_string_reloption
* Add a new string reloption
*
@@ -1190,6 +1232,49 @@ parse_one_reloption(relopt_value *option, char *text_str, int text_len,
optreal->min, optreal->max)));
}
break;
+ case RELOPT_TYPE_ENUM:
+ {
+ relopt_enum *opt_enum = (relopt_enum *) option->gen;
+ relopt_enum_element *elem = opt_enum->allowed_values;
+
+ parsed = false;
+
+ for (elem = opt_enum->allowed_values; elem->name; elem++)
+ {
+ if (elem->name != RELOPT_ENUM_DEFAULT &&
+ !pg_strcasecmp(value, elem->name))
+ {
+ option->values.enum_val = elem->value;
+ parsed = true;
+ break;
+ }
+ }
+
+ if (validate && !parsed)
+ {
+ StringInfoData buf;
+
+ initStringInfo(&buf);
+
+ for (elem = opt_enum->allowed_values; elem->name; elem++)
+ {
+ if (elem->name == RELOPT_ENUM_DEFAULT)
+ continue; /* should be the first in the list */
+
+ appendStringInfo(&buf, "\"%s\"", elem->name);
+
+ if (elem[1].name)
+ appendStringInfo(&buf, elem[2].name ? ", " : " and ");
+ }
+
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid value for \"%s\" option",
+ option->gen->name),
+ errdetail("Valid values are %s.", /*str*/ buf.data)));
+ }
+ }
+ break;
case RELOPT_TYPE_STRING:
{
relopt_string *optstring = (relopt_string *) option->gen;
@@ -1283,6 +1368,11 @@ fillRelOptions(void *rdopts, Size basesize,
options[i].values.real_val :
((relopt_real *) options[i].gen)->default_val;
break;
+ case RELOPT_TYPE_ENUM:
+ *(int *) itempos = options[i].isset ?
+ options[i].values.enum_val :
+ ((relopt_enum *) options[i].gen)->allowed_values[0].value;
+ break;
case RELOPT_TYPE_STRING:
optstring = (relopt_string *) options[i].gen;
if (options[i].isset)
@@ -1393,8 +1483,8 @@ view_reloptions(Datum reloptions, bool validate)
static const relopt_parse_elt tab[] = {
{"security_barrier", RELOPT_TYPE_BOOL,
offsetof(ViewOptions, security_barrier)},
- {"check_option", RELOPT_TYPE_STRING,
- offsetof(ViewOptions, check_option_offset)}
+ {"check_option", RELOPT_TYPE_ENUM,
+ offsetof(ViewOptions, check_option)}
};
options = parseRelOptions(reloptions, validate, RELOPT_KIND_VIEW, &numoptions);
diff --git a/src/backend/access/gist/gistbuild.c b/src/backend/access/gist/gistbuild.c
index 434f15f..a5ca2c3 100644
--- a/src/backend/access/gist/gistbuild.c
+++ b/src/backend/access/gist/gistbuild.c
@@ -74,6 +74,16 @@ typedef struct
GistBufferingMode bufferingMode;
} GISTBuildState;
+/* Definition of GiST enum option 'buffering' */
+relopt_enum_element
+gist_buffering_option_enum[] =
+{
+ { "auto", GIST_BUFFERING_AUTO }, /* default */
+ { "on", GIST_BUFFERING_STATS },
+ { "off", GIST_BUFFERING_DISABLED },
+ { NULL, 0 }
+};
+
/* prototypes for private functions */
static void gistInitBuffering(GISTBuildState *buildstate);
static int calculatePagesPerBuffer(GISTBuildState *buildstate, int levelStep);
@@ -126,15 +136,8 @@ gistbuild(Relation heap, Relation index, IndexInfo *indexInfo)
{
/* Get buffering mode from the options string */
GiSTOptions *options = (GiSTOptions *) index->rd_options;
- char *bufferingMode = (char *) options + options->bufferingModeOffset;
-
- if (strcmp(bufferingMode, "on") == 0)
- buildstate.bufferingMode = GIST_BUFFERING_STATS;
- else if (strcmp(bufferingMode, "off") == 0)
- buildstate.bufferingMode = GIST_BUFFERING_DISABLED;
- else
- buildstate.bufferingMode = GIST_BUFFERING_AUTO;
+ buildstate.bufferingMode = options->bufferingMode;
fillfactor = options->fillfactor;
}
else
@@ -234,25 +237,6 @@ gistbuild(Relation heap, Relation index, IndexInfo *indexInfo)
}
/*
- * Validator for "buffering" reloption on GiST indexes. Allows "on", "off"
- * and "auto" values.
- */
-void
-gistValidateBufferingOption(const char *value)
-{
- if (value == NULL ||
- (strcmp(value, "on") != 0 &&
- strcmp(value, "off") != 0 &&
- strcmp(value, "auto") != 0))
- {
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("invalid value for \"buffering\" option"),
- errdetail("Valid values are \"on\", \"off\", and \"auto\".")));
- }
-}
-
-/*
* Attempt to switch to buffering mode.
*
* If there is not enough memory for buffering build, sets bufferingMode
diff --git a/src/backend/access/gist/gistutil.c b/src/backend/access/gist/gistutil.c
index 55cccd2..389ecb7 100644
--- a/src/backend/access/gist/gistutil.c
+++ b/src/backend/access/gist/gistutil.c
@@ -840,7 +840,7 @@ gistoptions(Datum reloptions, bool validate)
int numoptions;
static const relopt_parse_elt tab[] = {
{"fillfactor", RELOPT_TYPE_INT, offsetof(GiSTOptions, fillfactor)},
- {"buffering", RELOPT_TYPE_STRING, offsetof(GiSTOptions, bufferingModeOffset)}
+ {"buffering", RELOPT_TYPE_ENUM, offsetof(GiSTOptions, bufferingMode)}
};
options = parseRelOptions(reloptions, validate, RELOPT_KIND_GIST,
diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c
index 7d4511c..5de0654 100644
--- a/src/backend/commands/view.c
+++ b/src/backend/commands/view.c
@@ -39,24 +39,6 @@
static void checkViewTupleDesc(TupleDesc newdesc, TupleDesc olddesc);
/*---------------------------------------------------------------------
- * Validator for "check_option" reloption on views. The allowed values
- * are "local" and "cascaded".
- */
-void
-validateWithCheckOption(const char *value)
-{
- if (value == NULL ||
- (strcmp(value, "local") != 0 &&
- strcmp(value, "cascaded") != 0))
- {
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("invalid value for \"check_option\" option"),
- errdetail("Valid values are \"local\" and \"cascaded\".")));
- }
-}
-
-/*---------------------------------------------------------------------
* DefineVirtualRelation
*
* Create a view relation and use the rules system to store the query
diff --git a/src/include/access/gist_private.h b/src/include/access/gist_private.h
index 36ed724..796264d 100644
--- a/src/include/access/gist_private.h
+++ b/src/include/access/gist_private.h
@@ -17,6 +17,7 @@
#include "access/amapi.h"
#include "access/gist.h"
#include "access/itup.h"
+#include "access/reloptions.h"
#include "fmgr.h"
#include "lib/pairingheap.h"
#include "storage/bufmgr.h"
@@ -368,13 +369,18 @@ typedef struct GISTBuildBuffers
} GISTBuildBuffers;
/*
+ * Definition of items of GiST enum option 'buffering'.
+ */
+extern relopt_enum_element gist_buffering_option_enum[];
+
+/*
* Storage type for GiST's reloptions
*/
typedef struct GiSTOptions
{
int32 vl_len_; /* varlena header (do not touch directly!) */
int fillfactor; /* page fill factor in percent (0..100) */
- int bufferingModeOffset; /* use buffering build? */
+ int bufferingMode; /* use buffering build? */
} GiSTOptions;
/* gist.c */
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index b32c1e9..68a10a4 100644
--- a/src/include/access/reloptions.h
+++ b/src/include/access/reloptions.h
@@ -31,6 +31,7 @@ typedef enum relopt_type
RELOPT_TYPE_BOOL,
RELOPT_TYPE_INT,
RELOPT_TYPE_REAL,
+ RELOPT_TYPE_ENUM,
RELOPT_TYPE_STRING
} relopt_type;
@@ -80,6 +81,7 @@ typedef struct relopt_value
bool bool_val;
int int_val;
double real_val;
+ int enum_val;
char *string_val; /* allocated separately */
} values;
} relopt_value;
@@ -107,6 +109,22 @@ typedef struct relopt_real
double max;
} relopt_real;
+typedef struct relopt_enum_element
+{
+ const char *name;
+ int value;
+} relopt_enum_element;
+
+#define RELOPT_ENUM_DEFAULT ((const char *) -1) /* pseudo-name for default value */
+
+typedef struct relopt_enum
+{
+ relopt_gen gen;
+ relopt_enum_element *allowed_values; /* Null terminated array of allowed
+ * values for the option. Its first
+ * element contains default value. */
+} relopt_enum;
+
/* validation routines for strings */
typedef void (*validate_string_relopt) (const char *value);
@@ -252,6 +270,8 @@ extern void add_int_reloption(bits32 kinds, const char *name, const char *desc,
int default_val, int min_val, int max_val);
extern void add_real_reloption(bits32 kinds, const char *name, const char *desc,
double default_val, double min_val, double max_val);
+extern void add_enum_reloption(bits32 kinds, const char *name, const char *desc,
+ relopt_enum_element *allowed_values);
extern void add_string_reloption(bits32 kinds, const char *name, const char *desc,
const char *default_val, validate_string_relopt validator);
diff --git a/src/include/commands/view.h b/src/include/commands/view.h
index 4703922..50d45a5 100644
--- a/src/include/commands/view.h
+++ b/src/include/commands/view.h
@@ -17,8 +17,6 @@
#include "catalog/objectaddress.h"
#include "nodes/parsenodes.h"
-extern void validateWithCheckOption(const char *value);
-
extern ObjectAddress DefineView(ViewStmt *stmt, const char *queryString,
int stmt_location, int stmt_len);
diff --git a/src/include/utils/rel.h b/src/include/utils/rel.h
index aa8add5..5e7b60d 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -336,6 +336,13 @@ typedef struct StdRdOptions
((relation)->rd_options ? \
((StdRdOptions *) (relation)->rd_options)->parallel_workers : (defaultpw))
+typedef enum view_option_check_option_value_numbers
+{
+ VIEW_OPTION_CHECK_OPTION_NOT_SET = -1,
+ VIEW_OPTION_CHECK_OPTION_LOCAL = 0,
+ VIEW_OPTION_CHECK_OPTION_CASCADED = 1,
+} view_option_check_option_value_numbers;
+
/*
* ViewOptions
@@ -345,7 +352,7 @@ typedef struct ViewOptions
{
int32 vl_len_; /* varlena header (do not touch directly!) */
bool security_barrier;
- int check_option_offset;
+ int check_option;
} ViewOptions;
/*
@@ -364,7 +371,8 @@ typedef struct ViewOptions
*/
#define RelationHasCheckOption(relation) \
((relation)->rd_options && \
- ((ViewOptions *) (relation)->rd_options)->check_option_offset != 0)
+ ((ViewOptions *) (relation)->rd_options)->check_option != \
+ VIEW_OPTION_CHECK_OPTION_NOT_SET)
/*
* RelationHasLocalCheckOption
@@ -373,10 +381,8 @@ typedef struct ViewOptions
*/
#define RelationHasLocalCheckOption(relation) \
((relation)->rd_options && \
- ((ViewOptions *) (relation)->rd_options)->check_option_offset != 0 ? \
- strcmp((char *) (relation)->rd_options + \
- ((ViewOptions *) (relation)->rd_options)->check_option_offset, \
- "local") == 0 : false)
+ ((ViewOptions *) (relation)->rd_options)->check_option == \
+ VIEW_OPTION_CHECK_OPTION_LOCAL)
/*
* RelationHasCascadedCheckOption
@@ -385,11 +391,8 @@ typedef struct ViewOptions
*/
#define RelationHasCascadedCheckOption(relation) \
((relation)->rd_options && \
- ((ViewOptions *) (relation)->rd_options)->check_option_offset != 0 ? \
- strcmp((char *) (relation)->rd_options + \
- ((ViewOptions *) (relation)->rd_options)->check_option_offset, \
- "cascaded") == 0 : false)
-
+ ((ViewOptions *) (relation)->rd_options)->check_option == \
+ VIEW_OPTION_CHECK_OPTION_CASCADED)
/*
* RelationIsValid
diff --git a/src/test/regress/expected/gist.out b/src/test/regress/expected/gist.out
index f5a2993..c0ced28 100644
--- a/src/test/regress/expected/gist.out
+++ b/src/test/regress/expected/gist.out
@@ -13,7 +13,7 @@ drop index gist_pointidx2, gist_pointidx3, gist_pointidx4;
-- Make sure bad values are refused
create index gist_pointidx5 on gist_point_tbl using gist(p) with (buffering = invalid_value);
ERROR: invalid value for "buffering" option
-DETAIL: Valid values are "on", "off", and "auto".
+DETAIL: Valid values are "auto", "on" and "off".
create index gist_pointidx5 on gist_point_tbl using gist(p) with (fillfactor=9);
ERROR: value 9 out of bounds for option "fillfactor"
DETAIL: Valid values are between "10" and "100".
