В письме от 1 ноября 2018 18:26:20 пользователь Nikolay Shaplov написал:

> In this case the only solution I can see is
>
> DETAIL:  Valid values are: "value1", "value2", "value3".
>
> Where list '"value1", "value2", "value3"' is built in runtime but have no
> any bindnings to any specific language. And the rest of the message is
> 'DETAIL:  Valid values are: %s' which can be properly translated.

I've fiex the patch. Now it does not add "and" at all.

--
Do code for fun.
diff --git a/src/backend/access/common/reloptions.c b/src/backend/access/common/reloptions.c
index db84da0..b266c86 100644
--- a/src/backend/access/common/reloptions.c
+++ b/src/backend/access/common/reloptions.c
@@ -422,7 +422,11 @@ static relopt_real realRelOpts[]  	{{NULL}}
 };

-static relopt_string stringRelOpts[] +static relopt_enum_element_definition gist_buffering_enum_def[] +										GIST_OPTION_BUFFERING_ENUM_DEF;
+static relopt_enum_element_definition view_check_option_enum_def[] +										VIEW_OPTION_CHECK_OPTION_ENUM_DEF;
+static relopt_enum enumRelOpts[]  {
 	{
 		{
@@ -431,10 +435,8 @@ static relopt_string stringRelOpts[]  			RELOPT_KIND_GIST,
 			AccessExclusiveLock
 		},
-		4,
-		false,
-		gistValidateBufferingOption,
-		"auto"
+		gist_buffering_enum_def,
+		GIST_OPTION_BUFFERING_AUTO
 	},
 	{
 		{
@@ -443,11 +445,14 @@ static relopt_string stringRelOpts[]  			RELOPT_KIND_VIEW,
 			AccessExclusiveLock
 		},
-		0,
-		true,
-		validateWithCheckOption,
-		NULL
+		view_check_option_enum_def,
+		VIEW_OPTION_CHECK_OPTION_NOT_SET
 	},
+	{{NULL}}
+};
+
+static relopt_string stringRelOpts[] +{
 	/* list terminator */
 	{{NULL}}
 };
@@ -494,6 +499,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,
@@ -532,6 +543,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;
@@ -629,6 +648,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;
@@ -708,6 +730,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,
+					 relopt_enum_element_definition *enum_def, int default_val)
+{
+	relopt_enum *newoption;
+
+	newoption = (relopt_enum *) allocate_reloption(kinds, RELOPT_TYPE_ENUM,
+												   name, desc);
+	newoption->enum_def = enum_def;
+	newoption->default_val = default_val;
+
+	add_reloption((relopt_gen *) newoption);
+}
+
+/*
  * add_string_reloption
  *		Add a new string reloption
  *
@@ -1208,6 +1248,51 @@ parse_one_reloption(relopt_value *option, char *text_str, int text_len,
 									   optreal->min, optreal->max)));
 			}
 			break;
+		case RELOPT_TYPE_ENUM:
+			{
+				relopt_enum *optenum = (relopt_enum *) option->gen;
+				relopt_enum_element_definition *el_def;
+
+				parsed = false;
+				for(el_def = optenum->enum_def; el_def->text_value; el_def++)
+				{
+					if (pg_strcasecmp(value, el_def->text_value) == 0)
+					{
+						option->values.enum_val = el_def->numeric_value;
+						parsed = true;
+						break;
+					}
+				}
+				if (!parsed)
+				{
+					/*
+					 * If value is not among allowed enum text values, but we
+					 * are not up to validating, just use default numeric value,
+					 * otherwise we raise an error
+					 */
+					if (!validate)
+						option->values.enum_val = optenum->default_val;
+					else
+					{
+						StringInfoData buf;
+						initStringInfo(&buf);
+						for(el_def = optenum->enum_def; el_def->text_value;
+																	 el_def++)
+						{
+							appendStringInfo(&buf,"\"%s\"",el_def->text_value);
+							if (el_def[1].text_value)
+								appendStringInfo(&buf,", ");
+						}
+						ereport(ERROR,
+								(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+								 errmsg("invalid value for \"%s\" option",
+										option->gen->name),
+								 errdetail("Valid values are: %s.", buf.data)));
+						pfree(buf.data);
+					}
+				}
+			}
+			break;
 		case RELOPT_TYPE_STRING:
 			{
 				relopt_string *optstring = (relopt_string *) option->gen;
@@ -1301,6 +1386,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)
@@ -1413,8 +1503,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 70627e5..a73b355 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 b670cad..35b3d75 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..d7f9330 100644
--- a/src/include/access/gist_private.h
+++ b/src/include/access/gist_private.h
@@ -368,13 +368,33 @@ typedef struct GISTBuildBuffers
 } GISTBuildBuffers;

 /*
+ * Buffering is an enum option
+ * gist_option_buffering_numeric_values defines a numeric representation of
+ * option values, and GIST_OPTION_BUFFERING_ENUM_DEF defines enum string values
+ * and maps them to numeric one.
+ */
+typedef enum gist_option_buffering_numeric_values
+{
+	GIST_OPTION_BUFFERING_ON = 0,
+	GIST_OPTION_BUFFERING_OFF = 1,
+	GIST_OPTION_BUFFERING_AUTO = 2,
+}	gist_option_buffering_numeric_values;
+
+#define GIST_OPTION_BUFFERING_ENUM_DEF { 	\
+	{ "on",		GIST_OPTION_BUFFERING_ON },		\
+	{ "off",	GIST_OPTION_BUFFERING_OFF },	\
+	{ "auto",	GIST_OPTION_BUFFERING_AUTO },	\
+	{ (const char *) NULL, 0 }					\
+}
+
+/*
  * 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 4022c14..9632faf 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;

@@ -81,6 +82,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;
@@ -108,6 +110,25 @@ typedef struct relopt_real
 	double		max;
 } relopt_real;

+/*
+ * relopt_enum_element_definition - An element that defines enum name-value
+ * pair. An array of such elements defines acceptable values of enum option,
+ * and their internal numeric representation
+ */
+typedef struct relopt_enum_element_definition
+{
+	const char *text_value;
+	int			numeric_value;
+} relopt_enum_element_definition;
+
+typedef struct relopt_enum
+{
+	relopt_gen	gen;
+	relopt_enum_element_definition *enum_def; /* Null terminated array of enum
+											   * elements definitions */
+	int			default_val; /* Value that is used when option is not defined */
+} relopt_enum;
+
 /* validation routines for strings */
 typedef void (*validate_string_relopt) (const char *value);

