Reorganize GUC structs

Instead of having five separate GUC structs, one for each type, with
the generic part contained in each of them, flip it around and have
one common struct, with the type-specific part has a subfield.

The very original GUC design had type-specific structs and
type-specific lists, and the membership in one of the lists defined
the type.  But now the structs themselves know the type (from the
.vartype field), and they are all loaded into a common hash table at
run time, and so this original separation no longer makes sense.  It
creates a bunch of inconsistencies in the code about whether the
type-specific or the generic struct is the primary struct, and a lot
of casting in between, which makes certain assumptions about the
struct layouts.

After the change, all these casts are gone and all the data is
accessed via normal field references.  Also, various code is
simplified because only one kind of struct needs to be processed.

Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: Heikki Linnakangas <hlinnaka@iki.fi>
Discussion: https://www.postgresql.org/message-id/flat/8fdfb91e-60fb-44fa-8df6-f5dea47353c9@eisentraut.org

src/backend/utils/misc/gen_guc_tables.pl

@@ -25,10 +25,7 @@ my $parse = Catalog::ParseData($input_fname);
 open my $ofh, '>', $output_fname or die;
 
 print_boilerplate($ofh, $output_fname, 'GUC tables');
-foreach my $type (qw(bool int real string enum))
-{
-	print_one_table($ofh, $type);
-}
+print_table($ofh);
 
 close $ofh;
 
@@ -41,56 +38,52 @@ sub dquote
 	return q{"} . $s =~ s/"/\\"/gr . q{"};
 }
 
