From: Nikolay Shaplov <dhyan@nataraj.su>
To: pgsql-hackers@lists.postgresql.org
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, pgsql-hackers@postgresql.org
Subject: Re: [PATCH][PROPOSAL] Add enum releation option type
Date: Thu, 15 Feb 2018 00:06:36 +0300
Message-ID: <9521565.ljkCOhfioU@x200m>
User-Agent: KMail/4.14.1 (Linux/3.16.0-4-amd64; KDE/4.14.2; x86_64; ; )
In-Reply-To: <20180209214529.mbz6iqji7v436xde@alvherre.pgsql>
References: <20180209214529.mbz6iqji7v436xde@alvherre.pgsql>
MIME-Version: 1.0
Content-Type: multipart/signed; boundary="nextPart2054907.g8nqithbEE";
 micalg="pgp-sha256"; protocol="application/pgp-signature"
Precedence: bulk


--nextPart2054907.g8nqithbEE
Content-Type: multipart/mixed; boundary="nextPart1613619.bdSzQO9meB"
Content-Transfer-Encoding: quoted-printable

This is a multi-part message in MIME format.

=2D-nextPart1613619.bdSzQO9meB
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="utf-8"

=D0=92 =D0=BF=D0=B8=D1=81=D1=8C=D0=BC=D0=B5 =D0=BE=D1=82 9 =D1=84=D0=B5=
=D0=B2=D1=80=D0=B0=D0=BB=D1=8F 2018 18:45:29 =D0=BF=D0=BE=D0=BB=D1=8C=D0=
=B7=D0=BE=D0=B2=D0=B0=D1=82=D0=B5=D0=BB=D1=8C Alvaro Herrera =D0=BD=D0=B0=
=D0=BF=D0=B8=D1=81=D0=B0=D0=BB:

> > 1. I've changed error message from 'Valid values are "XXX", "YYY" a=
nd
> > "ZZZ".' to 'Valid values are "XXX", "YYY", "ZZZ".' to make a code a=
 bit
> > simpler. If it is not acceptable, please let me know, I will add "a=
nd" to
> > the string.
> I don't think we care about this, but is this still the case if you u=
se
> a stringinfo?
>=20
> > 2. Also about the string with the list of acceptable values: the co=
de that
> > creates this string is inside parse_one_reloption function now.
>=20
> I think you could save most of that mess by using appendStringInfo an=
d
> friends.

Here is the second verion of the patch where I use appendStringInfo to =
prepare=20
error message. The code is much more simple here now, and it now create=
 value=20
list with "and" at the end: '"xxx", "yyy" and "zzz"'


=2D-=20
Do code for fun.
=2D-nextPart1613619.bdSzQO9meB
Content-Disposition: attachment; filename="enum-reloptions2.diff"
Content-Transfer-Encoding: 7Bit
Content-Type: text/x-patch; charset="UTF-8"; name="enum-reloptions2.diff"

diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 46276ce..82a0492 100644
=2D-- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -404,7 +404,9 @@ static relopt_real realRelOpts[] =
 	{{NULL}}
 };
 
=2Dstatic relopt_string stringRelOpts[] =
+static const char *gist_buffering_enum_names[] = GIST_OPTION_BUFFERING_VALUE_NAMES;
+static const char *view_check_option_names[] = VIEW_OPTION_CHECK_OPTION_VALUE_NAMES;
+static relopt_enum enumRelOpts[] =
 {
 	{
 		{
@@ -413,10 +415,8 @@ static relopt_string stringRelOpts[] =
 			RELOPT_KIND_GIST,
 			AccessExclusiveLock
 		},
=2D		4,
=2D		false,
=2D		gistValidateBufferingOption,
=2D		"auto"
+		gist_buffering_enum_names,
+		GIST_OPTION_BUFFERING_AUTO
 	},
 	{
 		{
@@ -425,11 +425,14 @@ static relopt_string stringRelOpts[] =
 			RELOPT_KIND_VIEW,
 			AccessExclusiveLock
 		},
=2D		0,
=2D		true,
=2D		validateWithCheckOption,
=2D		NULL
+		view_check_option_names,
+		VIEW_OPTION_CHECK_OPTION_NOT_SET
 	},
+	{{NULL}}
+};
+
+static relopt_string stringRelOpts[] =
+{
 	/* list terminator */
 	{{NULL}}
 };