@@ -253,6 +274,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_definition *enum_def, 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 84469f5..ffed97a 100644
--- a/src/include/utils/rel.h
+++ b/src/include/utils/rel.h
@@ -319,6 +319,25 @@ typedef struct StdRdOptions
 	((relation)->rd_options ? \
 	 ((StdRdOptions *) (relation)->rd_options)->parallel_workers : (defaultpw))

+/*
+ * check_option is an enum option
+ * view_option_check_option_numeric_values defines a numeric representation of
+ * option values, and VIEW_OPTION_CHECK_OPTION_ENUM_DEF defines enum string
+ * values and maps them to numeric one.
+ */
+
+typedef enum view_option_check_option_numeric_values
+{
+	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;
+
+#define VIEW_OPTION_CHECK_OPTION_ENUM_DEF {					\
+	{ "local", 		VIEW_OPTION_CHECK_OPTION_LOCAL},		\
+	{ "cascaded",	VIEW_OPTION_CHECK_OPTION_CASCADED },	\
+	{ (const char *) NULL, 0 }								\
+}

 /*
  * ViewOptions
@@ -328,7 +347,7 @@ typedef struct ViewOptions
 {
 	int32		vl_len_;		/* varlena header (do not touch directly!) */
 	bool		security_barrier;
-	int			check_option_offset;
+	int			check_option;
 } ViewOptions;

 /*
@@ -347,7 +366,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
@@ -356,10 +376,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
@@ -368,11 +386,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..32f5e25 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", "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".
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
index e64d693..c7ab1a8 100644
--- a/src/test/regress/expected/updatable_views.out
+++ b/src/test/regress/expected/updatable_views.out
@@ -1675,7 +1675,7 @@ SELECT * FROM base_tbl;

 ALTER VIEW rw_view1 SET (check_option=here); -- invalid
 ERROR:  invalid value for "check_option" option
-DETAIL:  Valid values are "local" and "cascaded".
+DETAIL:  Valid values are: "local", "cascaded".
 ALTER VIEW rw_view1 SET (check_option=local);
 INSERT INTO rw_view2 VALUES (-20); -- should fail
 ERROR:  new row violates check option for view "rw_view1"

Attachment: signature.asc
Description: This is a digitally signed message part.

Reply via email to