-# Print GUC table for one type.
-sub print_one_table
+# Print GUC table.
+sub print_table
 {
-	my ($ofh, $type) = @_;
-	my $Type = ucfirst $type;
+	my ($ofh) = @_;
 
 	print $ofh "\n\n";
-	print $ofh "struct config_${type} ConfigureNames${Type}[] =\n";
+	print $ofh "struct config_generic ConfigureNames[] =\n";
 	print $ofh "{\n";
 
 	foreach my $entry (@{$parse})
 	{
-		next if $entry->{type} ne $type;
-
 		print $ofh "#ifdef $entry->{ifdef}\n" if $entry->{ifdef};
 		print $ofh "\t{\n";
-		print $ofh "\t\t{\n";
-		printf $ofh "\t\t\t.name = %s,\n", dquote($entry->{name});
-		printf $ofh "\t\t\t.context = %s,\n", $entry->{context};
-		printf $ofh "\t\t\t.group = %s,\n", $entry->{group};
-		printf $ofh "\t\t\t.short_desc = gettext_noop(%s),\n",
+		printf $ofh "\t\t.name = %s,\n", dquote($entry->{name});
+		printf $ofh "\t\t.context = %s,\n", $entry->{context};
+		printf $ofh "\t\t.group = %s,\n", $entry->{group};
+		printf $ofh "\t\t.short_desc = gettext_noop(%s),\n",
 		  dquote($entry->{short_desc});
-		printf $ofh "\t\t\t.long_desc = gettext_noop(%s),\n",
+		printf $ofh "\t\t.long_desc = gettext_noop(%s),\n",
 		  dquote($entry->{long_desc})
 		  if $entry->{long_desc};
-		printf $ofh "\t\t\t.flags = %s,\n", $entry->{flags}
-		  if $entry->{flags};
-		printf $ofh "\t\t\t.vartype = %s,\n", ('PGC_' . uc($type));
-		print $ofh "\t\t},\n";
-		printf $ofh "\t\t.variable = &%s,\n", $entry->{variable};
-		printf $ofh "\t\t.boot_val = %s,\n", $entry->{boot_val};
-		printf $ofh "\t\t.min = %s,\n", $entry->{min}
+		printf $ofh "\t\t.flags = %s,\n", $entry->{flags} if $entry->{flags};
+		printf $ofh "\t\t.vartype = %s,\n", ('PGC_' . uc($entry->{type}));
+		printf $ofh "\t\t._%s = {\n", $entry->{type};
+		printf $ofh "\t\t\t.variable = &%s,\n", $entry->{variable};
+		printf $ofh "\t\t\t.boot_val = %s,\n", $entry->{boot_val};
+		printf $ofh "\t\t\t.min = %s,\n", $entry->{min}
 		  if $entry->{type} eq 'int' || $entry->{type} eq 'real';
-		printf $ofh "\t\t.max = %s,\n", $entry->{max}
+		printf $ofh "\t\t\t.max = %s,\n", $entry->{max}
 		  if $entry->{type} eq 'int' || $entry->{type} eq 'real';
-		printf $ofh "\t\t.options = %s,\n", $entry->{options}
+		printf $ofh "\t\t\t.options = %s,\n", $entry->{options}
 		  if $entry->{type} eq 'enum';
-		printf $ofh "\t\t.check_hook = %s,\n", $entry->{check_hook}
+		printf $ofh "\t\t\t.check_hook = %s,\n", $entry->{check_hook}
 		  if $entry->{check_hook};
-		printf $ofh "\t\t.assign_hook = %s,\n", $entry->{assign_hook}
+		printf $ofh "\t\t\t.assign_hook = %s,\n", $entry->{assign_hook}
 		  if $entry->{assign_hook};
-		printf $ofh "\t\t.show_hook = %s,\n", $entry->{show_hook}
+		printf $ofh "\t\t\t.show_hook = %s,\n", $entry->{show_hook}
 		  if $entry->{show_hook};
+		print $ofh "\t\t},\n";
 		print $ofh "\t},\n";
 		print $ofh "#endif\n" if $entry->{ifdef};
 		print $ofh "\n";
 	}
 
 	print $ofh "\t/* End-of-list marker */\n";
-	print $ofh "\t{{0}}\n";
+	print $ofh "\t{0}\n";
 	print $ofh "};\n";
 
 	return;

src/backend/utils/misc/guc_funcs.c

@@ -629,7 +629,7 @@ GetConfigOptionValues(const struct config_generic *conf, const char **values)
 	{
 		case PGC_BOOL:
 			{
-				const struct config_bool *lconf = (const struct config_bool *) conf;
+				const struct config_bool *lconf = &conf->_bool;
 
 				/* min_val */
 				values[9] = NULL;
@@ -650,7 +650,7 @@ GetConfigOptionValues(const struct config_generic *conf, const char **values)
 
 		case PGC_INT:
 			{
-				const struct config_int *lconf = (const struct config_int *) conf;
+				const struct config_int *lconf = &conf->_int;
 
 				/* min_val */
 				snprintf(buffer, sizeof(buffer), "%d", lconf->min);
@@ -675,7 +675,7 @@ GetConfigOptionValues(const struct config_generic *conf, const char **values)
 
 		case PGC_REAL:
 			{
-				const struct config_real *lconf = (const struct config_real *) conf;
+				const struct config_real *lconf = &conf->_real;
 
 				/* min_val */
 				snprintf(buffer, sizeof(buffer), "%g", lconf->min);
@@ -700,7 +700,7 @@ GetConfigOptionValues(const struct config_generic *conf, const char **values)
 
 		case PGC_STRING:
 			{
-				const struct config_string *lconf = (const struct config_string *) conf;
+				const struct config_string *lconf = &conf->_string;
 
 				/* min_val */
 				values[9] = NULL;
@@ -727,7 +727,7 @@ GetConfigOptionValues(const struct config_generic *conf, const char **values)
 
 		case PGC_ENUM:
 			{
-				const struct config_enum *lconf = (const struct config_enum *) conf;
+				const struct config_enum *lconf = &conf->_enum;
 
 				/* min_val */
 				values[9] = NULL;
@@ -745,11 +745,11 @@ GetConfigOptionValues(const struct config_generic *conf, const char **values)
 													 "{\"", "\"}", "\",\"");
 
 				/* boot_val */
-				values[12] = pstrdup(config_enum_lookup_by_value(lconf,
+				values[12] = pstrdup(config_enum_lookup_by_value(conf,
 																 lconf->boot_val));
 
 				/* reset_val */
-				values[13] = pstrdup(config_enum_lookup_by_value(lconf,
+				values[13] = pstrdup(config_enum_lookup_by_value(conf,
 																 lconf->reset_val));
 			}
 			break;

src/backend/utils/misc/help_config.c

@@ -23,23 +23,8 @@
 #include "utils/help_config.h"
 
 
-/*
- * This union allows us to mix the numerous different types of structs
- * that we are organizing.
- */
-typedef union
-{
-	struct config_generic generic;
-	struct config_bool _bool;
-	struct config_real real;
-	struct config_int integer;
-	struct config_string string;
-	struct config_enum _enum;
-} mixedStruct;
-
-
-static void printMixedStruct(mixedStruct *structToPrint);
-static bool displayStruct(mixedStruct *structToDisplay);
+static void printMixedStruct(const struct config_generic *structToPrint);
+static bool displayStruct(const struct config_generic *structToDisplay);
 
 
 void
@@ -55,7 +40,7 @@ GucInfoMain(void)
 
 	for (int i = 0; i < numOpts; i++)
 	{
-		mixedStruct *var = (mixedStruct *) guc_vars[i];
+		const struct config_generic *var = guc_vars[i];
 
 		if (displayStruct(var))
 			printMixedStruct(var);
@@ -70,11 +55,11 @@ GucInfoMain(void)
  * should be displayed to the user.
  */
 static bool
-displayStruct(mixedStruct *structToDisplay)
+displayStruct(const struct config_generic *structToDisplay)
 {
-	return !(structToDisplay->generic.flags & (GUC_NO_SHOW_ALL |
-											   GUC_NOT_IN_SAMPLE |
-											   GUC_DISALLOW_IN_FILE));
+	return !(structToDisplay->flags & (GUC_NO_SHOW_ALL |
+									   GUC_NOT_IN_SAMPLE |
+									   GUC_DISALLOW_IN_FILE));
 }
 
 
@@ -83,14 +68,14 @@ displayStruct(mixedStruct *structToDisplay)
  * a different format, depending on what the user wants to see.
  */
 static void
-printMixedStruct(mixedStruct *structToPrint)
+printMixedStruct(const struct config_generic *structToPrint)
 {
 	printf("%s\t%s\t%s\t",
-		   structToPrint->generic.name,
-		   GucContext_Names[structToPrint->generic.context],
-		   _(config_group_names[structToPrint->generic.group]));
+		   structToPrint->name,
+		   GucContext_Names[structToPrint->context],
+		   _(config_group_names[structToPrint->group]));
 
-	switch (structToPrint->generic.vartype)
+	switch (structToPrint->vartype)
 	{
 
 		case PGC_BOOL:
@@ -101,26 +86,26 @@ printMixedStruct(mixedStruct *structToPrint)
 
 		case PGC_INT:
 			printf("INTEGER\t%d\t%d\t%d\t",
-				   structToPrint->integer.reset_val,
-				   structToPrint->integer.min,
-				   structToPrint->integer.max);
+				   structToPrint->_int.reset_val,
+				   structToPrint->_int.min,
+				   structToPrint->_int.max);
 			break;
 
 		case PGC_REAL:
 			printf("REAL\t%g\t%g\t%g\t",
-				   structToPrint->real.reset_val,
-				   structToPrint->real.min,
-				   structToPrint->real.max);
+				   structToPrint->_real.reset_val,
+				   structToPrint->_real.min,
+				   structToPrint->_real.max);
 			break;
 
 		case PGC_STRING:
 			printf("STRING\t%s\t\t\t",
-				   structToPrint->string.boot_val ? structToPrint->string.boot_val : "");
+				   structToPrint->_string.boot_val ? structToPrint->_string.boot_val : "");
 			break;
 
 		case PGC_ENUM:
 			printf("ENUM\t%s\t\t\t",
-				   config_enum_lookup_by_value(&structToPrint->_enum,
+				   config_enum_lookup_by_value(structToPrint,
 											   structToPrint->_enum.boot_val));
 			break;
 
@@ -130,6 +115,6 @@ printMixedStruct(mixedStruct *structToPrint)
 	}
 
 	printf("%s\t%s\n",
-		   (structToPrint->generic.short_desc == NULL) ? "" : _(structToPrint->generic.short_desc),
-		   (structToPrint->generic.long_desc == NULL) ? "" : _(structToPrint->generic.long_desc));
+		   (structToPrint->short_desc == NULL) ? "" : _(structToPrint->short_desc),
+		   (structToPrint->long_desc == NULL) ? "" : _(structToPrint->long_desc));
 }

src/include/utils/guc_tables.h

@@ -132,6 +132,84 @@ typedef struct guc_stack
 	config_var_value masked;	/* SET value in a GUC_SET_LOCAL entry */
 } GucStack;
 
+
+/* GUC records for specific variable types */
+
+struct config_bool
+{
+	/* constant fields, must be set correctly in initial value: */
+	bool	   *variable;
+	bool		boot_val;
+	GucBoolCheckHook check_hook;
+	GucBoolAssignHook assign_hook;
+	GucShowHook show_hook;
+	/* variable fields, initialized at runtime: */
+	bool		reset_val;
+};
+
+struct config_int
+{
+	/* constant fields, must be set correctly in initial value: */
+	int		   *variable;
+	int			boot_val;
+	int			min;
+	int			max;
+	GucIntCheckHook check_hook;
+	GucIntAssignHook assign_hook;
+	GucShowHook show_hook;
+	/* variable fields, initialized at runtime: */
+	int			reset_val;
+};
+
+struct config_real
+{
+	/* constant fields, must be set correctly in initial value: */
+	double	   *variable;
+	double		boot_val;
+	double		min;
+	double		max;
+	GucRealCheckHook check_hook;
+	GucRealAssignHook assign_hook;
+	GucShowHook show_hook;
+	/* variable fields, initialized at runtime: */
+	double		reset_val;
+};
+
+/*
+ * A note about string GUCs: the boot_val is allowed to be NULL, which leads
+ * to the reset_val and the actual variable value (*variable) also being NULL.
+ * However, there is no way to set a NULL value subsequently using
+ * set_config_option or any other GUC API.  Also, GUC APIs such as SHOW will
+ * display a NULL value as an empty string.  Callers that choose to use a NULL
+ * boot_val should overwrite the setting later in startup, or else be careful
+ * that NULL doesn't have semantics that are visibly different from an empty
+ * string.
+ */
+struct config_string
+{
+	/* constant fields, must be set correctly in initial value: */
+	char	  **variable;
+	const char *boot_val;
+	GucStringCheckHook check_hook;
+	GucStringAssignHook assign_hook;
+	GucShowHook show_hook;
+	/* variable fields, initialized at runtime: */
+	char	   *reset_val;
+};
+
+struct config_enum
+{
+	/* constant fields, must be set correctly in initial value: */
+	int		   *variable;
+	int			boot_val;
+	const struct config_enum_entry *options;
+	GucEnumCheckHook check_hook;
+	GucEnumAssignHook assign_hook;
+	GucShowHook show_hook;
+	/* variable fields, initialized at runtime: */
+	int			reset_val;
+};
+
 /*
  * Generic fields applicable to all types of variables
  *
@@ -200,6 +278,16 @@ struct config_generic
 	char	   *sourcefile;		/* file current setting is from (NULL if not
 								 * set in config file) */
 	int			sourceline;		/* line in source file */
+
+	/* fields for specific variable types */
+	union
+	{
+		struct config_bool _bool;
+		struct config_int _int;
+		struct config_real _real;
+		struct config_string _string;
+		struct config_enum _enum;
+	};
 };
 
 /* bit values in status field */
@@ -212,100 +300,14 @@ struct config_generic
 #define GUC_NEEDS_REPORT	0x0004	/* new value must be reported to client */
 
 
-/* GUC records for specific variable types */
-
-struct config_bool
-{
-	struct config_generic gen;
-	/* constant fields, must be set correctly in initial value: */
-	bool	   *variable;
-	bool		boot_val;
-	GucBoolCheckHook check_hook;
-	GucBoolAssignHook assign_hook;
-	GucShowHook show_hook;
-	/* variable fields, initialized at runtime: */
-	bool		reset_val;
-};
-
-struct config_int
-{
-	struct config_generic gen;
-	/* constant fields, must be set correctly in initial value: */
-	int		   *variable;
-	int			boot_val;
-	int			min;
-	int			max;
-	GucIntCheckHook check_hook;
-	GucIntAssignHook assign_hook;
-	GucShowHook show_hook;
-	/* variable fields, initialized at runtime: */
-	int			reset_val;
-};
-
-struct config_real
-{
-	struct config_generic gen;
-	/* constant fields, must be set correctly in initial value: */
-	double	   *variable;
-	double		boot_val;
-	double		min;
-	double		max;
-	GucRealCheckHook check_hook;
-	GucRealAssignHook assign_hook;
-	GucShowHook show_hook;
-	/* variable fields, initialized at runtime: */
-	double		reset_val;
-};
-
-/*
- * A note about string GUCs: the boot_val is allowed to be NULL, which leads
- * to the reset_val and the actual variable value (*variable) also being NULL.
- * However, there is no way to set a NULL value subsequently using
- * set_config_option or any other GUC API.  Also, GUC APIs such as SHOW will
- * display a NULL value as an empty string.  Callers that choose to use a NULL
- * boot_val should overwrite the setting later in startup, or else be careful
- * that NULL doesn't have semantics that are visibly different from an empty
- * string.
- */
-struct config_string
-{
-	struct config_generic gen;
-	/* constant fields, must be set correctly in initial value: */
-	char	  **variable;
-	const char *boot_val;
-	GucStringCheckHook check_hook;
-	GucStringAssignHook assign_hook;
-	GucShowHook show_hook;
-	/* variable fields, initialized at runtime: */
-	char	   *reset_val;
-};
-
-struct config_enum
-{
-	struct config_generic gen;
-	/* constant fields, must be set correctly in initial value: */
-	int		   *variable;
-	int			boot_val;
-	const struct config_enum_entry *options;
-	GucEnumCheckHook check_hook;
-	GucEnumAssignHook assign_hook;
-	GucShowHook show_hook;
-	/* variable fields, initialized at runtime: */
-	int			reset_val;
-};
-
 /* constant tables corresponding to enums above and in guc.h */
 extern PGDLLIMPORT const char *const config_group_names[];
 extern PGDLLIMPORT const char *const config_type_names[];
 extern PGDLLIMPORT const char *const GucContext_Names[];
 extern PGDLLIMPORT const char *const GucSource_Names[];
 
-/* data arrays defining all the built-in GUC variables */
-extern PGDLLIMPORT struct config_bool ConfigureNamesBool[];
-extern PGDLLIMPORT struct config_int ConfigureNamesInt[];
-extern PGDLLIMPORT struct config_real ConfigureNamesReal[];
-extern PGDLLIMPORT struct config_string ConfigureNamesString[];
-extern PGDLLIMPORT struct config_enum ConfigureNamesEnum[];
+/* data array defining all the built-in GUC variables */
+extern PGDLLIMPORT struct config_generic ConfigureNames[];
 
 /* lookup GUC variables, returning config_generic pointers */
 extern struct config_generic *find_option(const char *name,
@@ -326,7 +328,7 @@ extern struct config_generic **get_guc_variables(int *num_vars);
 extern void build_guc_variables(void);
 
 /* search in enum options */
-extern const char *config_enum_lookup_by_value(const struct config_enum *record, int val);
+extern const char *config_enum_lookup_by_value(const struct config_generic *record, int val);
 extern bool config_enum_lookup_by_name(const struct config_enum *record,
 									   const char *value, int *retval);
 extern char *config_enum_get_options(const struct config_enum *record,

src/tools/pgindent/typedefs.list

@@ -3808,7 +3808,6 @@ memoize_iterator
 metastring
 missing_cache_key
 mix_data_t
-mixedStruct
 mode_t
 movedb_failure_params
 multirange_bsearch_comparison