@@ -476,6 +479,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 +523,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 +628,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 +710,24 @@ 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,
+					 const char **allowed_values, int default_val)
+{
+	relopt_enum *newoption;
+
+	newoption = (relopt_enum *) allocate_reloption(kinds, RELOPT_TYPE_ENUM,
+												   name, desc);
+	newoption->allowed_values = allowed_values;
+	newoption->default_val = default_val;
+
+	add_reloption((relopt_gen *) newoption);
+}
+
+/*
  * add_string_reloption
  *		Add a new string reloption
  *
@@ -1190,6 +1228,50 @@ 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;
+				int			i = 0;
+
+				parsed = false;
+				while (opt_enum->allowed_values[i])
+				{
+					if (pg_strcasecmp(value, opt_enum->allowed_values[i]) == 0)
+					{
+						option->values.enum_val = i;
+						parsed = true;
+						break;
+					}
+					i++;
+				}
+				if (!parsed)
+				{
+					StringInfoData buf;
+
+					initStringInfo(&buf);
+					i = 0;
+					while (opt_enum->allowed_values[i])
+					{
+						appendStringInfo(&buf,"\"%s\"",
+										 opt_enum->allowed_values[i]);
+						if (opt_enum->allowed_values[i + 1])
+						{
+							if (opt_enum->allowed_values[i + 2])
+								appendStringInfo(&buf,", ");
+							else
+								appendStringInfo(&buf," and ");
+						}
+						i++;
+					}
+					ereport(ERROR,
+							(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+							 errmsg("invalid value for \"%s\" option",
+									option->gen->name),
+							 errdetail("Valid values are %s.", /*str*/ buf.data)));
+					pfree(buf.data);
+				}
+			}
+			break;
 		case RELOPT_TYPE_STRING:
 			{
 				relopt_string *optstring = (relopt_string *) option->gen;
@@ -1283,6 +1365,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)->default_val;
+						break;
 					case RELOPT_TYPE_STRING:
 						optstring = (relopt_string *) options[i].gen;
 						if (options[i].isset)
