Attached is an update to my earlier patch. This clears my own bug,
usability concerns, and implementation ideas list on this one.
There's full documentation on this now, including some suggested ways
all these include features might be used. Since there's so much
controversy around the way I would like to see things organized by
default, instead of kicking that off again I just outline a couple of
ways people might do things, showing both the flexibility and some good
practices I've seen that people can adopt--or not. Possibilities rather
than policy. The include-related section got big enough that it was
really disruptive, so I moved it to a new sub-section. I like the flow
of this better, it slightly slimmed down the too big "Setting
Parameters" section. You can see a snapshot of the new doc page I built
at http://http://www.westnet.com/~gsmith/config-setting.html
Here's a partial demo highlighting the updated ignoring logic (which I
tested more extensively with some debugging messages about its
decisions, then removed those). Both .02test.conf and .conf appear, but
neither are processed:
$ tail -n 1 $PGDATA/postgresql.conf
include_dir='conf.d'
$ ls -a $PGDATA/conf.d
. .. 00test.conf 01test.conf .02test.conf .conf
$ cat $PGDATA/conf.d/00test.conf
work_mem = 2MB
$ cat $PGDATA/conf.d/01test.conf
work_mem = 3MB
$ cat $PGDATA/conf.d/.conf
checkpoint_segments=10
$ psql -x -c "select name,setting,sourcefile from pg_settings where
name='work_mem'"
-[ RECORD 1 ]---------------------------------------------------
name | work_mem
setting | 3072
sourcefile | /home/gsmith/pgwork/data/confdir/conf.d/01test.conf
$ psql -x -c "select name,setting,sourcefile from pg_settings where
name='checkpoint_segments'"
-[ RECORD 1 ]-------------------
name | checkpoint_segments
setting | 3
sourcefile |
In addition to adding the docs, I changed these major things relative to
the version that was reviewed:
-The configuration directory name is now considered relative to the
directory that the including file is located in. That's the same logic
ParseConfigFile used to convert relative to absolute paths, and as Noah
suggested it nicely eliminates several of the concerns I had about what
I'd submitted before. I was concerned before about creating a packaging
problem, now I'm not. Relocate postgresql.conf, and the include
directory base goes with it, regardless of the method you used to do
so. The shared logic for this has been refactored into a new
AbsoluteConfigLocation function that both ParseConfigFile and this new
ParseConfigDirectory call. That's an improvement to the readability of
both functions too.
-With that change, all of the hackery around exporting configdir goes
away too. Which means that the code works as expected during SIGHUP
reloads too. I love it when a plan comes together.
-Hidden files (starts with ".") are now skipped. This also eliminates
the concern about whether ".conf" is considered a valid name for a file;
it is clearly not.
-Per renaming suggestion from Robert to make my other submission use
include_if_exists, I've made this one include_dir instead of includedir.
There's some new error handling:
-If the configuration directory does not exist at all, throw an error.
It was quietly accepted before. I'm not sure why Magnus had coded it
that way originally, and I just passed that through without considering
a change. This still quietly accepts a directory that exists but has no
files in it. I consider that a reasonable behavior, but I wouldn't
reject the idea of giving a warning when that happens, if someone feels
that's appropriate here.
-When a file that was in the directory goes away before it is checked
with stat, consider that an error too.
There are some internal changes that should eliminate the main concerns
about Windows compatibility in particular:
-File name joining uses join_path_components, eliminating all sorts of bugs
-Use pstrdup() instead of strdup when building the list of files in the
directory
-Switch from using guc_realloc to [re]palloc for that temporary file list.
-Eliminated now unneeded export of guc_malloc and guc_realloc
-Moved ParseConfigDirectory prototype to the right place
-Don't bother trying to free individual bits of memory now that it's all
in the same context. Saves some lines of code, and I do not miss the
asserts I am no longer triggering.
The only review suggestion I failed to incorporate was this one from Noah:
> > > + if (strcmp(de->d_name + strlen(de->d_name) - 5,
".conf") != 0)
> > + continue;
> That may as well be memcmp().
While true, his idea bothers both my abstraction and unexpected bug
concern sides for reasons I can't really justify. I seem to have
received too many past beatings toward using the string variants of all
these functions whenever operating on things that are clearly strings.
I'll punt this one toward whoever looks at this next to decide, both
strcmp and strncmp are used in this section now.
--
Greg Smith 2ndQuadrant US g...@2ndquadrant.com Baltimore, MD
PostgreSQL Training, Services, and 24x7 Support www.2ndQuadrant.us
diff --git a/doc/src/sgml/config.sgml b/doc/src/sgml/config.sgml
index d1e628f..5df214e 100644
*** a/doc/src/sgml/config.sgml
--- b/doc/src/sgml/config.sgml
*************** shared_buffers = 128MB
*** 74,96 ****
<para>
<indexterm>
- <primary><literal>include</></primary>
- <secondary>in configuration file</secondary>
- </indexterm>
- In addition to parameter settings, the <filename>postgresql.conf</>
- file can contain <firstterm>include directives</>, which specify
- another file to read and process as if it were inserted into the
- configuration file at this point. Include directives simply look like:
- <programlisting>
- include 'filename'
- </programlisting>
- If the file name is not an absolute path, it is taken as relative to
- the directory containing the referencing configuration file.
- Inclusions can be nested.
- </para>
-
- <para>
- <indexterm>
<primary>SIGHUP</primary>
</indexterm>
The configuration file is reread whenever the main server process receives a
--- 74,79 ----
*************** SET ENABLE_SEQSCAN TO OFF;
*** 178,184 ****
any desired selection condition. It also contains more information about
what values are allowed for the parameters.
</para>
! </sect1>
<sect1 id="runtime-config-file-locations">
<title>File Locations</title>
--- 161,273 ----
any desired selection condition. It also contains more information about
what values are allowed for the parameters.
</para>
!
! <sect2 id="config-includes">
! <title>Configuration file includes</title>
! <para>
! <indexterm>
! <primary><literal>include</></primary>
! <secondary>in configuration file</secondary>
! </indexterm>
! In addition to parameter settings, the <filename>postgresql.conf</>
! file can contain <firstterm>include directives</>, which specify
! another file to read and process as if it were inserted into the
! configuration file at this point. Include directives simply look like:
! <programlisting>
! include 'filename'
! </programlisting>
! If the file name is not an absolute path, it is taken as relative to
! the directory containing the referencing configuration file.
! All types of inclusion directives can be nested.
! </para>
!
! <para>
! <indexterm>
! <primary><literal>include_dir</></primary>
! <secondary>in configuration file</secondary>
! </indexterm>
! The <filename>postgresql.conf</> file can also contain
! <firstterm>include_dir directives</>, which specify an entire directory
! of configuration files to include. It is used similarly:
! <programlisting>
! include_dir 'directory'
! </programlisting>
! Non-absolute directory names follow the same rules as single file include
! directives: they are relative to the directory containing the referencing
! configuration file. Within that directory, only files whose names end
! with the suffix <literal>.conf</literal> will be included. File names
! that start with the <literal>.</literal> character are also excluded,
! to prevent mistakes as they are hidden on some platforms. Multiple files
! within an include directory are ordered by an alphanumeric sorting, so
! that ones beginning with numbers are considered before those starting
! with letters.
! </para>
!
! <para>
! Include files or directories can be used to logically separate portions
! of the database configuration, rather than having a single large
! <filename>postgresql.conf</> file. Consider a company that has two
! database servers, each with a different amount of memory. There are likely
! elements of the configuration both will share, for things such as logging.
! But memory-related parameters on the server will vary between the two. And
! there might be server specific customizations too. One way to manage this
! situation is to break the custom configuration changes for your site into
! three files. You could add this to the end of your
! <filename>postgresql.conf</> file to include them:
! <programlisting>
! include 'shared.conf'
! include 'memory.conf'
! include 'server.conf'
! </programlisting>
! All systems would have the same <filename>shared.conf</>. Each server
! with a particular amount of memory could share the same
! <filename>memory.conf</>; you might have one for all servers with 8GB of RAM,
! another for those having 16GB. And finally <filename>server.conf</> could
! have truly server-specific configuration information in it.
! </para>
!
! <para>
! Another possibility for this same sort of organization is to create a
! configuration file directory and put this information into files there.
! Other programs such as <productname>Apache</productname> use a
! <filename>conf.d</> directory for this purpose. And using numbered names
! to enforce an ordering is common in UNIX startup script directories named
! like <filename>/etc/rcN.d</filename>, where <literal>N</> gives the
! associated system runlevel. Both of these ideas might be combined by
! adding a <filename>conf.d</> directory where the
! <filename>postgresql.conf</> file is at, then referencing it at the end:
! <programlisting>
! include_dir 'conf.d'
! </programlisting>
! Then you could name the files in the <filename>conf.d</> directory like this:
! <programlisting>
! include '00shared.conf'
! include '01memory.conf'
! include '02server.conf'
! </programlisting>
! This shows a clear order in which these files will be loaded. This is
! important because only the last setting encountered when the server is
! reading its configuration will be used. Something set in
! <filename>conf.d/02server.conf</> in this example would override a value
! set in <filename>conf.d/01memory.conf</>.
! </para>
!
! <para>
! You might instead use this configuration directory approach while naming
! these files more descriptively:
! <programlisting>
! 00shared.conf
! 01memory-8GB.conf
! 02server-foo.conf
! </programlisting>
! This sort of arrangement gives a unique name for each configuration file
! variation. This can help eliminate ambiguity when several servers have
! their configurations all stored in one place, such as in a version
! control repository. (Storing database configuration files under version
! control is another good practice to consider).
! </para>
! </sect2>
! </sect1>
<sect1 id="runtime-config-file-locations">
<title>File Locations</title>
diff --git a/src/backend/utils/misc/guc-file.l b/src/backend/utils/misc/guc-file.l
index a094c7a..ae33fdd 100644
*** a/src/backend/utils/misc/guc-file.l
--- b/src/backend/utils/misc/guc-file.l
*************** ProcessConfigFile(GucContext context)
*** 355,360 ****
--- 355,396 ----
}
/*
+ * Given a configuration file or directory location that may be a relative
+ * path, return an absolute one. We consider the location to be relative to
+ * the directory holding the calling file.
+ */
+ static char *
+ AbsoluteConfigLocation(const char *location,
+ const char *calling_file)
+ {
+ char abs_path[MAXPGPATH];
+
+ if (is_absolute_path(location))
+ {
+ return pstrdup(location);
+ }
+ else
+ {
+ if (calling_file != NULL)
+ {
+ strlcpy(abs_path, calling_file, sizeof(abs_path));
+ get_parent_directory(abs_path);
+ join_path_components(abs_path, abs_path, location);
+ canonicalize_path(abs_path);
+ }
+ else
+ {
+ /*
+ * calling_file is NULL, we make an absolute path from $PGDATA
+ */
+ join_path_components(abs_path, data_directory, location);
+ canonicalize_path(abs_path);
+ }
+ return pstrdup(abs_path);
+ }
+ }
+
+ /*
* Read and parse a single configuration file. This function recurses
* to handle "include" directives.
*
*************** ParseConfigFile(const char *config_file,
*** 370,376 ****
{
bool OK = true;
FILE *fp;
- char abs_path[MAXPGPATH];
/*
* Reject too-deep include nesting depth. This is just a safety check
--- 406,411 ----
*************** ParseConfigFile(const char *config_file,
*** 386,416 ****
return false;
}
! /*
! * If config_file is a relative path, convert to absolute. We consider
! * it to be relative to the directory holding the calling file.
! */
! if (!is_absolute_path(config_file))
! {
! if (calling_file != NULL)
! {
! strlcpy(abs_path, calling_file, sizeof(abs_path));
! get_parent_directory(abs_path);
! join_path_components(abs_path, abs_path, config_file);
! canonicalize_path(abs_path);
! config_file = abs_path;
! }
! else
! {
! /*
! * calling_file is NULL, we make an absolute path from $PGDATA
! */
! join_path_components(abs_path, data_directory, config_file);
! canonicalize_path(abs_path);
! config_file = abs_path;
! }
! }
!
fp = AllocateFile(config_file, "r");
if (!fp)
{
--- 421,427 ----
return false;
}
! config_file=AbsoluteConfigLocation(config_file,calling_file);
fp = AllocateFile(config_file, "r");
if (!fp)
{
*************** ParseConfigFp(FILE *fp, const char *conf
*** 512,518 ****
}
/* OK, process the option name and value */
! if (guc_name_compare(opt_name, "include") == 0)
{
/*
* An include directive isn't a variable and should be processed
--- 523,546 ----
}
/* OK, process the option name and value */
! if (guc_name_compare(opt_name, "include_dir") == 0)
! {
! /*
! * An include_dir directive isn't a variable and should be processed
! * immediately.
! */
! unsigned int save_ConfigFileLineno = ConfigFileLineno;
!
! if (!ParseConfigDirectory(opt_value, config_file,
! depth + 1, elevel,
! head_p, tail_p))
! OK = false;
! yy_switch_to_buffer(lex_buffer);
! ConfigFileLineno = save_ConfigFileLineno;
! pfree(opt_name);
! pfree(opt_value);
! }
! else if (guc_name_compare(opt_name, "include") == 0)
{
/*
* An include directive isn't a variable and should be processed
*************** ParseConfigFp(FILE *fp, const char *conf
*** 599,604 ****
--- 627,747 ----
return OK;
}
+ /*
+ * String comparison code for use by qsort. Borrowed from
+ * src/backend/tsearch/ts_utils.c
+ */
+ static int
+ comparestr(const void *a, const void *b)
+ {
+ return strcmp(*(char **) a, *(char **) b);
+ }
+
+ /*
+ * Read and parse all config files in a subdirectory in alphabetical order
+ */
+ bool
+ ParseConfigDirectory(const char *includedir,
+ const char *calling_file,
+ int depth, int elevel,
+ ConfigVariable **head_p,
+ ConfigVariable **tail_p)
+ {
+ char *directory;
+ DIR *d;
+ struct dirent *de;
+ char **filenames = NULL;
+ int num_filenames = 0;
+ int size_filenames = 0;
+ bool status;
+
+ directory=AbsoluteConfigLocation(includedir,calling_file);
+ d = AllocateDir(directory);
+ if (d == NULL)
+ {
+ ereport(elevel,
+ (errcode_for_file_access(),
+ errmsg("could not open configuration directory \"%s\": %m",
+ directory)));
+ return false;
+
+ }
+
+ /*
+ * Read the directory and put the filenames in an array, so we can sort
+ * them prior to processing the contents.
+ */
+ while ((de = ReadDir(d, directory)) != NULL)
+ {
+ struct stat st;
+ char filename[MAXPGPATH];
+
+ /*
+ * Only parse files with names ending in ".conf". Explicitly reject
+ * files starting with ".". This excludes things like "." and "..",
+ * as well as typical hidden files, backup files, and editor debris.
+ */
+ if (strlen(de->d_name) < 6)
+ continue;
+ if (strncmp(de->d_name, ".", 1) == 0)
+ continue;
+ if (strcmp(de->d_name + strlen(de->d_name) - 5, ".conf") != 0)
+ continue;
+
+ join_path_components(filename, directory, de->d_name);
+ if (stat(filename, &st) == 0)
+ {
+ if (!S_ISDIR(st.st_mode))
+ {
+ /* Add file to list, increasing its size in blocks of 32 */
+ if (num_filenames == size_filenames)
+ {
+ size_filenames += 32;
+ if (num_filenames == 0)
+ /* Must initialize, repalloc won't take NULL input */
+ filenames = palloc(size_filenames * sizeof(char *));
+ else
+ filenames = repalloc(filenames, size_filenames * sizeof(char *));
+ }
+ filenames[num_filenames] = pstrdup(filename);
+ num_filenames++;
+ }
+ else
+ /*
+ * stat does not care about permissions, so the most likely reason
+ * a file can't be accessed now is if it was removed between the
+ * directory listing and now.
+ */
+ {
+ ereport(elevel,
+ (errcode_for_file_access(),
+ errmsg("configuration file \"%s\" can no longer be accessed: %m",
+ filename)));
+ return false;
+ }
+ }
+ }
+
+ if (num_filenames > 0)
+ {
+ int i;
+ qsort(filenames, num_filenames, sizeof(char *), comparestr);
+ for (i = 0; i < num_filenames; i++)
+ {
+ if (!ParseConfigFile(filenames[i], NULL,
+ 0, elevel,head_p, tail_p))
+ {
+ status = false;
+ goto cleanup;
+ }
+ }
+ }
+ status = true;
+
+ cleanup:
+ FreeDir(d);
+ return status;
+ }
/*
* Free a list of ConfigVariables, including the names and the values
diff --git a/src/include/utils/guc.h b/src/include/utils/guc.h
index 8e3057a..2005ce9 100644
*** a/src/include/utils/guc.h
--- b/src/include/utils/guc.h
*************** extern bool ParseConfigFile(const char *
*** 116,121 ****
--- 116,126 ----
extern bool ParseConfigFp(FILE *fp, const char *config_file,
int depth, int elevel,
ConfigVariable **head_p, ConfigVariable **tail_p);
+ extern bool ParseConfigDirectory(const char *includedir,
+ const char *calling_file,
+ int depth, int elevel,
+ ConfigVariable **head_p,
+ ConfigVariable **tail_p);
extern void FreeConfigVariables(ConfigVariable *list);
/*
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
