Subject: Set arbitrary GUC options during initdb

From: Tom Lane <[email-masked]>
Date: Wed, 25 Jan 2023 16:25:19 -0500

The attached patch responds to the discussion at [1] about how
we ought to offer a way to set any server GUC from the initdb
command line.  Currently, if for some reason the server won't
start with default parameters, the only way to get through initdb
is to change the installed version of postgresql.conf.sample.
And even that is just a kluge, because the initial probes to
choose max_connections and shared_buffers will all fail, causing
initdb to choose rock-bottom-minimum values of those settings.
You can fix that up after the fact if you notice it, but you
might not.

So this invents an initdb switch "-c NAME=VALUE" just like the
one that the server itself has long had.  The specified settings
are applied on the command line of the initial probe calls
(which happen before we've made any config files), and then they
are added to postgresql.auto.conf, which causes them to take
effect for the bootstrap backend runs as well as subsequent
postmaster starts.

I also invented "--set NAME=VALUE", mainly because just about
every other initdb switch has a long form.  The server itself
doesn't have that spelling, so I'm not wedded to that part.

Thoughts?

			regards, tom lane

[1] https://www.postgresql.org/message-id/flat/17757-dbdfc1f1c954a6db%40postgresql.org

diff --git a/doc/src/sgml/ref/initdb.sgml b/doc/src/sgml/ref/initdb.sgml
index 5b2bdac101..904cee7858 100644
--- a/doc/src/sgml/ref/initdb.sgml
+++ b/doc/src/sgml/ref/initdb.sgml
@@ -433,6 +433,23 @@ PostgreSQL documentation
     Other, less commonly used, options are also available:
 
     <variablelist>
+     <varlistentry id="app-initdb-option-set">
+      <term><option>-c <replaceable>name</replaceable>=<replaceable>value</replaceable></option></term>
+      <term><option>--set <replaceable>name</replaceable>=<replaceable>value</replaceable></option></term>
+      <listitem>
+       <para>
+        Forcibly set the server parameter <replaceable>name</replaceable>
+        to <replaceable>value</replaceable> during <command>initdb</command>,
+        and also set up that assignment in the
+        generated <filename>postgresql.auto.conf</filename> file, as though
+        it had been issued via <command>ALTER SYSTEM SET</command>.
+        This option can be used more than once to set several parameters.
+        It is primarily useful when the environment is such that the server
+        will not start at all using the default parameters.
+       </para>
+      </listitem>
+     </varlistentry>
+
      <varlistentry id="app-initdb-option-debug">
       <term><option>-d</option></term>
       <term><option>--debug</option></term>
diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c
index 7a58c33ace..8daad6a1f6 100644
--- a/src/bin/initdb/initdb.c
+++ b/src/bin/initdb/initdb.c
@@ -82,6 +82,13 @@
 /* Ideally this would be in a .h file, but it hardly seems worth the trouble */
 extern const char *select_default_timezone(const char *share_path);
 
+/* simple list of strings */
+typedef struct _stringlist
+{
+	char	   *str;
+	struct _stringlist *next;
+} _stringlist;
+
 static const char *const auth_methods_host[] = {
 	"trust", "reject", "scram-sha-256", "md5", "password", "ident", "radius",
 #ifdef ENABLE_GSS
@@ -142,6 +149,8 @@ static char *pwfilename = NULL;
 static char *superuser_password = NULL;
 static const char *authmethodhost = NULL;
 static const char *authmethodlocal = NULL;
+static _stringlist *extra_guc_names = NULL;
+static _stringlist *extra_guc_values = NULL;
 static bool debug = false;
 static bool noclean = false;
 static bool noinstructions = false;
@@ -253,6 +262,7 @@ static void check_input(char *path);
 static void write_version_file(const char *extrapath);
 static void set_null_conf(void);
 static void test_config_settings(void);
+static bool test_specific_config_settings(int test_conns, int test_buffs);
 static void setup_config(void);
 static void bootstrap_template1(void);
 static void setup_auth(FILE *cmdfd);
@@ -359,6 +369,27 @@ escape_quotes_bki(const char *src)
 	return result;
 }
 
+/*
+ * Add an item at the end of a stringlist.
+ */
+static void
+add_stringlist_item(_stringlist **listhead, const char *str)
+{
+	_stringlist *newentry = pg_malloc(sizeof(_stringlist));
+	_stringlist *oldentry;
+
+	newentry->str = pg_strdup(str);
+	newentry->next = NULL;
+	if (*listhead == NULL)
+		*listhead = newentry;
+	else
+	{
+		for (oldentry = *listhead; oldentry->next; oldentry = oldentry->next)
+			 /* skip */ ;
+		oldentry->next = newentry;
+	}
+}
+
 /*
  * make a copy of the array of lines, with token replaced by replacement
  * the first time it occurs on each line.
@@ -873,11 +904,9 @@ test_config_settings(void)
 		400, 300, 200, 100, 50
 	};
 
-	char		cmd[MAXPGPATH];
 	const int	connslen = sizeof(trial_conns) / sizeof(int);
 	const int	bufslen = sizeof(trial_bufs) / sizeof(int);
 	int			i,
-				status,
 				test_conns,
 				test_buffs,
 				ok_buffers = 0;
@@ -903,19 +932,7 @@ test_config_settings(void)
 		test_conns = trial_conns[i];
 		test_buffs = MIN_BUFS_FOR_CONNS(test_conns);
 
-		snprintf(cmd, sizeof(cmd),
-				 "\"%s\" --check %s %s "
-				 "-c max_connections=%d "
-				 "-c shared_buffers=%d "
-				 "-c dynamic_shared_memory_type=%s "
-				 "< \"%s\" > \"%s\" 2>&1",
-				 backend_exec, boot_options, extra_options,
-				 test_conns, test_buffs,
-				 dynamic_shared_memory_type,
-				 DEVNULL, DEVNULL);
-		fflush(NULL);
-		status = system(cmd);
-		if (status == 0)
+		if (test_specific_config_settings(test_conns, test_buffs))
 		{
 			ok_buffers = test_buffs;
 			break;
@@ -940,19 +957,7 @@ test_config_settings(void)
 			break;
 		}
 
-		snprintf(cmd, sizeof(cmd),
-				 "\"%s\" --check %s %s "
-				 "-c max_connections=%d "
-				 "-c shared_buffers=%d "
-				 "-c dynamic_shared_memory_type=%s "
-				 "< \"%s\" > \"%s\" 2>&1",
-				 backend_exec, boot_options, extra_options,
-				 n_connections, test_buffs,
-				 dynamic_shared_memory_type,
-				 DEVNULL, DEVNULL);
-		fflush(NULL);
-		status = system(cmd);
-		if (status == 0)
+		if (test_specific_config_settings(n_connections, test_buffs))
 			break;
 	}
 	n_buffers = test_buffs;
@@ -968,6 +973,48 @@ test_config_settings(void)
 	printf("%s\n", default_timezone ? default_timezone : "GMT");
 }
 
+/*
+ * Test a specific combination of configuration settings.
+ */
+static bool
+test_specific_config_settings(int test_conns, int test_buffs)
+{
+	PQExpBuffer cmd = createPQExpBuffer();
+	_stringlist *gnames,
+			   *gvalues;
+	int			status;
+
+	/* Set up the test postmaster invocation */
+	printfPQExpBuffer(cmd,
+					  "\"%s\" --check %s %s "
+					  "-c max_connections=%d "
+					  "-c shared_buffers=%d "
+					  "-c dynamic_shared_memory_type=%s",
+					  backend_exec, boot_options, extra_options,
+					  test_conns, test_buffs,
+					  dynamic_shared_memory_type);
+
+	/* Add any user-given setting overrides */
+	for (gnames = extra_guc_names, gvalues = extra_guc_values;
+		 gnames != NULL;		/* assume lists have the same length */
+		 gnames = gnames->next, gvalues = gvalues->next)
+	{
+		appendPQExpBuffer(cmd, " -c %s=", gnames->str);
+		appendShellString(cmd, gvalues->str);
+	}
+
+	appendPQExpBuffer(cmd,
+					  " < \"%s\" > \"%s\" 2>&1",
+					  DEVNULL, DEVNULL);
+
+	fflush(NULL);
+	status = system(cmd->data);
+
+	destroyPQExpBuffer(cmd);
+
+	return (status == 0);
+}
+
 /*
  * Calculate the default wal_size with a "pretty" unit.
  */
@@ -994,7 +1041,10 @@ setup_config(void)
 	char	  **conflines;
 	char		repltok[MAXPGPATH];
 	char		path[MAXPGPATH];
-	char	   *autoconflines[3];
+	char	  **autoconflines;
+	int			numautoconflines;
+	_stringlist *gnames,
+			   *gvalues;
 
 	fputs(_("creating configuration files ... "), stdout);
 	fflush(stdout);
@@ -1152,15 +1202,32 @@ setup_config(void)
 	if (chmod(path, pg_file_create_mode) != 0)
 		pg_fatal("could not change permissions of \"%s\": %m", path);
 
+	free(conflines);
+
+
 	/*
-	 * create the automatic configuration file to store the configuration
-	 * parameters set by ALTER SYSTEM command. The parameters present in this
-	 * file will override the value of parameters that exists before parse of
-	 * this file.
+	 * Create the automatic configuration file that stores the configuration
+	 * parameters set by the ALTER SYSTEM command.  We also place any
+	 * parameter values set with initdb's -c option into this file.
 	 */
+	numautoconflines = 2;		/* fixed header lines */
+	for (gnames = extra_guc_names; gnames != NULL; gnames = gnames->next)
+		numautoconflines++;
+	autoconflines = (char **) pg_malloc((numautoconflines + 1) * sizeof(char *));
+
 	autoconflines[0] = pg_strdup("# Do not edit this file manually!\n");
 	autoconflines[1] = pg_strdup("# It will be overwritten by the ALTER SYSTEM command.\n");
-	autoconflines[2] = NULL;
+	numautoconflines = 2;
+	for (gnames = extra_guc_names, gvalues = extra_guc_values;
+		 gnames != NULL;		/* assume lists have the same length */
+		 gnames = gnames->next, gvalues = gvalues->next)
+	{
+		autoconflines[numautoconflines++] =
+			psprintf("%s = '%s'\n",
+					 gnames->str,
+					 escape_quotes(gvalues->str));
+	}
+	autoconflines[numautoconflines] = NULL;
 
 	sprintf(path, "%s/postgresql.auto.conf", pg_data);
 
@@ -1168,7 +1235,7 @@ setup_config(void)
 	if (chmod(path, pg_file_create_mode) != 0)
 		pg_fatal("could not change permissions of \"%s\": %m", path);
 
-	free(conflines);
+	free(autoconflines);

 	/* pg_hba.conf */
@@ -2124,6 +2191,7 @@ usage(const char *progname)
 	printf(_("  -X, --waldir=WALDIR       location for the write-ahead log directory\n"));
 	printf(_("      --wal-segsize=SIZE    size of WAL segments, in megabytes\n"));
 	printf(_("\nLess commonly used options:\n"));
+	printf(_("  -c, --set NAME=VALUE      override default setting for server parameter\n"));
 	printf(_("  -d, --debug               generate lots of debugging output\n"));
 	printf(_("      --discard-caches      set debug_discard_caches=1\n"));
 	printf(_("  -L DIRECTORY              where to find the input files\n"));
@@ -2759,6 +2827,7 @@ main(int argc, char *argv[])
 		{"nosync", no_argument, NULL, 'N'}, /* for backwards compatibility */
 		{"no-sync", no_argument, NULL, 'N'},
 		{"no-instructions", no_argument, NULL, 13},
+		{"set", required_argument, NULL, 'c'},
 		{"sync-only", no_argument, NULL, 'S'},
 		{"waldir", required_argument, NULL, 'X'},
 		{"wal-segsize", required_argument, NULL, 12},
@@ -2808,7 +2877,8 @@ main(int argc, char *argv[])
 
 	/* process command-line options */
 
-	while ((c = getopt_long(argc, argv, "A:dD:E:gkL:nNsST:U:WX:", long_options, &option_index)) != -1)
+	while ((c = getopt_long(argc, argv, "A:c:dD:E:gkL:nNsST:U:WX:",
+							long_options, &option_index)) != -1)
 	{
 		switch (c)
 		{
@@ -2831,6 +2901,24 @@ main(int argc, char *argv[])
 			case 11:
 				authmethodhost = pg_strdup(optarg);
 				break;
+			case 'c':
+				{
+					char	   *buf = pg_strdup(optarg);
+					char	   *equals = strchr(buf, '=');
+
+					if (!equals)
+					{
+						pg_log_error("-c %s requires a value", buf);
+						pg_log_error_hint("Try \"%s --help\" for more information.",
+										  progname);
+						exit(1);
+					}
+					*equals++ = '\0';	/* terminate variable name */
+					add_stringlist_item(&extra_guc_names, buf);
+					add_stringlist_item(&extra_guc_values, equals);
+					pfree(buf);
+				}
+				break;
 			case 'D':
 				pg_data = pg_strdup(optarg);
 				break;
diff --git a/src/bin/initdb/t/001_initdb.pl b/src/bin/initdb/t/001_initdb.pl
index 772769acab..1e23ac8d0c 100644
--- a/src/bin/initdb/t/001_initdb.pl
+++ b/src/bin/initdb/t/001_initdb.pl
@@ -48,7 +48,13 @@ mkdir $datadir;
 	local (%ENV) = %ENV;
 	delete $ENV{TZ};
 
-	command_ok([ 'initdb', '-N', '-T', 'german', '-X', $xlogdir, $datadir ],
+	# while we are here, also exercise -T and -c options
+	command_ok(
+		[
+			'initdb', '-N', '-T', 'german', '-c',
+			'default_text_search_config=german',
+			'-X', $xlogdir, $datadir
+		],
 		'successful creation');
 
 	# Permissions on PGDATA should be default
@@ -147,4 +153,7 @@ command_fails(
 	],
 	'fails for invalid option combination');
 
+command_fails([ 'initdb', '--no-sync', '--set', 'foo=bar', "$tempdir/dataX" ],
+	'fails for invalid --set option');
+
 done_testing();

---

From: Andres Freund <[email-masked]>
Date: Wed, 25 Jan 2023 14:03:39 -0800

Hi,

On 2023-01-25 16:25:19 -0500, Tom Lane wrote:
> command line.

Are you thinking of backpatching this, to offer the people affected by the
issue in [1] a way out?

> one that the server itself has long had.

I still am +1 on the idea. I've actually wanted this for development purposes
a couple times...

> The specified settings are applied on the command line of the initial probe
> calls (which happen before we've made any config files), and then they are
> added to postgresql.auto.conf, which causes them to take effect for the
> bootstrap backend runs as well as subsequent postmaster starts.

I think this means that if you set e.g. max_connections as an initdb
parameter, the probes won't do much. Probably fine?

Perhaps worth memorializing the priority of the -c options in a test?
E.g. setting shared_buffers = 20MB or so and then testing that that's the
value when starting the server?

> doesn't have that spelling, so I'm not wedded to that part.

Fine with me, but also fine to leave out.

Greetings,

Andres Freund

---

From: Robert Haas <[email-masked]>
Date: Fri, 27 Jan 2023 10:29:37 -0500

> one that the server itself has long had.

HUGE +1 from me. This will, I think, be extremely convenient in many situations.

> The specified settings

I agree with others that it would seem more natural to edit them in
postgresql.conf itself, but I also think it doesn't matter nearly as
much as getting the feature in some form.

-- 
Robert Haas
EDB: http://www.enterprisedb.com