@@ -1393,8 +1480,8 @@ view_reloptions(Datum reloptions, bool validate)
 	static const relopt_parse_elt tab[] = {
 		{"security_barrier", RELOPT_TYPE_BOOL,
 		offsetof(ViewOptions, security_barrier)},
=2D		{"check_option", RELOPT_TYPE_STRING,
=2D		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..02c630d 100644
=2D-- a/src/backend/access/gist/gistbuild.c
+++ b/src/backend/access/gist/gistbuild.c
@@ -126,11 +126,10 @@ gistbuild(Relation heap, Relation index, IndexInfo *indexInfo)
 	{
 		/* Get buffering mode from the options string */
 		GiSTOptions *options = (GiSTOptions *) index->rd_options;
=2D		char	   *bufferingMode = (char *) options + options->bufferingModeOffset;
 
=2D		if (strcmp(bufferingMode, "on") == 0)
+		if (options->buffering_mode == GIST_OPTION_BUFFERING_ON)
 			buildstate.bufferingMode = GIST_BUFFERING_STATS;
=2D		else if (strcmp(bufferingMode, "off") == 0)
+		else if (options->buffering_mode == GIST_OPTION_BUFFERING_OFF)
 			buildstate.bufferingMode = GIST_BUFFERING_DISABLED;
 		else
 			buildstate.bufferingMode = GIST_BUFFERING_AUTO;
@@ -234,25 +233,6 @@ gistbuild(Relation heap, Relation index, IndexInfo *indexInfo)
 }
 
 /*
=2D * Validator for "buffering" reloption on GiST indexes. Allows "on", "off"
=2D * and "auto" values.
=2D */
=2Dvoid
=2DgistValidateBufferingOption(const char *value)
=2D{
=2D	if (value == NULL ||
=2D		(strcmp(value, "on") != 0 &&
=2D		 strcmp(value, "off") != 0 &&
=2D		 strcmp(value, "auto") != 0))
=2D	{
=2D		ereport(ERROR,
=2D				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
=2D				 errmsg("invalid value for \"buffering\" option"),
=2D				 errdetail("Valid values are \"on\", \"off\", and \"auto\".")));
=2D	}
=2D}
=2D
=2D/*
  * 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..fe49f53 100644
=2D-- 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)},
=2D		{"buffering", RELOPT_TYPE_STRING, offsetof(GiSTOptions, bufferingModeOffset)}
+		{"buffering", RELOPT_TYPE_ENUM, offsetof(GiSTOptions, buffering_mode)}
 	};
 
 	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
=2D-- a/src/backend/commands/view.c
+++ b/src/backend/commands/view.c
@@ -39,24 +39,6 @@
 static void checkViewTupleDesc(TupleDesc newdesc, TupleDesc olddesc);
 
 /*---------------------------------------------------------------------
=2D * Validator for "check_option" reloption on views. The allowed values
=2D * are "local" and "cascaded".
=2D */
=2Dvoid
=2DvalidateWithCheckOption(const char *value)
=2D{
=2D	if (value == NULL ||
=2D		(strcmp(value, "local") != 0 &&
=2D		 strcmp(value, "cascaded") != 0))
=2D	{
=2D		ereport(ERROR,
=2D				(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
=2D				 errmsg("invalid value for \"check_option\" option"),
=2D				 errdetail("Valid values are \"local\" and \"cascaded\".")));
=2D	}
=2D}
=2D
=2D/*---------------------------------------------------------------------
  * 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..d492ad4 100644
=2D-- a/src/include/access/gist_private.h
+++ b/src/include/access/gist_private.h
@@ -368,13 +368,30 @@ typedef struct GISTBuildBuffers
 } GISTBuildBuffers;
 
 /*
+ * Definition of items of enum type. Names and codes. To add or modify item
+ * edit both lists
+ */
+#define GIST_OPTION_BUFFERING_VALUE_NAMES { \
+	"on",									\
+	"off",									\
+	"auto",									\
+	(const char *) NULL						\
+}
+typedef enum gist_option_buffering_value_numbers
+{
+	GIST_OPTION_BUFFERING_ON = 0,
+	GIST_OPTION_BUFFERING_OFF = 1,
+	GIST_OPTION_BUFFERING_AUTO = 2,
+}	gist_option_buffering_value_numbers;
+
+/*
  * 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) */
=2D	int			bufferingModeOffset;	/* use buffering build? */
+	int			buffering_mode; /* use buffering build? */
 } GiSTOptions;
 
 /* gist.c */
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index b32c1e9..068745f 100644
=2D-- 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,14 @@ typedef struct relopt_real
 	double		max;
 } relopt_real;
 
+typedef struct relopt_enum
+{
+	relopt_gen	gen;
+	const char **allowed_values;/* Null terminated array of allowed values for
+								 * the option */
+	int			default_val;	/* Number of item of allowed_values array */
+} relopt_enum;
+
 /* validation routines for strings */
 typedef void (*validate_string_relopt) (const char *value);
 
@@ -252,6 +262,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,
+					 const char **allowed_values, int default_val);
 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
=2D-- a/src/include/commands/view.h
+++ b/src/include/commands/view.h
@@ -17,8 +17,6 @@
 #include "catalog/objectaddress.h"
 #include "nodes/parsenodes.h"
 
=2Dextern void validateWithCheckOption(const char *value);
=2D
 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..9fe1afa 100644
=2D-- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -336,6 +336,22 @@ typedef struct StdRdOptions
 	((relation)->rd_options ? \
 	 ((StdRdOptions *) (relation)->rd_options)->parallel_workers : (defaultpw))
 
+/*
+ * Definition of items of enum type. Names and codes. To add or modify item
+ * edit both lists
+ */
+#define VIEW_OPTION_CHECK_OPTION_VALUE_NAMES {	\
+	"local",									\
+	"cascaded",									\
+	(const char *) NULL							\
+}
+
+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 +361,7 @@ typedef struct ViewOptions
 {
 	int32		vl_len_;		/* varlena header (do not touch directly!) */
 	bool		security_barrier;
=2D	int			check_option_offset;
+	int			check_option;
 } ViewOptions;
 
 /*
@@ -364,7 +380,8 @@ typedef struct ViewOptions
  */
 #define RelationHasCheckOption(relation)									\
 	((relation)->rd_options &&												\
=2D	 ((ViewOptions *) (relation)->rd_options)->check_option_offset != 0)
+	 ((ViewOptions *) (relation)->rd_options)->check_option !=				\
+	 VIEW_OPTION_CHECK_OPTION_NOT_SET)
 
 /*
  * RelationHasLocalCheckOption
@@ -373,10 +390,8 @@ typedef struct ViewOptions
  */
 #define RelationHasLocalCheckOption(relation)								\
 	((relation)->rd_options &&												\
=2D	 ((ViewOptions *) (relation)->rd_options)->check_option_offset != 0 ?	\
=2D	 strcmp((char *) (relation)->rd_options +								\
=2D			((ViewOptions *) (relation)->rd_options)->check_option_offset,	\
=2D			"local") == 0 : false)
+	 ((ViewOptions *) (relation)->rd_options)->check_option ==				\
+	 VIEW_OPTION_CHECK_OPTION_LOCAL)
 
 /*
  * RelationHasCascadedCheckOption
@@ -385,11 +400,8 @@ typedef struct ViewOptions
  */
 #define RelationHasCascadedCheckOption(relation)							\
 	((relation)->rd_options &&												\
=2D	 ((ViewOptions *) (relation)->rd_options)->check_option_offset != 0 ?	\
=2D	 strcmp((char *) (relation)->rd_options +								\
=2D			((ViewOptions *) (relation)->rd_options)->check_option_offset,	\
=2D			"cascaded") == 0 : false)
=2D
+	 ((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..e5cf179 100644
=2D-- 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
=2DDETAIL:  Valid values are "on", "off", and "auto".
+DETAIL:  Valid values are "on", "off" and "auto".
 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".

=2D-nextPart1613619.bdSzQO9meB--
This is a multi-part message in MIME format.

--nextPart1613619.bdSzQO9meB
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain; charset="utf-8"

=D0=92 =D0=BF=D0=B8=D1=81=D1=8C=D0=BC=D0=B5 =D0=BE=D1=82 9 =D1=84=D0=B5=
=D0=B2=D1=80=D0=B0=D0=BB=D1=8F 2018 18:45:29 =D0=BF=D0=BE=D0=BB=D1=8C=D0=
=B7=D0=BE=D0=B2=D0=B0=D1=82=D0=B5=D0=BB=D1=8C Alvaro Herrera =D0=BD=D0=B0=
=D0=BF=D0=B8=D1=81=D0=B0=D0=BB:

> > 1. I've changed error message from 'Valid values are "XXX", "YYY" a=
nd
> > "ZZZ".' to 'Valid values are "XXX", "YYY", "ZZZ".' to make a code a=
 bit
> > simpler. If it is not acceptable, please let me know, I will add "a=
nd" to
> > the string.
> I don't think we care about this, but is this still the case if you u=
se
> a stringinfo?
>=20
> > 2. Also about the string with the list of acceptable values: the co=
de that
> > creates this string is inside parse_one_reloption function now.
>=20
> I think you could save most of that mess by using appendStringInfo an=
d
> friends.

Here is the second verion of the patch where I use appendStringInfo to =
prepare=20
error message. The code is much more simple here now, and it now create=
 value=20
list with "and" at the end: '"xxx", "yyy" and "zzz"'


--=20
Do code for fun.
--nextPart1613619.bdSzQO9meB
Content-Disposition: attachment; filename="enum-reloptions2.diff"
Content-Transfer-Encoding: 7Bit
Content-Type: text/x-patch; charset="UTF-8"; name="enum-reloptions2.diff"

diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index 46276ce..82a0492 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -404,7 +404,9 @@ static relopt_real realRelOpts[] =
 	{{NULL}}
 };
 
-static relopt_string stringRelOpts[] =
+static const char *gist_buffering_enum_names[] = GIST_OPTION_BUFFERING_VALUE_NAMES;
+static const char *view_check_option_names[] = VIEW_OPTION_CHECK_OPTION_VALUE_NAMES;
+static relopt_enum enumRelOpts[] =
 {
 	{
 		{
@@ -413,10 +415,8 @@ static relopt_string stringRelOpts[] =
 			RELOPT_KIND_GIST,
 			AccessExclusiveLock
 		},
-		4,
-		false,
-		gistValidateBufferingOption,
-		"auto"
+		gist_buffering_enum_names,
+		GIST_OPTION_BUFFERING_AUTO
 	},
 	{
 		{
@@ -425,11 +425,14 @@ static relopt_string stringRelOpts[] =
 			RELOPT_KIND_VIEW,
 			AccessExclusiveLock
 		},
-		0,
-		true,
-		validateWithCheckOption,
-		NULL
+		view_check_option_names,
+		VIEW_OPTION_CHECK_OPTION_NOT_SET
 	},
+	{{NULL}}
+};
+
+static relopt_string stringRelOpts[] =
+{
 	/* list terminator */
 	{{NULL}}
 };
@@ -476,6 +479,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 +523,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 +628,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 +710,24 @@ 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,
+					 const char **allowed_values, int default_val)
+{
+	relopt_enum *newoption;
+
+	newoption = (relopt_enum *) allocate_reloption(kinds, RELOPT_TYPE_ENUM,
+												   name, desc);
+	newoption->allowed_values = allowed_values;
+	newoption->default_val = default_val;
+
+	add_reloption((relopt_gen *) newoption);
+}
+
+/*
  * add_string_reloption
  *		Add a new string reloption
  *
@@ -1190,6 +1228,50 @@ 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;
+				int			i = 0;
+
+				parsed = false;
+				while (opt_enum->allowed_values[i])
+				{
+					if (pg_strcasecmp(value, opt_enum->allowed_values[i]) == 0)
+					{
+						option->values.enum_val = i;
+						parsed = true;
+						break;
+					}
+					i++;
+				}
+				if (!parsed)
+				{
+					StringInfoData buf;
+
+					initStringInfo(&buf);
+					i = 0;
+					while (opt_enum->allowed_values[i])
+					{
+						appendStringInfo(&buf,"\"%s\"",
+										 opt_enum->allowed_values[i]);
+						if (opt_enum->allowed_values[i + 1])
+						{
+							if (opt_enum->allowed_values[i + 2])
+								appendStringInfo(&buf,", ");
+							else
+								appendStringInfo(&buf," and ");
+						}
+						i++;
+					}
+					ereport(ERROR,
+							(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+							 errmsg("invalid value for \"%s\" option",
+									option->gen->name),
+							 errdetail("Valid values are %s.", /*str*/ buf.data)));
+					pfree(buf.data);
+				}
+			}
+			break;
 		case RELOPT_TYPE_STRING:
 			{
 				relopt_string *optstring = (relopt_string *) option->gen;
@@ -1283,6 +1365,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)->default_val;
+						break;
 					case RELOPT_TYPE_STRING:
 						optstring = (relopt_string *) options[i].gen;
 						if (options[i].isset)
@@ -1393,8 +1480,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..02c630d 100644
--- a/src/backend/access/gist/gistbuild.c
+++ b/src/backend/access/gist/gistbuild.c
@@ -126,11 +126,10 @@ 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)
+		if (options->buffering_mode == GIST_OPTION_BUFFERING_ON)
 			buildstate.bufferingMode = GIST_BUFFERING_STATS;
-		else if (strcmp(bufferingMode, "off") == 0)
+		else if (options->buffering_mode == GIST_OPTION_BUFFERING_OFF)
 			buildstate.bufferingMode = GIST_BUFFERING_DISABLED;
 		else
 			buildstate.bufferingMode = GIST_BUFFERING_AUTO;
@@ -234,25 +233,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..fe49f53 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, buffering_mode)}
 	};
 
 	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..d492ad4 100644
--- a/src/include/access/gist_private.h
+++ b/src/include/access/gist_private.h
@@ -368,13 +368,30 @@ typedef struct GISTBuildBuffers
 } GISTBuildBuffers;
 
 /*
+ * Definition of items of enum type. Names and codes. To add or modify item
+ * edit both lists
+ */
+#define GIST_OPTION_BUFFERING_VALUE_NAMES { \
+	"on",									\
+	"off",									\
+	"auto",									\
+	(const char *) NULL						\
+}
+typedef enum gist_option_buffering_value_numbers
+{
+	GIST_OPTION_BUFFERING_ON = 0,
+	GIST_OPTION_BUFFERING_OFF = 1,
+	GIST_OPTION_BUFFERING_AUTO = 2,
+}	gist_option_buffering_value_numbers;
+
+/*
  * 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			buffering_mode; /* use buffering build? */
 } GiSTOptions;
 
 /* gist.c */
diff --git a/src/include/access/reloptions.h b/src/include/access/reloptions.h
index b32c1e9..068745f 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,14 @@ typedef struct relopt_real
 	double		max;
 } relopt_real;
 
+typedef struct relopt_enum
+{
+	relopt_gen	gen;
+	const char **allowed_values;/* Null terminated array of allowed values for
+								 * the option */
+	int			default_val;	/* Number of item of allowed_values array */
+} relopt_enum;
+
 /* validation routines for strings */
 typedef void (*validate_string_relopt) (const char *value);
 
@@ -252,6 +262,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,
+					 const char **allowed_values, int default_val);
 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..9fe1afa 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -336,6 +336,22 @@ typedef struct StdRdOptions
 	((relation)->rd_options ? \
 	 ((StdRdOptions *) (relation)->rd_options)->parallel_workers : (defaultpw))
 
+/*
+ * Definition of items of enum type. Names and codes. To add or modify item
+ * edit both lists
+ */
+#define VIEW_OPTION_CHECK_OPTION_VALUE_NAMES {	\
+	"local",									\
+	"cascaded",									\
+	(const char *) NULL							\
+}
+
+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 +361,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 +380,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 +390,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 +400,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..e5cf179 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 "on", "off" and "auto".
 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".

--nextPart1613619.bdSzQO9meB--

--nextPart2054907.g8nqithbEE
Content-Type: application/pgp-signature; name="signature.asc"
Content-Description: This is a digitally signed message part.
Content-Transfer-Encoding: 7Bit

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v2

iQEcBAABCAAGBQJahKTcAAoJEKc13HowK+H4mZoH/16HH9rbj+cvyfX8HixA3tiq
Dfqkc3bK8SdV978XfK0mWNhpqT1UKMsGhGlDSAadT11+pX5XeSRVIrDxXFQvVHtu
m0O8s1imAQVjap00wYMRwKRYE40cPvAix+fspelj1hihtRx6oZhYrVKKAQNhQ/AE
f9bXCjn1TG8RarES1/hVciGGfPMtFV6GJ+TUgY/Ov5hsm/f/RAi2okj92Lg0GmeV
/OJGMLEpAYAG1XXanT/AwQ1/edJxuGMzJW6tIRtBkgpos6Elh0LQ9JeRD3BwuB1v
O2R3mgzmgR46dCqqXjFnJRfckBjfiX8JxcGeOKGh/D9kxVkPCY4E3O9X9Qr8X+4=
=OF4W
-----END PGP SIGNATURE-----

--nextPart2054907.g8nqithbEE--