Thread: Configuration patch
This patch incorporates a number of changes suggested by the group. The purpose of this patch is to move postgresql to a position where all configuration options are specified in one place. The postgresql.conf file could completely document a postgresql environment. It adds this functionality: The "-D' option will work as it always has if it is set to a standard postgresql database cluster directory. If it is set to a "postgresql.conf" file, it will use that file for configuration. If it is set to a directory which is not a cluster directory, i.e. "/somepath/etc" it will look for pg_hba.conf, pg_ident.conf, and postgresql.conf there. For postgresql to work only with a configuration file, some options have been added: include = '/etc/postgres/debug.conf' pgdata = '/vol01/postgres' hba_conf = '/etc/postgres/pg_hba_conf' ident_conf = '/etc/postgres/pg_ident.conf' runtime_pidfile = '/var/run/postgresql.conf' tablespace = '/somevol/somepath' "include" allows files with configuration parameters to be included. "pgdata" (used to be data_dir in old patch) tells PostgreSQL where it's database cluster directory is located. "hba_conf" tells PostgreSQL where to find pg_hba.conf file. "ident_conf" tells PostgreSQL where to find pg_ident.conf. "runtime_pidfile" tells postgres to write it's PID to a file that would be used by external applications. It is *NOT* the pid file which postgresql uses. "tablespace" allows postgresql to use alternate locations without environment variables. Using SIGHUP, tablespaces are reloaded. This allows you to add tablespaces to a running PostgreSQL process. (I know this has a limited lifetime, but it may make "CREATE DATABASE ... WITH LOCATION" a little bit more sane in the meantime.
Attachment
This looks very close to the method we had agreed to. I think it need a little adjustment, like removing tablespace now that the tablespaces patch is coming, but it is a great start. I could probably clean it up and apply it. Is that OK? --------------------------------------------------------------------------- pgsql@mohawksoft.com wrote: > This patch incorporates a number of changes suggested by the group. The > purpose of this patch is to move postgresql to a position where all > configuration options are specified in one place. The postgresql.conf file > could completely document a postgresql environment. > > > It adds this functionality: > > The "-D' option will work as it always has if it is set to a standard > postgresql database cluster directory. If it is set to a "postgresql.conf" > file, it will use that file for configuration. If it is set to a directory > which is not a cluster directory, i.e. "/somepath/etc" it will look for > pg_hba.conf, pg_ident.conf, and postgresql.conf there. > > For postgresql to work only with a configuration file, some options have > been added: > > include = '/etc/postgres/debug.conf' > pgdata = '/vol01/postgres' > hba_conf = '/etc/postgres/pg_hba_conf' > ident_conf = '/etc/postgres/pg_ident.conf' > runtime_pidfile = '/var/run/postgresql.conf' > tablespace = '/somevol/somepath' > > "include" allows files with configuration parameters to be included. > > "pgdata" (used to be data_dir in old patch) tells PostgreSQL where it's > database cluster directory is located. > > "hba_conf" tells PostgreSQL where to find pg_hba.conf file. > > "ident_conf" tells PostgreSQL where to find pg_ident.conf. > > "runtime_pidfile" tells postgres to write it's PID to a file that would be > used by external applications. It is *NOT* the pid file which postgresql > uses. > > "tablespace" allows postgresql to use alternate locations without > environment variables. Using SIGHUP, tablespaces are reloaded. This allows > you to add tablespaces to a running PostgreSQL process. (I know this has a > limited lifetime, but it may make "CREATE DATABASE ... WITH LOCATION" a > little bit more sane in the meantime. [ Attachment, skipping... ] > > ---------------------------(end of broadcast)--------------------------- > TIP 6: Have you searched our list archives? > > http://archives.postgresql.org -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073
[ Will apply with adjustment, removing tablespaces.
Your patch has been added to the PostgreSQL unapplied patches list at:
http://momjian.postgresql.org/cgi-bin/pgpatches
I will try to apply it within the next 48 hours.
---------------------------------------------------------------------------
pgsql@mohawksoft.com wrote:
> This patch incorporates a number of changes suggested by the group. The
> purpose of this patch is to move postgresql to a position where all
> configuration options are specified in one place. The postgresql.conf file
> could completely document a postgresql environment.
>
>
> It adds this functionality:
>
> The "-D' option will work as it always has if it is set to a standard
> postgresql database cluster directory. If it is set to a "postgresql.conf"
> file, it will use that file for configuration. If it is set to a directory
> which is not a cluster directory, i.e. "/somepath/etc" it will look for
> pg_hba.conf, pg_ident.conf, and postgresql.conf there.
>
> For postgresql to work only with a configuration file, some options have
> been added:
>
> include = '/etc/postgres/debug.conf'
> pgdata = '/vol01/postgres'
> hba_conf = '/etc/postgres/pg_hba_conf'
> ident_conf = '/etc/postgres/pg_ident.conf'
> runtime_pidfile = '/var/run/postgresql.conf'
> tablespace = '/somevol/somepath'
>
> "include" allows files with configuration parameters to be included.
>
> "pgdata" (used to be data_dir in old patch) tells PostgreSQL where it's
> database cluster directory is located.
>
> "hba_conf" tells PostgreSQL where to find pg_hba.conf file.
>
> "ident_conf" tells PostgreSQL where to find pg_ident.conf.
>
> "runtime_pidfile" tells postgres to write it's PID to a file that would be
> used by external applications. It is *NOT* the pid file which postgresql
> uses.
>
> "tablespace" allows postgresql to use alternate locations without
> environment variables. Using SIGHUP, tablespaces are reloaded. This allows
> you to add tablespaces to a running PostgreSQL process. (I know this has a
> limited lifetime, but it may make "CREATE DATABASE ... WITH LOCATION" a
> little bit more sane in the meantime.
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
I am working on integrating this patch now.
What is the logic if checkConfigDir(). It seems incompleted or wrong.
Your code had:
+ if(S_ISREG(stat_buf.st_mode)) /* It's a regular file, so assume it's an explict */
+ {
+ return TRUE;
+ }
+ else if(S_ISDIR(stat_buf.st_mode)) /* It's a directory, is it a config or system dir */
+ {
+ snprintf(path, sizeof(path), "%s/global/pg_control", checkdir);
+ /* If this is not found, then, hey it wasn't going to work as a data dir either. */
+ if(stat(path, &stat_buf) == -1)
+ return TRUE;
+ }
+ return FALSE;
The last stat() seems to say that if the directory has no
global/pg_control, then it is good a a config-only directory, and if not
it is a data directory. Is that right?
---------------------------------------------------------------------------
pgsql@mohawksoft.com wrote:
> This patch incorporates a number of changes suggested by the group. The
> purpose of this patch is to move postgresql to a position where all
> configuration options are specified in one place. The postgresql.conf file
> could completely document a postgresql environment.
>
>
> It adds this functionality:
>
> The "-D' option will work as it always has if it is set to a standard
> postgresql database cluster directory. If it is set to a "postgresql.conf"
> file, it will use that file for configuration. If it is set to a directory
> which is not a cluster directory, i.e. "/somepath/etc" it will look for
> pg_hba.conf, pg_ident.conf, and postgresql.conf there.
>
> For postgresql to work only with a configuration file, some options have
> been added:
>
> include = '/etc/postgres/debug.conf'
> pgdata = '/vol01/postgres'
> hba_conf = '/etc/postgres/pg_hba_conf'
> ident_conf = '/etc/postgres/pg_ident.conf'
> runtime_pidfile = '/var/run/postgresql.conf'
> tablespace = '/somevol/somepath'
>
> "include" allows files with configuration parameters to be included.
>
> "pgdata" (used to be data_dir in old patch) tells PostgreSQL where it's
> database cluster directory is located.
>
> "hba_conf" tells PostgreSQL where to find pg_hba.conf file.
>
> "ident_conf" tells PostgreSQL where to find pg_ident.conf.
>
> "runtime_pidfile" tells postgres to write it's PID to a file that would be
> used by external applications. It is *NOT* the pid file which postgresql
> uses.
>
> "tablespace" allows postgresql to use alternate locations without
> environment variables. Using SIGHUP, tablespaces are reloaded. This allows
> you to add tablespaces to a running PostgreSQL process. (I know this has a
> limited lifetime, but it may make "CREATE DATABASE ... WITH LOCATION" a
> little bit more sane in the meantime.
[ Attachment, skipping... ]
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
>
> I am working on integrating this patch now.
>
> What is the logic if checkConfigDir(). It seems incompleted or wrong.
>
> Your code had:
>
> + if(S_ISREG(stat_buf.st_mode)) /* It's a regular file, so
> assume it's an explict */
> + {
> + return TRUE;
> + }
> + else if(S_ISDIR(stat_buf.st_mode)) /* It's a directory, is it
> a config or system dir */
> + {
> + snprintf(path, sizeof(path), "%s/global/pg_control",
> checkdir);
> + /* If this is not found, then, hey it wasn't going to work
> as a data dir either. */
> + if(stat(path, &stat_buf) == -1)
> + return TRUE;
> + }
> + return FALSE;
>
> The last stat() seems to say that if the directory has no
> global/pg_control, then it is good a a config-only directory, and if not
> it is a data directory. Is that right?
Yes, that is right. If "pg_control" exists in the directory, then we
should not assume it is a config-only directory.
>
> ---------------------------------------------------------------------------
>
> pgsql@mohawksoft.com wrote:
>> This patch incorporates a number of changes suggested by the group. The
>> purpose of this patch is to move postgresql to a position where all
>> configuration options are specified in one place. The postgresql.conf
>> file
>> could completely document a postgresql environment.
>>
>>
>> It adds this functionality:
>>
>> The "-D' option will work as it always has if it is set to a standard
>> postgresql database cluster directory. If it is set to a
>> "postgresql.conf"
>> file, it will use that file for configuration. If it is set to a
>> directory
>> which is not a cluster directory, i.e. "/somepath/etc" it will look for
>> pg_hba.conf, pg_ident.conf, and postgresql.conf there.
>>
>> For postgresql to work only with a configuration file, some options have
>> been added:
>>
>> include = '/etc/postgres/debug.conf'
>> pgdata = '/vol01/postgres'
>> hba_conf = '/etc/postgres/pg_hba_conf'
>> ident_conf = '/etc/postgres/pg_ident.conf'
>> runtime_pidfile = '/var/run/postgresql.conf'
>> tablespace = '/somevol/somepath'
>>
>> "include" allows files with configuration parameters to be included.
>>
>> "pgdata" (used to be data_dir in old patch) tells PostgreSQL where it's
>> database cluster directory is located.
>>
>> "hba_conf" tells PostgreSQL where to find pg_hba.conf file.
>>
>> "ident_conf" tells PostgreSQL where to find pg_ident.conf.
>>
>> "runtime_pidfile" tells postgres to write it's PID to a file that would
>> be
>> used by external applications. It is *NOT* the pid file which postgresql
>> uses.
>>
>> "tablespace" allows postgresql to use alternate locations without
>> environment variables. Using SIGHUP, tablespaces are reloaded. This
>> allows
>> you to add tablespaces to a running PostgreSQL process. (I know this has
>> a
>> limited lifetime, but it may make "CREATE DATABASE ... WITH LOCATION" a
>> little bit more sane in the meantime.
>
> [ Attachment, skipping... ]
>
>>
>> ---------------------------(end of broadcast)---------------------------
>> TIP 6: Have you searched our list archives?
>>
>> http://archives.postgresql.org
>
> --
> Bruce Momjian | http://candle.pha.pa.us
> pgman@candle.pha.pa.us | (610) 359-1001
> + If your life is a hard drive, | 13 Roberts Road
> + Christ can be your backup. | Newtown Square, Pennsylvania
> 19073
>
Here is an updated version of your patch. I removed the tablespace part because we are going to have real tablespaces in 7.5 rather than initlocation hacks. I added documention of the new guc parameters, and a paragraph in the postmaster manual page describing the new -D/PGDATA behavior. (Is that enough?) I see a few malloc/strup's in the guc-file.l patch, but they seem OK. I cleaned up the patch to be more intuitive in its use of variable names and structure. I will apply it in a day or if I get other feedback. --------------------------------------------------------------------------- pgsql@mohawksoft.com wrote: > This patch incorporates a number of changes suggested by the group. The > purpose of this patch is to move postgresql to a position where all > configuration options are specified in one place. The postgresql.conf file > could completely document a postgresql environment. > > > It adds this functionality: > > The "-D' option will work as it always has if it is set to a standard > postgresql database cluster directory. If it is set to a "postgresql.conf" > file, it will use that file for configuration. If it is set to a directory > which is not a cluster directory, i.e. "/somepath/etc" it will look for > pg_hba.conf, pg_ident.conf, and postgresql.conf there. > > For postgresql to work only with a configuration file, some options have > been added: > > include = '/etc/postgres/debug.conf' > pgdata = '/vol01/postgres' > hba_conf = '/etc/postgres/pg_hba_conf' > ident_conf = '/etc/postgres/pg_ident.conf' > runtime_pidfile = '/var/run/postgresql.conf' > tablespace = '/somevol/somepath' > > "include" allows files with configuration parameters to be included. > > "pgdata" (used to be data_dir in old patch) tells PostgreSQL where it's > database cluster directory is located. > > "hba_conf" tells PostgreSQL where to find pg_hba.conf file. > > "ident_conf" tells PostgreSQL where to find pg_ident.conf. > > "runtime_pidfile" tells postgres to write it's PID to a file that would be > used by external applications. It is *NOT* the pid file which postgresql > uses. > > "tablespace" allows postgresql to use alternate locations without > environment variables. Using SIGHUP, tablespaces are reloaded. This allows > you to add tablespaces to a running PostgreSQL process. (I know this has a > limited lifetime, but it may make "CREATE DATABASE ... WITH LOCATION" a > little bit more sane in the meantime. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073 Index: doc/src/sgml/runtime.sgml =================================================================== RCS file: /cvsroot/pgsql-server/doc/src/sgml/runtime.sgml,v retrieving revision 1.265 diff -c -c -r1.265 runtime.sgml *** doc/src/sgml/runtime.sgml 26 May 2004 18:51:43 -0000 1.265 --- doc/src/sgml/runtime.sgml 2 Jun 2004 20:31:36 -0000 *************** *** 563,568 **** --- 563,625 ---- any desired selection condition. </para> + <sect2 id="runtime-config-configuration-files"> + <title>Configuration Files</title> + + <variablelist> + + <varlistentry id="guc-include" xreflabel="include"> + <term><varname>include</varname> (<type>string</type>)</term> + <listitem> + <para> + Specifies an additional file to read for configuration settings. + </para> + </listitem> + </varlistentry> + + <varlistentry id="guc-hba-conf" xreflabel="hba-conf"> + <term><varname>hba_conf</varname> (<type>string</type>)</term> + <listitem> + <para> + Specifies the file name to use for configuration of host-based + authentication (HBA). + </para> + </listitem> + </varlistentry> + + <varlistentry id="guc-ident-conf" xreflabel="ident-conf"> + <term><varname>ident_conf</varname> (<type>string</type>)</term> + <listitem> + <para> + Specifies the file name to use for configuration of + <application>ident</> authentication. + </para> + </listitem> + </varlistentry> + + <varlistentry id="guc-pgdata" xreflabel="pgdata"> + <term><varname>pgdata</varname> (<type>string</type>)</term> + <listitem> + <para> + Specifies the directory to use for data storage (everything except + configuration files). + </para> + </listitem> + </varlistentry> + + <varlistentry id="external-pidfile" xreflabel="external-pidfile"> + <term><varname>external_pidfile</varname> (<type>string</type>)</term> + <listitem> + <para> + Specifies the location of an additional <application>postmaster</> + process-id (PID) file for use by server administration programs. + </para> + </listitem> + </varlistentry> + + </variablelist> + </sect2> + <sect2 id="runtime-config-connection"> <title>Connections and Authentication</title> Index: doc/src/sgml/ref/postmaster.sgml =================================================================== RCS file: /cvsroot/pgsql-server/doc/src/sgml/ref/postmaster.sgml,v retrieving revision 1.49 diff -c -c -r1.49 postmaster.sgml *** doc/src/sgml/ref/postmaster.sgml 23 Mar 2004 06:09:00 -0000 1.49 --- doc/src/sgml/ref/postmaster.sgml 2 Jun 2004 20:31:36 -0000 *************** *** 67,80 **** One <command>postmaster</command> always manages the data from exactly one database cluster. A database cluster is a collection of databases that is stored at a common file system ! location. When the <command>postmaster</command> starts it needs to know the location ! of the database cluster files (<quote>data area</quote>). This is ! done with the <option>-D</option> invocation option or the ! <envar>PGDATA</envar> environment variable; there is no default. ! More than one <command>postmaster</command> process can run on a system at one time, ! as long as they use different data areas and different communication ports (see below). A data area is created with <xref linkend="app-initdb">. </para> </refsect1> --- 67,92 ---- One <command>postmaster</command> always manages the data from exactly one database cluster. A database cluster is a collection of databases that is stored at a common file system ! location. When the <command>postmaster</command> starts it needs ! to know the location of the database cluster files (<quote>data ! area</quote>). ! More than one <command>postmaster</command> process can run on a system ! at one time as long as they use different data areas and different communication ports (see below). A data area is created with <xref linkend="app-initdb">. + </para> + + <para> + The <quote>data area</> is specified by the <option>-D</option> option + or the <envar>PGDATA</envar> environment variable; there is no default. + They typically point to a directory created by <application> + initdb</>. However, for administrative flexibility, you can + point directly to a configuration file like <filename>postgresql.conf</>. + This file must then specify the location of data directory using + <filename>postgresql.conf</>'s variable <varname>pgdata</>. + You can also point to a directory containing just configuration files, + and use <filename>postgresql.conf</>'s <varname>pgdata</> to point + to the directory containing the remaining files. </para> </refsect1> Index: src/backend/bootstrap/bootstrap.c =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/bootstrap/bootstrap.c,v retrieving revision 1.182 diff -c -c -r1.182 bootstrap.c *** src/backend/bootstrap/bootstrap.c 29 May 2004 22:48:18 -0000 1.182 --- src/backend/bootstrap/bootstrap.c 2 Jun 2004 20:31:37 -0000 *************** *** 213,219 **** char *dbname; int flag; int xlogop = BS_XLOG_NOP; ! char *potential_DataDir = NULL; /* * initialize globals --- 213,219 ---- char *dbname; int flag; int xlogop = BS_XLOG_NOP; ! char *userPGDATA = NULL; /* * initialize globals *************** *** 237,244 **** if (!IsUnderPostmaster) { InitializeGUCOptions(); ! potential_DataDir = getenv("PGDATA"); /* Null if no PGDATA ! * variable */ } /* Ignore the initial -boot argument, if present */ --- 237,243 ---- if (!IsUnderPostmaster) { InitializeGUCOptions(); ! userPGDATA = getenv("PGDATA"); /* Null if no PGDATA variable */ } /* Ignore the initial -boot argument, if present */ *************** *** 253,259 **** switch (flag) { case 'D': ! potential_DataDir = optarg; break; case 'd': { --- 252,258 ---- switch (flag) { case 'D': ! userPGDATA = optarg; break; case 'd': { *************** *** 327,333 **** if (!IsUnderPostmaster) { ! if (!potential_DataDir) { fprintf(stderr, gettext("%s does not know where to find the database system data.\n" --- 326,332 ---- if (!IsUnderPostmaster) { ! if (!userPGDATA) { fprintf(stderr, gettext("%s does not know where to find the database system data.\n" *************** *** 337,343 **** argv[0]); proc_exit(1); } ! SetDataDir(potential_DataDir); } /* Validate we have been given a reasonable-looking DataDir */ --- 336,342 ---- argv[0]); proc_exit(1); } ! SetDataDir(userPGDATA); } /* Validate we have been given a reasonable-looking DataDir */ Index: src/backend/libpq/hba.c =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/libpq/hba.c,v retrieving revision 1.125 diff -c -c -r1.125 hba.c *** src/backend/libpq/hba.c 30 May 2004 23:40:26 -0000 1.125 --- src/backend/libpq/hba.c 2 Jun 2004 20:31:38 -0000 *************** *** 35,40 **** --- 35,41 ---- #include "miscadmin.h" #include "nodes/pg_list.h" #include "storage/fd.h" + #include "utils/guc.h" /* Max size of username ident server can return */ *************** *** 1029,1045 **** void load_hba(void) { - int bufsize; FILE *file; /* The config file we have to read */ char *conf_file; /* The name of the config file */ if (hba_lines || hba_line_nums) free_lines(&hba_lines, &hba_line_nums); ! /* Put together the full pathname to the config file. */ ! bufsize = (strlen(DataDir) + strlen(CONF_FILE) + 2) * sizeof(char); ! conf_file = (char *) palloc(bufsize); ! snprintf(conf_file, bufsize, "%s/%s", DataDir, CONF_FILE); file = AllocateFile(conf_file, "r"); if (file == NULL) --- 1030,1051 ---- void load_hba(void) { FILE *file; /* The config file we have to read */ char *conf_file; /* The name of the config file */ if (hba_lines || hba_line_nums) free_lines(&hba_lines, &hba_line_nums); ! /* HBA filename in config file */ ! if (guc_hbafile) ! conf_file = pstrdup(guc_hbafile); ! else ! { ! char *confloc = (user_pgconfig_is_dir) ? user_pgconfig : DataDir; ! /* put together the full pathname to the config file */ ! conf_file = palloc(strlen(confloc) + strlen(CONF_FILE) + 2); ! sprintf(conf_file, "%s/%s", confloc, CONF_FILE); ! } file = AllocateFile(conf_file, "r"); if (file == NULL) *************** *** 1178,1193 **** FILE *file; /* The map file we have to read */ char *map_file; /* The name of the map file we have to * read */ - int bufsize; - if (ident_lines || ident_line_nums) free_lines(&ident_lines, &ident_line_nums); ! /* put together the full pathname to the map file */ ! bufsize = (strlen(DataDir) + strlen(USERMAP_FILE) + 2) * sizeof(char); ! map_file = (char *) palloc(bufsize); ! snprintf(map_file, bufsize, "%s/%s", DataDir, USERMAP_FILE); ! file = AllocateFile(map_file, "r"); if (file == NULL) { --- 1184,1203 ---- FILE *file; /* The map file we have to read */ char *map_file; /* The name of the map file we have to * read */ if (ident_lines || ident_line_nums) free_lines(&ident_lines, &ident_line_nums); ! /* IDENT filename in config file */ ! if (guc_identfile) ! map_file = pstrdup(guc_identfile); ! else ! { ! /* put together the full pathname to the map file */ ! char *confloc = (user_pgconfig_is_dir) ? user_pgconfig : DataDir; ! map_file = (char *) palloc(strlen(confloc) + strlen(USERMAP_FILE) + 2); ! sprintf(map_file, "%s/%s", confloc, USERMAP_FILE); ! } ! file = AllocateFile(map_file, "r"); if (file == NULL) { Index: src/backend/postmaster/postmaster.c =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/postmaster/postmaster.c,v retrieving revision 1.401 diff -c -c -r1.401 postmaster.c *** src/backend/postmaster/postmaster.c 30 May 2004 03:50:11 -0000 1.401 --- src/backend/postmaster/postmaster.c 2 Jun 2004 20:31:41 -0000 *************** *** 226,231 **** --- 226,232 ---- * postmaster.c - function prototypes */ static void checkDataDir(const char *checkdir); + static bool onlyConfigSpecified(const char *checkdir); #ifdef USE_RENDEZVOUS static void reg_reply(DNSServiceRegistrationReplyErrorType errorCode, void *context); *************** *** 303,309 **** { int opt; int status; ! char *potential_DataDir = NULL; int i; progname = get_progname(argv[0]); --- 304,310 ---- { int opt; int status; ! char *userPGDATA = NULL; int i; progname = get_progname(argv[0]); *************** *** 367,373 **** */ InitializeGUCOptions(); ! potential_DataDir = getenv("PGDATA"); /* default value */ opterr = 1; --- 368,374 ---- */ InitializeGUCOptions(); ! userPGDATA = getenv("PGDATA"); /* default value */ opterr = 1; *************** *** 392,398 **** /* Can no longer set the backend executable file to use. */ break; case 'D': ! potential_DataDir = optarg; break; case 'd': { --- 393,399 ---- /* Can no longer set the backend executable file to use. */ break; case 'D': ! userPGDATA = optarg; break; case 'd': { *************** *** 521,533 **** ExitPostmaster(1); } ! /* ! * Now we can set the data directory, and then read postgresql.conf. ! */ ! checkDataDir(potential_DataDir); /* issues error messages */ ! SetDataDir(potential_DataDir); ! ProcessConfigFile(PGC_POSTMASTER); /* If timezone is not set, determine what the OS uses */ pg_timezone_initialize(); --- 522,565 ---- ExitPostmaster(1); } ! if (onlyConfigSpecified(userPGDATA)) ! { ! /* ! * It is either a file name or a directory with no ! * global/pg_control file, and hence not a data directory. ! */ ! user_pgconfig = userPGDATA; ! ProcessConfigFile(PGC_POSTMASTER); ! ! if (!guc_pgdata) /* Got a pgdata from the config file? */ ! checkDataDir(NULL); /* throw error */ ! ! checkDataDir(guc_pgdata); ! SetDataDir(guc_pgdata); ! } ! else ! { ! /* Now we can set the data directory, and then read postgresql.conf. */ ! checkDataDir(userPGDATA); ! SetDataDir(userPGDATA); ! ProcessConfigFile(PGC_POSTMASTER); ! } ! ! if (external_pidfile) ! { ! FILE *fpidfile = fopen(external_pidfile, "w"); ! if (fpidfile) ! { ! fprintf(fpidfile, "%d\n", MyProcPid); ! fclose(fpidfile); ! /* Should we remove the pid file on postmaster exit? */ ! } ! else ! fprintf(stderr, ! gettext("%s could not write to external pid file %s\n"), ! progname, external_pidfile); ! } /* If timezone is not set, determine what the OS uses */ pg_timezone_initialize(); *************** *** 839,844 **** --- 871,902 ---- ExitPostmaster(status != STATUS_OK); return 0; /* not reached */ + } + + + + static bool + onlyConfigSpecified(const char *checkdir) + { + char path[MAXPGPATH]; + struct stat stat_buf; + + if (checkdir == NULL) /* checkDataDir handles this */ + return FALSE; + + if (stat(checkdir, &stat_buf) == -1) /* ditto */ + return FALSE; + + if (S_ISREG(stat_buf.st_mode)) /* It's a regular file, so assume it's explict */ + return TRUE; + else if (S_ISDIR(stat_buf.st_mode)) /* It's a directory, is it a config or system dir? */ + { + snprintf(path, MAXPGPATH, "%s/global/pg_control", checkdir); + /* If this is not found, it is a config-only directory */ + if (stat(path, &stat_buf) == -1) + return TRUE; + } + return FALSE; } Index: src/backend/tcop/postgres.c =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/tcop/postgres.c,v retrieving revision 1.417 diff -c -c -r1.417 postgres.c *** src/backend/tcop/postgres.c 29 May 2004 22:48:20 -0000 1.417 --- src/backend/tcop/postgres.c 2 Jun 2004 20:31:43 -0000 *************** *** 2137,2143 **** { int flag; const char *dbname = NULL; ! char *potential_DataDir = NULL; bool secure; int errs = 0; int debug_flag = 0; --- 2137,2143 ---- { int flag; const char *dbname = NULL; ! char *userPGDATA = NULL; bool secure; int errs = 0; int debug_flag = 0; *************** *** 2208,2214 **** if (!IsUnderPostmaster) { InitializeGUCOptions(); ! potential_DataDir = getenv("PGDATA"); } /* ---------------- --- 2208,2214 ---- if (!IsUnderPostmaster) { InitializeGUCOptions(); ! userPGDATA = getenv("PGDATA"); } /* ---------------- *************** *** 2256,2262 **** case 'D': /* PGDATA directory */ if (secure) ! potential_DataDir = optarg; break; case 'd': /* debug level */ --- 2256,2262 ---- case 'D': /* PGDATA directory */ if (secure) ! userPGDATA = optarg; break; case 'd': /* debug level */ *************** *** 2547,2558 **** * set up handler to log session end. */ if (IsUnderPostmaster && Log_disconnections) ! on_proc_exit(log_disconnections,0); } if (!IsUnderPostmaster) { ! if (!potential_DataDir) { fprintf(stderr, gettext("%s does not know where to find the database system data.\n" --- 2547,2558 ---- * set up handler to log session end. */ if (IsUnderPostmaster && Log_disconnections) ! on_proc_exit(log_disconnections, 0); } if (!IsUnderPostmaster) { ! if (!userPGDATA) { fprintf(stderr, gettext("%s does not know where to find the database system data.\n" *************** *** 2562,2568 **** argv[0]); proc_exit(1); } ! SetDataDir(potential_DataDir); } Assert(DataDir); --- 2562,2568 ---- argv[0]); proc_exit(1); } ! SetDataDir(userPGDATA); } Assert(DataDir); Index: src/backend/utils/misc/guc-file.l =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/utils/misc/guc-file.l,v retrieving revision 1.22 diff -c -c -r1.22 guc-file.l *** src/backend/utils/misc/guc-file.l 26 May 2004 15:07:38 -0000 1.22 --- src/backend/utils/misc/guc-file.l 2 Jun 2004 20:31:44 -0000 *************** *** 129,162 **** * function does not return if an error occurs. If an error occurs, no * values will be changed. */ ! void ! ProcessConfigFile(GucContext context) { int token, parse_state; char *opt_name, *opt_value; - char *filename; struct name_value_pair *item, *head, *tail; int elevel; FILE * fp; - Assert(context == PGC_POSTMASTER || context == PGC_BACKEND - || context == PGC_SIGHUP); - Assert(DataDir); elevel = (context == PGC_SIGHUP) ? DEBUG4 : ERROR; - /* - * Open file - */ - filename = malloc(strlen(DataDir) + strlen(CONFIG_FILENAME) + 2); - if (filename == NULL) - { - ereport(elevel, - (errcode(ERRCODE_OUT_OF_MEMORY), - errmsg("out of memory"))); - return; - } - sprintf(filename, "%s/" CONFIG_FILENAME, DataDir); - fp = AllocateFile(filename, "r"); if (!fp) { --- 129,145 ---- * function does not return if an error occurs. If an error occurs, no * values will be changed. */ ! static void ! ReadConfigFile(char *filename, GucContext context) { int token, parse_state; char *opt_name, *opt_value; struct name_value_pair *item, *head, *tail; int elevel; FILE * fp; elevel = (context == PGC_SIGHUP) ? DEBUG4 : ERROR; fp = AllocateFile(filename, "r"); if (!fp) { *************** *** 165,171 **** if (errno != ENOENT) ereport(elevel, (errcode_for_file_access(), ! errmsg("could not open configuration file \"%s\": %m", CONFIG_FILENAME))); return; } --- 148,154 ---- if (errno != ENOENT) ereport(elevel, (errcode_for_file_access(), ! errmsg("could not open configuration file \"%s\": %m", filename))); return; } *************** *** 197,203 **** token = yylex(); if (token != GUC_ID && token != GUC_STRING && ! token != GUC_INTEGER && token != GUC_REAL && token != GUC_UNQUOTED_STRING) goto parse_error; opt_value = strdup(yytext); --- 180,187 ---- token = yylex(); if (token != GUC_ID && token != GUC_STRING && ! token != GUC_INTEGER && ! token != GUC_REAL && token != GUC_UNQUOTED_STRING) goto parse_error; opt_value = strdup(yytext); *************** *** 259,265 **** } FreeFile(fp); - free(filename); /* * Check if all options are valid --- 243,248 ---- *************** *** 282,293 **** parse_error: FreeFile(fp); - free(filename); free_name_value_list(head); ereport(elevel, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("syntax error in file \"%s\" line %u, near token \"%s\"", ! CONFIG_FILENAME, ConfigFileLineno, yytext))); return; out_of_memory: --- 265,275 ---- parse_error: FreeFile(fp); free_name_value_list(head); ereport(elevel, (errcode(ERRCODE_SYNTAX_ERROR), errmsg("syntax error in file \"%s\" line %u, near token \"%s\"", ! filename, ConfigFileLineno, yytext))); return; out_of_memory: *************** *** 298,303 **** --- 280,344 ---- (errcode(ERRCODE_OUT_OF_MEMORY), errmsg("out of memory"))); return; + } + + /* + * Function to read and process the configuration file. The + * parameter indicates the context that the file is being read + * (postmaster startup, backend startup, or SIGHUP). All options + * mentioned in the configuration file are set to new values. This + * function does not return if an error occurs. If an error occurs, no + * values will be changed. + */ + void + ProcessConfigFile(GucContext context) + { + char *filename; + + Assert(context == PGC_POSTMASTER || context == PGC_BACKEND || context == PGC_SIGHUP); + + /* Added for explicit config file */ + if (user_pgconfig) + { + struct stat sb; + + if (stat(user_pgconfig, &sb) != 0) + { + int elevel = (context == PGC_SIGHUP) ? DEBUG3 : ERROR; + elog(elevel, "Configuration file \"%s\" does not exist", user_pgconfig); + return; + } + + if (S_ISDIR(sb.st_mode)) + { + /* This will cause a small one time memory leak + * if the user also specifies hba_conf, + * ident_conf, and data_dir + */ + filename = malloc(strlen(user_pgconfig) + strlen(CONFIG_FILENAME) + 2); + sprintf(filename, "%s/%s", user_pgconfig, CONFIG_FILENAME); + user_pgconfig_is_dir = true; + } + else + filename = strdup(user_pgconfig); /* Use explicit file */ + } + else + { + /* Use datadir for config */ + filename = malloc(strlen(DataDir) + strlen(CONFIG_FILENAME) + 2); + sprintf(filename, "%s/%s", DataDir, CONFIG_FILENAME); + } + + if (filename == NULL) + { + int elevel = (context == PGC_SIGHUP) ? DEBUG3 : ERROR; + elog(elevel, "out of memory"); + return; + } + + ReadConfigFile(filename, context); + + free(filename); } Index: src/backend/utils/misc/guc.c =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/utils/misc/guc.c,v retrieving revision 1.210 diff -c -c -r1.210 guc.c *** src/backend/utils/misc/guc.c 30 May 2004 23:40:38 -0000 1.210 --- src/backend/utils/misc/guc.c 2 Jun 2004 20:31:47 -0000 *************** *** 57,62 **** --- 57,69 ---- #include "utils/pg_locale.h" #include "pgstat.h" + char *guc_pgdata; + char *guc_hbafile; + char *guc_identfile; + char *external_pidfile; + + char *user_pgconfig = NULL; + bool user_pgconfig_is_dir = false; #ifndef PG_KRB_SRVTAB #define PG_KRB_SRVTAB "" *************** *** 106,111 **** --- 113,119 ---- static bool assign_stage_log_stats(bool newval, bool doit, GucSource source); static bool assign_log_stats(bool newval, bool doit, GucSource source); + static void ReadConfigFile(char *filename, GucContext context); /* * Debugging options *************** *** 174,179 **** --- 182,194 ---- static int block_size; static bool integer_datetimes; + struct config_function + { + struct config_generic gen; + void (*function)(char *param, GucContext context); + }; + + /* Macros for freeing malloc'd pointers only if appropriate to do so */ /* Some of these tests are probably redundant, but be safe ... */ #define SET_STRING_VARIABLE(rec, newval) \ *************** *** 336,342 **** /* PGC_BOOL */ "bool", /* PGC_INT */ "integer", /* PGC_REAL */ "real", ! /* PGC_STRING */ "string" }; --- 351,358 ---- /* PGC_BOOL */ "bool", /* PGC_INT */ "integer", /* PGC_REAL */ "real", ! /* PGC_STRING */ "string", ! /* PGC_FUNCTION */ "function" }; *************** *** 1739,1753 **** NULL, assign_custom_variable_classes, NULL }, /* End-of-list marker */ { {NULL, 0, 0, NULL, NULL}, NULL, NULL, NULL, NULL } }; - /******** end of options list ********/ /* * To allow continued support of obsolete names for GUC variables, we apply * the following mappings to any unrecognized name. Note that an old name --- 1755,1800 ---- NULL, assign_custom_variable_classes, NULL }, + { + {"pgdata", PGC_POSTMASTER, 0, gettext_noop("Sets the location of the data directory"), NULL}, + &guc_pgdata, + NULL, NULL, NULL + }, + + { + {"hba_conf", PGC_SIGHUP, 0, gettext_noop("Sets the location of the \"hba\" configuration file"), NULL}, + &guc_hbafile, + NULL, NULL, NULL + }, + + { + {"ident_conf", PGC_SIGHUP, 0, gettext_noop("Sets the location of the \"ident\" configuration file"), NULL}, + &guc_identfile, + NULL, NULL, NULL + }, + + { + {"external_pidfile", PGC_POSTMASTER, 0, gettext_noop("Writes the postmaster PID to the specified file"), NULL}, + &external_pidfile, + NULL, NULL, NULL + }, + /* End-of-list marker */ { {NULL, 0, 0, NULL, NULL}, NULL, NULL, NULL, NULL } }; + static struct config_function ConfigureFunctions[] = + { + { {"include", PGC_POSTMASTER}, ReadConfigFile}, + { {NULL,0}, NULL} + }; + + /******** end of options list ********/ + /* * To allow continued support of obsolete names for GUC variables, we apply * the following mappings to any unrecognized name. Note that an old name *************** *** 1837,1842 **** --- 1884,1897 ---- num_vars++; } + for(i = 0; ConfigureFunctions[i].gen.name; i++) + { + struct config_function *conf = &ConfigureFunctions[i]; + + conf->gen.vartype = PGC_FUNCTION; + num_vars++; + } + /* Create table with 20% slack */ size_vars = num_vars + num_vars / 4; *************** *** 1862,1867 **** --- 1917,1925 ---- for (i = 0; ConfigureNamesString[i].gen.name; i++) guc_vars[num_vars++] = &ConfigureNamesString[i].gen; + for (i = 0; ConfigureFunctions[i].gen.name; i++) + guc_vars[num_vars++] = &ConfigureFunctions[i].gen; + if (guc_variables) free(guc_variables); guc_variables = guc_vars; *************** *** 2193,2198 **** --- 2251,2258 ---- conf->session_val = str; break; } + case PGC_FUNCTION: /* Nothing to do */ + break; } } *************** *** 2344,2349 **** --- 2404,2411 ---- guc_dirty = true; break; } + case PGC_FUNCTION: /* Nothing to do */ + break; } if (gconf->flags & GUC_REPORT) *************** *** 2500,2505 **** --- 2562,2569 ---- conf->gen.status = 0; break; } + case PGC_FUNCTION: /* Nothing to do */ + break; } if (changed && (gconf->flags & GUC_REPORT)) *************** *** 3301,3306 **** --- 3365,3382 ---- free(newval); break; } + + case PGC_FUNCTION: + if (!changeVal) + { + /* During the "checking" stage of configuration + * read, run functions + */ + struct config_function *fn = + (struct config_function *)record; + fn->function((char *)value, context); + } + break; } if (changeVal && (record->flags & GUC_REPORT)) *************** *** 3361,3366 **** --- 3437,3446 ---- case PGC_STRING: return *((struct config_string *) record)->variable; + + case PGC_FUNCTION: + /* Should never really happen */ + return NULL; } return NULL; } *************** *** 3397,3402 **** --- 3477,3486 ---- case PGC_STRING: return ((struct config_string *) record)->reset_val; + + case PGC_FUNCTION: + /* Should never really happen */ + return NULL; } return NULL; } *************** *** 4351,4356 **** --- 4435,4443 ---- fprintf(fp, "%s", *conf->variable); } break; + + case PGC_FUNCTION: /* do nothing */ + break; } fputc(0, fp); *************** *** 5131,5133 **** --- 5218,5221 ---- #include "guc-file.c" + Index: src/backend/utils/misc/postgresql.conf.sample =================================================================== RCS file: /cvsroot/pgsql-server/src/backend/utils/misc/postgresql.conf.sample,v retrieving revision 1.113 diff -c -c -r1.113 postgresql.conf.sample *** src/backend/utils/misc/postgresql.conf.sample 7 Apr 2004 05:05:50 -0000 1.113 --- src/backend/utils/misc/postgresql.conf.sample 2 Jun 2004 20:31:47 -0000 *************** *** 22,27 **** --- 22,38 ---- #--------------------------------------------------------------------------- + # CONFIGURATION FILES + #--------------------------------------------------------------------------- + + # include = '/somedir/pgdefs.conf' # include another file + # hba_conf = '/etc/postgres/pg_hba.conf' # use file in another directory + # ident_conf = '/etc/postgres/pg_ident.conf' # use file in another directory + # pgdata = '/usr/local/pgsql/data' # use /data in another directory + # external_pidfile= '/var/run/postgresql.pid' # write an extra pid file + + + #--------------------------------------------------------------------------- # CONNECTIONS AND AUTHENTICATION #--------------------------------------------------------------------------- Index: src/include/utils/guc.h =================================================================== RCS file: /cvsroot/pgsql-server/src/include/utils/guc.h,v retrieving revision 1.47 diff -c -c -r1.47 guc.h *** src/include/utils/guc.h 28 May 2004 05:13:32 -0000 1.47 --- src/include/utils/guc.h 2 Jun 2004 20:31:48 -0000 *************** *** 135,140 **** --- 135,147 ---- extern int client_min_messages; extern int log_min_duration_statement; + extern char *guc_pgdata; + extern char *guc_hbafile; + extern char *guc_identfile; + extern char *external_pidfile; + + extern char *user_pgconfig; + extern bool user_pgconfig_is_dir; extern void SetConfigOption(const char *name, const char *value, GucContext context, GucSource source); Index: src/include/utils/guc_tables.h =================================================================== RCS file: /cvsroot/pgsql-server/src/include/utils/guc_tables.h,v retrieving revision 1.11 diff -c -c -r1.11 guc_tables.h *** src/include/utils/guc_tables.h 26 May 2004 15:07:41 -0000 1.11 --- src/include/utils/guc_tables.h 2 Jun 2004 20:31:48 -0000 *************** *** 63,69 **** PGC_BOOL, PGC_INT, PGC_REAL, ! PGC_STRING }; /* --- 63,70 ---- PGC_BOOL, PGC_INT, PGC_REAL, ! PGC_STRING, ! PGC_FUNCTION }; /*
Bruce Momjian <pgman@candle.pha.pa.us> writes:
>> This patch incorporates a number of changes suggested by the group. The
>> purpose of this patch is to move postgresql to a position where all
>> configuration options are specified in one place. The postgresql.conf file
>> could completely document a postgresql environment.
AFAICS this patch breaks standalone backends, since the smarts involved
in dealing with the new flavors of directory layouts were not taught to
postgres.c.
The documentation is rather lacking as well. "include" is not really a
variable and should not be documented as if it were --- for one thing,
that leaves the reader wondering if he can only specify it once. The
other added variables are insufficiently doc'd because there is no
explanation of the defaults. Also I should think that somewhere in the
admin guide there should be an explanation of the different ways you can
lay out the files and why you might choose different ones. It's also
highly unclear how you get such a setup established, when there's been
no change in the behavior of initdb.
ProcessConfigFile will dump core on out-of-memory (test for malloc
failure is in the wrong place). I also think some memory leaks have
been introduced in ReadConfigFile.
The whole concept of a "function" GUC variable seems very ill-advised to
me; for one thing, what will "show include" or "set include" do? Can a
user do ALTER USER SET include = foo? I think it would have been better
to hard-wire the handling of 'include' in the guc file reader, and not
try to make it act like a variable.
regards, tom lane
> Bruce Momjian <pgman@candle.pha.pa.us> writes: >>> This patch incorporates a number of changes suggested by the group. The >>> purpose of this patch is to move postgresql to a position where all >>> configuration options are specified in one place. The postgresql.conf >>> file >>> could completely document a postgresql environment. > > AFAICS this patch breaks standalone backends, since the smarts involved > in dealing with the new flavors of directory layouts were not taught to > postgres.c. Hmm, I guess its time to get a CVS version of PG. This was originally done in 7.3 and migrated to 7.4. 7.5 is substantially different? > > The documentation is rather lacking as well. "include" is not really a > variable and should not be documented as if it were --- for one thing, > that leaves the reader wondering if he can only specify it once. The > other added variables are insufficiently doc'd because there is no > explanation of the defaults. Also I should think that somewhere in the > admin guide there should be an explanation of the different ways you can > lay out the files and why you might choose different ones. It's also > highly unclear how you get such a setup established, when there's been > no change in the behavior of initdb. I can write the docs. The primary purpose of this patch is to enable the functionality for those who need it. I was sure it would be impractical to get a concensus on changing PostgreSQL's default behavior, but this functionality can be used by OS vendors and consultants alike. As for include not being a variable, no it isn't. It is a new class of GUC parameter. Would you like a better syntax? FWIW: This is exactly the same syntax that was discussed, and no one brought up that it was a problem. You even said you liked the idea of "include." > > ProcessConfigFile will dump core on out-of-memory (test for malloc > failure is in the wrong place). I also think some memory leaks have > been introduced in ReadConfigFile. I will double check. The test for malloc failure may have drifted over time. There should be no memory leaks, but again, I'll double check. > > The whole concept of a "function" GUC variable seems very ill-advised to > me; for one thing, what will "show include" or "set include" do? Can a > user do ALTER USER SET include = foo? I think it would have been better > to hard-wire the handling of 'include' in the guc file reader, and not > try to make it act like a variable. I wanted to create a generic facility for "smarter" configuration. Being able to create functions and pass parameters should allow smaller more focused configuration parsing. I'm open to suggestions. Would you prefer stating the function parameters with a special character? '#' is taken, how about '!' ? is in: !include ...
pgsql@mohawksoft.com writes:
>> AFAICS this patch breaks standalone backends, since the smarts involved
>> in dealing with the new flavors of directory layouts were not taught to
>> postgres.c.
> Hmm, I guess its time to get a CVS version of PG. This was originally done
> in 7.3 and migrated to 7.4. 7.5 is substantially different?
The same issue applied in 7.4 and before; but yes, all that code looks
noticeably different in CVS tip.
> As for include not being a variable, no it isn't. It is a new class of GUC
> parameter. Would you like a better syntax?
As I said, I think this "class of GUC parameter" is ill-considered.
> FWIW: This is exactly the same syntax that was discussed, and no one
> brought up that it was a problem. You even said you liked the idea of
> "include."
I like the idea of include. I don't like this implementation.
> I wanted to create a generic facility for "smarter" configuration. Being
> able to create functions and pass parameters should allow smaller more
> focused configuration parsing.
I don't believe "include" is a representative of a class; it seems a
specialized one-of-a-kind thing. If you had one or two more examples
of this class then I might think you are onto something, but as-is
there is no reason to think that you've got a well-defined idea or
have made the right decisions about how it should work.
Basically, include is not a variable because neither "set include" nor
"show include" are sensible operations at all. Implementing it in a way
that makes you have to deal with these possibilities is simply
wrongheaded.
Another reason why it's not a variable is that processing it as a
variable means the sub-file is not read in the order that the user would
naturally expect; with the implementation as you have it, the behavior
would be quite surprising as to which file wins if both outer file and
subfile set the same variable. (Take another look at guc-file.l: it
eats the whole file and only then starts to evaluate variables.)
What I'd think is reasonable is a special case hack in guc-file.l that
checks for opt_name = "include" and does something immediately upon
seeing it. (CVS tip has a special hack for "custom_variable_classes"
here, which has got problems of its own because that *is* a variable,
but that's right around where I'd envision inserting code for include.)
> I'm open to suggestions. Would you prefer stating the function parameters
> with a special character? '#' is taken, how about '!' ? is in:
It's not the syntax I'm objecting to; it's the implementation.
regards, tom lane
> pgsql@mohawksoft.com writes: >>> AFAICS this patch breaks standalone backends, since the smarts involved >>> in dealing with the new flavors of directory layouts were not taught to >>> postgres.c. > >> Hmm, I guess its time to get a CVS version of PG. This was originally >> done >> in 7.3 and migrated to 7.4. 7.5 is substantially different? > > The same issue applied in 7.4 and before; but yes, all that code looks > noticeably different in CVS tip. > >> As for include not being a variable, no it isn't. It is a new class of >> GUC >> parameter. Would you like a better syntax? > > As I said, I think this "class of GUC parameter" is ill-considered. See below > >> FWIW: This is exactly the same syntax that was discussed, and no one >> brought up that it was a problem. You even said you liked the idea of >> "include." > > I like the idea of include. I don't like this implementation. OK. > >> I wanted to create a generic facility for "smarter" configuration. Being >> able to create functions and pass parameters should allow smaller more >> focused configuration parsing. > > I don't believe "include" is a representative of a class; it seems a > specialized one-of-a-kind thing. If you had one or two more examples > of this class then I might think you are onto something, but as-is > there is no reason to think that you've got a well-defined idea or > have made the right decisions about how it should work. OK, imagine this, maybe not right now, but in the future...... In the configuration file, one can load C code modules like in apache. Not just SQL functions, but modules of functionality, like foreign table types. > > Basically, include is not a variable because neither "set include" nor > "show include" are sensible operations at all. Implementing it in a way > that makes you have to deal with these possibilities is simply > wrongheaded. > > Another reason why it's not a variable is that processing it as a > variable means the sub-file is not read in the order that the user would > naturally expect; with the implementation as you have it, the behavior > would be quite surprising as to which file wins if both outer file and > subfile set the same variable. (Take another look at guc-file.l: it > eats the whole file and only then starts to evaluate variables.) > > What I'd think is reasonable is a special case hack in guc-file.l that > checks for opt_name = "include" and does something immediately upon > seeing it. (CVS tip has a special hack for "custom_variable_classes" > here, which has got problems of its own because that *is* a variable, > but that's right around where I'd envision inserting code for include.) > >> I'm open to suggestions. Would you prefer stating the function >> parameters >> with a special character? '#' is taken, how about '!' ? is in: > > It's not the syntax I'm objecting to; it's the implementation. > > regards, tom lane >
pgsql@mohawksoft.com wrote:
> >> I wanted to create a generic facility for "smarter" configuration. Being
> >> able to create functions and pass parameters should allow smaller more
> >> focused configuration parsing.
> >
> > I don't believe "include" is a representative of a class; it seems a
> > specialized one-of-a-kind thing. If you had one or two more examples
> > of this class then I might think you are onto something, but as-is
> > there is no reason to think that you've got a well-defined idea or
> > have made the right decisions about how it should work.
>
> OK, imagine this, maybe not right now, but in the future......
>
> In the configuration file, one can load C code modules like in apache. Not
> just SQL functions, but modules of functionality, like foreign table
> types.
I agree with Tom that "include" isn't really a setting, but more of an
action. What would "SHOW include" output? I think that outlines why it
is different from other setttings. Your "load" capability would be
another non-setting, or perhaps a read-only one, but not the same as
include either. Include includes other settings.
One format idea would be to suppress the equals for include, so:
include '/somedir/pgdefs.conf' # include another file
hba_conf = '/etc/postgres/pg_hba.conf' # use file in another directory
ident_conf = '/etc/postgres/pg_ident.conf' # use file in another directory
pgdata = '/usr/local/pgsql/data' # use /data in another directory
external_pidfile= '/var/run/postgresql.pid' # write an extra pid file
Notice include has no equals. This would more clearly suggest that
multiple includes could be used. I think a SHOW of include should
report an error.
One interesting idea would be for "SET include" to work like this:
SET include '/var/run/xx'
Notice there is no equals here. This would allow users to create files
with various settings and enable them all with one SET command.
However, this does open a security issue. Seems we might have to
disallow this and only allow include in postgresql.conf, where the
super-user has full control.
I agree with Tom that the documentation is sparse. Hopefully folks can
add to that. If someone is going to keep improving this patch, please
use the version I posted rather than the original because mine has lots
of cleanups.
ftp://candle.pha.pa.us/pub/postgresql/mypatches/guc
In summary, I think we need to treat include specially in
postgresql.conf (no equals) and remove it as an actual GUC parameter and
just have it do includes immediately. (This will probably require
special-casing it in the guc-file grammar.) Also, we need more docs on
how to set up the new -D/PGDATA functionality. I was wondering myself
how someone would configure this. Do we need to add an initdb
capability?
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> One interesting idea would be for "SET include" to work like this:
> SET include '/var/run/xx'
> Notice there is no equals here. This would allow users to create files
> with various settings and enable them all with one SET command.
> However, this does open a security issue.
More than one, in fact. In the first place, as the code presently
works, anything coming in from the file would be treated on an equal
footing with values sourced from postgresql.conf, thereby allowing
unprivileged users to set things they shouldn't. This is potentially
fixable, but the other issue isn't: such a facility would allow anyone
to ask the backend to read any file the Postgres user account can
access. Not very successfully, perhaps, but even the error messages
might give useful info about the file's contents to an attacker. This
is the same reason that "COPY FROM file" is a privileged operation.
I think it's important that include be restricted to appear only in
config files, and not be in any way shape or form a SETtable thing.
> In summary, I think we need to treat include specially in
> postgresql.conf (no equals) and remove it as an actual GUC parameter and
> just have it do includes immediately. (This will probably require
> special-casing it in the guc-file grammar.)
Yes. In fact, it'll be a less-than-trivial change in guc-file, at least
if you want the thing to act intuitively (that is, "include" acts like
the target file is actually included right here). This will mean
splitting ProcessConfigFile into a recursive read step followed by a
nonrecursive apply step. Also, I think that invoking the flex lexer
recursively will take a little bit of work.
I would suggest splitting the patch into two separate patches, one that
handles "include" and one that handles the other changes. The other
stuff is reasonably close to being ready to apply (modulo docs and
fixing the standalone-backend case), but "include" I think is still a
ways off.
regards, tom lane
Tom Lane wrote: >Bruce Momjian <pgman@candle.pha.pa.us> writes: > > >>In summary, I think we need to treat include specially in >>postgresql.conf (no equals) and remove it as an actual GUC parameter and >>just have it do includes immediately. (This will probably require >>special-casing it in the guc-file grammar.) >> >> > >Yes. In fact, it'll be a less-than-trivial change in guc-file, at least >if you want the thing to act intuitively (that is, "include" acts like >the target file is actually included right here). This will mean >splitting ProcessConfigFile into a recursive read step followed by a >nonrecursive apply step. Also, I think that invoking the flex lexer >recursively will take a little bit of work. > >I would suggest splitting the patch into two separate patches, one that >handles "include" and one that handles the other changes. The other >stuff is reasonably close to being ready to apply (modulo docs and >fixing the standalone-backend case), but "include" I think is still a >ways off. > > One classic way to do include files is inside the lexer itself - at least that's the way I have always done it in the past. Then the client code doesn't need to know anything about it at all - it just sees a stream of lexemes with no idea what file they come from. Since inclusion can be done recursively, the lexer keeps a stack of files, of some reasonable size - in our case surely 5 or 10 would be a more than reasonable maximum recursion depth. You do need to keep track of the filename and line number for error reporting, though (use parallel stacks for those). cheers andrew
I have applied the non-"include" parts of this patch, which allows the
configuration files to exist outside the data directory. I have added a
TODO:
* Add include functionality to postgresql.conf
Patch attached and applied. It still needs more documentation. Right
now only the postmaster manual page describes this functionality. Maybe
it needs initdb support too so it can be done automatically. Is this a
TODO?
Also, the old code allowed the postmaster to start if it couldn't find
postgresql.conf. I changed it in this patch because the new decoupling
of the file locations makes a missing postgresql.conf much more likely,
and of course it will never work without a postgresql.conf when they are
decoupled anyway. Should we fail if we can't find pg_ident.conf? Right
now we don't.
---------------------------------------------------------------------------
pgsql@mohawksoft.com wrote:
> > pgsql@mohawksoft.com writes:
> >>> AFAICS this patch breaks standalone backends, since the smarts involved
> >>> in dealing with the new flavors of directory layouts were not taught to
> >>> postgres.c.
> >
> >> Hmm, I guess its time to get a CVS version of PG. This was originally
> >> done
> >> in 7.3 and migrated to 7.4. 7.5 is substantially different?
> >
> > The same issue applied in 7.4 and before; but yes, all that code looks
> > noticeably different in CVS tip.
> >
> >> As for include not being a variable, no it isn't. It is a new class of
> >> GUC
> >> parameter. Would you like a better syntax?
> >
> > As I said, I think this "class of GUC parameter" is ill-considered.
>
> See below
> >
> >> FWIW: This is exactly the same syntax that was discussed, and no one
> >> brought up that it was a problem. You even said you liked the idea of
> >> "include."
> >
> > I like the idea of include. I don't like this implementation.
>
> OK.
>
> >
> >> I wanted to create a generic facility for "smarter" configuration. Being
> >> able to create functions and pass parameters should allow smaller more
> >> focused configuration parsing.
> >
> > I don't believe "include" is a representative of a class; it seems a
> > specialized one-of-a-kind thing. If you had one or two more examples
> > of this class then I might think you are onto something, but as-is
> > there is no reason to think that you've got a well-defined idea or
> > have made the right decisions about how it should work.
>
> OK, imagine this, maybe not right now, but in the future......
>
> In the configuration file, one can load C code modules like in apache. Not
> just SQL functions, but modules of functionality, like foreign table
> types.
> >
> > Basically, include is not a variable because neither "set include" nor
> > "show include" are sensible operations at all. Implementing it in a way
> > that makes you have to deal with these possibilities is simply
> > wrongheaded.
> >
> > Another reason why it's not a variable is that processing it as a
> > variable means the sub-file is not read in the order that the user would
> > naturally expect; with the implementation as you have it, the behavior
> > would be quite surprising as to which file wins if both outer file and
> > subfile set the same variable. (Take another look at guc-file.l: it
> > eats the whole file and only then starts to evaluate variables.)
> >
> > What I'd think is reasonable is a special case hack in guc-file.l that
> > checks for opt_name = "include" and does something immediately upon
> > seeing it. (CVS tip has a special hack for "custom_variable_classes"
> > here, which has got problems of its own because that *is* a variable,
> > but that's right around where I'd envision inserting code for include.)
> >
> >> I'm open to suggestions. Would you prefer stating the function
> >> parameters
> >> with a special character? '#' is taken, how about '!' ? is in:
> >
> > It's not the syntax I'm objecting to; it's the implementation.
> >
> > regards, tom lane
> >
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org
>
--
Bruce Momjian | http://candle.pha.pa.us
pgman@candle.pha.pa.us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073
Index: doc/src/sgml/runtime.sgml
===================================================================
RCS file: /cvsroot/pgsql-server/doc/src/sgml/runtime.sgml,v
retrieving revision 1.268
diff -c -c -r1.268 runtime.sgml
*** doc/src/sgml/runtime.sgml 27 Jun 2004 22:58:19 -0000 1.268
--- doc/src/sgml/runtime.sgml 11 Jul 2004 00:06:48 -0000
***************
*** 563,568 ****
--- 563,616 ----
any desired selection condition.
</para>
+ <sect2 id="runtime-config-configuration-files">
+ <title>Configuration Files</title>
+
+ <variablelist>
+
+ <varlistentry id="guc-pgdata" xreflabel="pgdata">
+ <term><varname>pgdata</varname> (<type>string</type>)</term>
+ <listitem>
+ <para>
+ Specifies the directory to use for data storage (everything except
+ configuration files).
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="guc-hba-conf" xreflabel="hba-conf">
+ <term><varname>hba_conf</varname> (<type>string</type>)</term>
+ <listitem>
+ <para>
+ Specifies the file name to use for configuration of host-based
+ authentication (HBA).
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="guc-ident-conf" xreflabel="ident-conf">
+ <term><varname>ident_conf</varname> (<type>string</type>)</term>
+ <listitem>
+ <para>
+ Specifies the file name to use for configuration of
+ <application>ident</> authentication.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ <varlistentry id="external-pidfile" xreflabel="external-pidfile">
+ <term><varname>external_pidfile</varname> (<type>string</type>)</term>
+ <listitem>
+ <para>
+ Specifies the location of an additional <application>postmaster</>
+ process-id (PID) file for use by server administration programs.
+ </para>
+ </listitem>
+ </varlistentry>
+
+ </variablelist>
+ </sect2>
+
<sect2 id="runtime-config-connection">
<title>Connections and Authentication</title>
Index: doc/src/sgml/ref/postmaster.sgml
===================================================================
RCS file: /cvsroot/pgsql-server/doc/src/sgml/ref/postmaster.sgml,v
retrieving revision 1.50
diff -c -c -r1.50 postmaster.sgml
*** doc/src/sgml/ref/postmaster.sgml 21 Jun 2004 04:06:04 -0000 1.50
--- doc/src/sgml/ref/postmaster.sgml 11 Jul 2004 00:06:49 -0000
***************
*** 67,80 ****
One <command>postmaster</command> always manages the data
from exactly one database cluster. A database cluster is a
collection of databases that is stored at a common file system
! location. When the <command>postmaster</command> starts it needs to know the location
! of the database cluster files (<quote>data area</quote>). This is
! done with the <option>-D</option> invocation option or the
! <envar>PGDATA</envar> environment variable; there is no default.
! More than one <command>postmaster</command> process can run on a system at one time,
! as long as they use different data areas and different
communication ports (see below). A data area is created with <xref
linkend="app-initdb">.
</para>
</refsect1>
--- 67,93 ----
One <command>postmaster</command> always manages the data
from exactly one database cluster. A database cluster is a
collection of databases that is stored at a common file system
! location. When the <command>postmaster</command> starts it needs
! to know the location of the database cluster files (<quote>data
! area</quote>).
! More than one <command>postmaster</command> process can run on a system
! at one time as long as they use different data areas and different
communication ports (see below). A data area is created with <xref
linkend="app-initdb">.
+ </para>
+
+ <para>
+ The <quote>data area</> is specified by the <option>-D</option> option
+ or the <envar>PGDATA</envar> environment variable; there is no default.
+ Typically, it points to a directory created by <application>
+ initdb</>. However, for administrative flexibility, you can
+ point to a directory containing only configuration files:
+ <filename>postgresql.conf</>, <filename>pg_hba.conf</>, and
+ <filename>pg_ident.conf</>. You can then set
+ <filename>postgresql.conf</>'s <varname>pgdata</> to point to the
+ data directory. You can also point just to the server configuration file
+ like <filename>postgresql.conf</> and set its variables to point to the
+ other configuration files and the data directory.
</para>
</refsect1>
Index: src/backend/bootstrap/bootstrap.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/bootstrap/bootstrap.c,v
retrieving revision 1.185
diff -c -c -r1.185 bootstrap.c
*** src/backend/bootstrap/bootstrap.c 24 Jun 2004 21:02:24 -0000 1.185
--- src/backend/bootstrap/bootstrap.c 11 Jul 2004 00:06:50 -0000
***************
*** 212,218 ****
char *dbname;
int flag;
int xlogop = BS_XLOG_NOP;
! char *potential_DataDir = NULL;
/*
* initialize globals
--- 212,218 ----
char *dbname;
int flag;
int xlogop = BS_XLOG_NOP;
! char *userPGDATA = NULL;
/*
* initialize globals
***************
*** 236,243 ****
if (!IsUnderPostmaster)
{
InitializeGUCOptions();
! potential_DataDir = getenv("PGDATA"); /* Null if no PGDATA
! * variable */
}
/* Ignore the initial -boot argument, if present */
--- 236,242 ----
if (!IsUnderPostmaster)
{
InitializeGUCOptions();
! userPGDATA = getenv("PGDATA"); /* Null if no PGDATA variable */
}
/* Ignore the initial -boot argument, if present */
***************
*** 252,258 ****
switch (flag)
{
case 'D':
! potential_DataDir = optarg;
break;
case 'd':
{
--- 251,257 ----
switch (flag)
{
case 'D':
! userPGDATA = optarg;
break;
case 'd':
{
***************
*** 326,332 ****
if (!IsUnderPostmaster)
{
! if (!potential_DataDir)
{
write_stderr("%s does not know where to find the database system data.\n"
"You must specify the directory that contains the database system\n"
--- 325,331 ----
if (!IsUnderPostmaster)
{
! if (!userPGDATA)
{
write_stderr("%s does not know where to find the database system data.\n"
"You must specify the directory that contains the database system\n"
***************
*** 335,341 ****
argv[0]);
proc_exit(1);
}
! SetDataDir(potential_DataDir);
}
/* Validate we have been given a reasonable-looking DataDir */
--- 334,340 ----
argv[0]);
proc_exit(1);
}
! SetDataDir(userPGDATA);
}
/* Validate we have been given a reasonable-looking DataDir */
Index: src/backend/libpq/hba.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/libpq/hba.c,v
retrieving revision 1.125
diff -c -c -r1.125 hba.c
*** src/backend/libpq/hba.c 30 May 2004 23:40:26 -0000 1.125
--- src/backend/libpq/hba.c 11 Jul 2004 00:06:52 -0000
***************
*** 35,40 ****
--- 35,41 ----
#include "miscadmin.h"
#include "nodes/pg_list.h"
#include "storage/fd.h"
+ #include "utils/guc.h"
/* Max size of username ident server can return */
***************
*** 1029,1045 ****
void
load_hba(void)
{
- int bufsize;
FILE *file; /* The config file we have to read */
char *conf_file; /* The name of the config file */
if (hba_lines || hba_line_nums)
free_lines(&hba_lines, &hba_line_nums);
! /* Put together the full pathname to the config file. */
! bufsize = (strlen(DataDir) + strlen(CONF_FILE) + 2) * sizeof(char);
! conf_file = (char *) palloc(bufsize);
! snprintf(conf_file, bufsize, "%s/%s", DataDir, CONF_FILE);
file = AllocateFile(conf_file, "r");
if (file == NULL)
--- 1030,1051 ----
void
load_hba(void)
{
FILE *file; /* The config file we have to read */
char *conf_file; /* The name of the config file */
if (hba_lines || hba_line_nums)
free_lines(&hba_lines, &hba_line_nums);
! /* HBA filename in config file */
! if (guc_hbafile)
! conf_file = pstrdup(guc_hbafile);
! else
! {
! char *confloc = (user_pgconfig_is_dir) ? user_pgconfig : DataDir;
! /* put together the full pathname to the config file */
! conf_file = palloc(strlen(confloc) + strlen(CONF_FILE) + 2);
! sprintf(conf_file, "%s/%s", confloc, CONF_FILE);
! }
file = AllocateFile(conf_file, "r");
if (file == NULL)
***************
*** 1178,1193 ****
FILE *file; /* The map file we have to read */
char *map_file; /* The name of the map file we have to
* read */
- int bufsize;
-
if (ident_lines || ident_line_nums)
free_lines(&ident_lines, &ident_line_nums);
! /* put together the full pathname to the map file */
! bufsize = (strlen(DataDir) + strlen(USERMAP_FILE) + 2) * sizeof(char);
! map_file = (char *) palloc(bufsize);
! snprintf(map_file, bufsize, "%s/%s", DataDir, USERMAP_FILE);
!
file = AllocateFile(map_file, "r");
if (file == NULL)
{
--- 1184,1203 ----
FILE *file; /* The map file we have to read */
char *map_file; /* The name of the map file we have to
* read */
if (ident_lines || ident_line_nums)
free_lines(&ident_lines, &ident_line_nums);
! /* IDENT filename in config file */
! if (guc_identfile)
! map_file = pstrdup(guc_identfile);
! else
! {
! /* put together the full pathname to the map file */
! char *confloc = (user_pgconfig_is_dir) ? user_pgconfig : DataDir;
! map_file = (char *) palloc(strlen(confloc) + strlen(USERMAP_FILE) + 2);
! sprintf(map_file, "%s/%s", confloc, USERMAP_FILE);
! }
!
file = AllocateFile(map_file, "r");
if (file == NULL)
{
Index: src/backend/postmaster/postmaster.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/postmaster/postmaster.c,v
retrieving revision 1.405
diff -c -c -r1.405 postmaster.c
*** src/backend/postmaster/postmaster.c 24 Jun 2004 21:02:55 -0000 1.405
--- src/backend/postmaster/postmaster.c 11 Jul 2004 00:06:57 -0000
***************
*** 233,238 ****
--- 233,239 ----
* postmaster.c - function prototypes
*/
static void checkDataDir(const char *checkdir);
+ static bool onlyConfigSpecified(const char *checkdir);
#ifdef USE_RENDEZVOUS
static void reg_reply(DNSServiceRegistrationReplyErrorType errorCode,
void *context);
***************
*** 306,312 ****
{
int opt;
int status;
! char *potential_DataDir = NULL;
int i;
progname = get_progname(argv[0]);
--- 307,313 ----
{
int opt;
int status;
! char *userPGDATA = NULL;
int i;
progname = get_progname(argv[0]);
***************
*** 370,376 ****
*/
InitializeGUCOptions();
! potential_DataDir = getenv("PGDATA"); /* default value */
opterr = 1;
--- 371,377 ----
*/
InitializeGUCOptions();
! userPGDATA = getenv("PGDATA"); /* default value */
opterr = 1;
***************
*** 395,401 ****
/* Can no longer set the backend executable file to use. */
break;
case 'D':
! potential_DataDir = optarg;
break;
case 'd':
{
--- 396,402 ----
/* Can no longer set the backend executable file to use. */
break;
case 'D':
! userPGDATA = optarg;
break;
case 'd':
{
***************
*** 523,535 ****
ExitPostmaster(1);
}
! /*
! * Now we can set the data directory, and then read postgresql.conf.
! */
! checkDataDir(potential_DataDir); /* issues error messages */
! SetDataDir(potential_DataDir);
! ProcessConfigFile(PGC_POSTMASTER);
/* If timezone is not set, determine what the OS uses */
pg_timezone_initialize();
--- 524,572 ----
ExitPostmaster(1);
}
! if (onlyConfigSpecified(userPGDATA))
! {
! /*
! * It is either a file name or a directory with no
! * global/pg_control file, and hence not a data directory.
! */
! user_pgconfig = userPGDATA;
! ProcessConfigFile(PGC_POSTMASTER);
!
! if (!guc_pgdata) /* Got a pgdata from the config file? */
! {
! write_stderr("%s does not know where to find the database system data.\n"
! "This should be specified as 'pgdata' in %s%s.\n",
! progname, userPGDATA,
! user_pgconfig_is_dir ? "/postgresql.conf" : "");
! ExitPostmaster(2);
! }
! checkDataDir(guc_pgdata);
! SetDataDir(guc_pgdata);
! }
! else
! {
! /* Now we can set the data directory, and then read postgresql.conf. */
! checkDataDir(userPGDATA);
! SetDataDir(userPGDATA);
! ProcessConfigFile(PGC_POSTMASTER);
! }
! if (external_pidfile)
! {
! FILE *fpidfile = fopen(external_pidfile, "w");
!
! if (fpidfile)
! {
! fprintf(fpidfile, "%d\n", MyProcPid);
! fclose(fpidfile);
! /* Should we remove the pid file on postmaster exit? */
! }
! else
! fprintf(stderr,
! gettext("%s could not write to external pid file %s\n"),
! progname, external_pidfile);
! }
/* If timezone is not set, determine what the OS uses */
pg_timezone_initialize();
***************
*** 847,852 ****
--- 884,915 ----
}
+
+ static bool
+ onlyConfigSpecified(const char *checkdir)
+ {
+ char path[MAXPGPATH];
+ struct stat stat_buf;
+
+ if (checkdir == NULL) /* checkDataDir handles this */
+ return FALSE;
+
+ if (stat(checkdir, &stat_buf) == -1) /* ditto */
+ return FALSE;
+
+ if (S_ISREG(stat_buf.st_mode)) /* It's a regular file, so assume it's explict */
+ return TRUE;
+ else if (S_ISDIR(stat_buf.st_mode)) /* It's a directory, is it a config or system dir? */
+ {
+ snprintf(path, MAXPGPATH, "%s/global/pg_control", checkdir);
+ /* If this is not found, it is a config-only directory */
+ if (stat(path, &stat_buf) == -1)
+ return TRUE;
+ }
+ return FALSE;
+ }
+
+
/*
* Validate the proposed data directory
*/
***************
*** 861,868 ****
{
write_stderr("%s does not know where to find the database system data.\n"
"You must specify the directory that contains the database system\n"
! "either by specifying the -D invocation option or by setting the\n"
! "PGDATA environment variable.\n",
progname);
ExitPostmaster(2);
}
--- 924,931 ----
{
write_stderr("%s does not know where to find the database system data.\n"
"You must specify the directory that contains the database system\n"
! "or configuration files by either specifying the -D invocation option\n"
! "or by setting the PGDATA environment variable.\n",
progname);
ExitPostmaster(2);
}
***************
*** 872,883 ****
if (errno == ENOENT)
ereport(FATAL,
(errcode_for_file_access(),
! errmsg("data directory \"%s\" does not exist",
checkdir)));
else
ereport(FATAL,
(errcode_for_file_access(),
! errmsg("could not read permissions of directory \"%s\": %m",
checkdir)));
}
--- 935,946 ----
if (errno == ENOENT)
ereport(FATAL,
(errcode_for_file_access(),
! errmsg("data or configuration location \"%s\" does not exist",
checkdir)));
else
ereport(FATAL,
(errcode_for_file_access(),
! errmsg("could not read permissions of \"%s\": %m",
checkdir)));
}
Index: src/backend/tcop/postgres.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/tcop/postgres.c,v
retrieving revision 1.422
diff -c -c -r1.422 postgres.c
*** src/backend/tcop/postgres.c 1 Jul 2004 00:51:11 -0000 1.422
--- src/backend/tcop/postgres.c 11 Jul 2004 00:07:04 -0000
***************
*** 2164,2170 ****
{
int flag;
const char *dbname = NULL;
! char *potential_DataDir = NULL;
bool secure;
int errs = 0;
int debug_flag = 0;
--- 2164,2170 ----
{
int flag;
const char *dbname = NULL;
! char *userPGDATA = NULL;
bool secure;
int errs = 0;
int debug_flag = 0;
***************
*** 2235,2241 ****
if (!IsUnderPostmaster)
{
InitializeGUCOptions();
! potential_DataDir = getenv("PGDATA");
}
/* ----------------
--- 2235,2241 ----
if (!IsUnderPostmaster)
{
InitializeGUCOptions();
! userPGDATA = getenv("PGDATA");
}
/* ----------------
***************
*** 2283,2289 ****
case 'D': /* PGDATA directory */
if (secure)
! potential_DataDir = optarg;
break;
case 'd': /* debug level */
--- 2283,2289 ----
case 'D': /* PGDATA directory */
if (secure)
! userPGDATA = optarg;
break;
case 'd': /* debug level */
***************
*** 2574,2585 ****
* set up handler to log session end.
*/
if (IsUnderPostmaster && Log_disconnections)
! on_proc_exit(log_disconnections,0);
}
if (!IsUnderPostmaster)
{
! if (!potential_DataDir)
{
write_stderr("%s does not know where to find the database system data.\n"
"You must specify the directory that contains the database system\n"
--- 2574,2585 ----
* set up handler to log session end.
*/
if (IsUnderPostmaster && Log_disconnections)
! on_proc_exit(log_disconnections, 0);
}
if (!IsUnderPostmaster)
{
! if (!userPGDATA)
{
write_stderr("%s does not know where to find the database system data.\n"
"You must specify the directory that contains the database system\n"
***************
*** 2588,2594 ****
argv[0]);
proc_exit(1);
}
! SetDataDir(potential_DataDir);
}
Assert(DataDir);
--- 2588,2594 ----
argv[0]);
proc_exit(1);
}
! SetDataDir(userPGDATA);
}
Assert(DataDir);
Index: src/backend/utils/misc/guc-file.l
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/utils/misc/guc-file.l,v
retrieving revision 1.22
diff -c -c -r1.22 guc-file.l
*** src/backend/utils/misc/guc-file.l 26 May 2004 15:07:38 -0000 1.22
--- src/backend/utils/misc/guc-file.l 11 Jul 2004 00:07:05 -0000
***************
*** 129,171 ****
* function does not return if an error occurs. If an error occurs, no
* values will be changed.
*/
! void
! ProcessConfigFile(GucContext context)
{
int token, parse_state;
char *opt_name, *opt_value;
- char *filename;
struct name_value_pair *item, *head, *tail;
int elevel;
FILE * fp;
- Assert(context == PGC_POSTMASTER || context == PGC_BACKEND
- || context == PGC_SIGHUP);
- Assert(DataDir);
elevel = (context == PGC_SIGHUP) ? DEBUG4 : ERROR;
- /*
- * Open file
- */
- filename = malloc(strlen(DataDir) + strlen(CONFIG_FILENAME) + 2);
- if (filename == NULL)
- {
- ereport(elevel,
- (errcode(ERRCODE_OUT_OF_MEMORY),
- errmsg("out of memory")));
- return;
- }
- sprintf(filename, "%s/" CONFIG_FILENAME, DataDir);
-
fp = AllocateFile(filename, "r");
if (!fp)
{
free(filename);
! /* File not found is fine */
! if (errno != ENOENT)
! ereport(elevel,
! (errcode_for_file_access(),
! errmsg("could not open configuration file \"%s\": %m", CONFIG_FILENAME)));
return;
}
--- 129,152 ----
* function does not return if an error occurs. If an error occurs, no
* values will be changed.
*/
! static void
! ReadConfigFile(char *filename, GucContext context)
{
int token, parse_state;
char *opt_name, *opt_value;
struct name_value_pair *item, *head, *tail;
int elevel;
FILE * fp;
elevel = (context == PGC_SIGHUP) ? DEBUG4 : ERROR;
fp = AllocateFile(filename, "r");
if (!fp)
{
free(filename);
! ereport(elevel,
! (errcode_for_file_access(),
! errmsg("could not open configuration file \"%s\": %m", filename)));
return;
}
***************
*** 197,203 ****
token = yylex();
if (token != GUC_ID && token != GUC_STRING &&
! token != GUC_INTEGER && token != GUC_REAL &&
token != GUC_UNQUOTED_STRING)
goto parse_error;
opt_value = strdup(yytext);
--- 178,185 ----
token = yylex();
if (token != GUC_ID && token != GUC_STRING &&
! token != GUC_INTEGER &&
! token != GUC_REAL &&
token != GUC_UNQUOTED_STRING)
goto parse_error;
opt_value = strdup(yytext);
***************
*** 259,265 ****
}
FreeFile(fp);
- free(filename);
/*
* Check if all options are valid
--- 241,246 ----
***************
*** 282,293 ****
parse_error:
FreeFile(fp);
- free(filename);
free_name_value_list(head);
ereport(elevel,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("syntax error in file \"%s\" line %u, near token \"%s\"",
! CONFIG_FILENAME, ConfigFileLineno, yytext)));
return;
out_of_memory:
--- 263,273 ----
parse_error:
FreeFile(fp);
free_name_value_list(head);
ereport(elevel,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("syntax error in file \"%s\" line %u, near token \"%s\"",
! filename, ConfigFileLineno, yytext)));
return;
out_of_memory:
***************
*** 298,303 ****
--- 278,342 ----
(errcode(ERRCODE_OUT_OF_MEMORY),
errmsg("out of memory")));
return;
+ }
+
+ /*
+ * Function to read and process the configuration file. The
+ * parameter indicates the context that the file is being read
+ * (postmaster startup, backend startup, or SIGHUP). All options
+ * mentioned in the configuration file are set to new values. This
+ * function does not return if an error occurs. If an error occurs, no
+ * values will be changed.
+ */
+ void
+ ProcessConfigFile(GucContext context)
+ {
+ char *filename;
+
+ Assert(context == PGC_POSTMASTER || context == PGC_BACKEND || context == PGC_SIGHUP);
+
+ /* Added for explicit config file */
+ if (user_pgconfig)
+ {
+ struct stat sb;
+
+ if (stat(user_pgconfig, &sb) != 0)
+ {
+ int elevel = (context == PGC_SIGHUP) ? DEBUG3 : ERROR;
+ elog(elevel, "Configuration file \"%s\" does not exist", user_pgconfig);
+ return;
+ }
+
+ if (S_ISDIR(sb.st_mode))
+ {
+ /* This will cause a small one time memory leak
+ * if the user also specifies hba_conf,
+ * ident_conf, and data_dir
+ */
+ filename = malloc(strlen(user_pgconfig) + strlen(CONFIG_FILENAME) + 2);
+ sprintf(filename, "%s/%s", user_pgconfig, CONFIG_FILENAME);
+ user_pgconfig_is_dir = true;
+ }
+ else
+ filename = strdup(user_pgconfig); /* Use explicit file */
+ }
+ else
+ {
+ /* Use datadir for config */
+ filename = malloc(strlen(DataDir) + strlen(CONFIG_FILENAME) + 2);
+ sprintf(filename, "%s/%s", DataDir, CONFIG_FILENAME);
+ }
+
+ if (filename == NULL)
+ {
+ int elevel = (context == PGC_SIGHUP) ? DEBUG3 : ERROR;
+ elog(elevel, "out of memory");
+ return;
+ }
+
+ ReadConfigFile(filename, context);
+
+ free(filename);
}
Index: src/backend/utils/misc/guc.c
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/utils/misc/guc.c,v
retrieving revision 1.213
diff -c -c -r1.213 guc.c
*** src/backend/utils/misc/guc.c 5 Jul 2004 23:14:14 -0000 1.213
--- src/backend/utils/misc/guc.c 11 Jul 2004 00:07:08 -0000
***************
*** 57,62 ****
--- 57,69 ----
#include "utils/pg_locale.h"
#include "pgstat.h"
+ char *guc_pgdata;
+ char *guc_hbafile;
+ char *guc_identfile;
+ char *external_pidfile;
+
+ char *user_pgconfig = NULL;
+ bool user_pgconfig_is_dir = false;
#ifndef PG_KRB_SRVTAB
#define PG_KRB_SRVTAB ""
***************
*** 107,112 ****
--- 114,120 ----
static bool assign_log_stats(bool newval, bool doit, GucSource source);
static bool assign_transaction_read_only(bool newval, bool doit, GucSource source);
+ static void ReadConfigFile(char *filename, GucContext context);
/*
* Debugging options
***************
*** 1701,1715 ****
NULL, assign_custom_variable_classes, NULL
},
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, NULL, NULL, NULL
}
};
- /******** end of options list ********/
/*
* To allow continued support of obsolete names for GUC variables, we apply
* the following mappings to any unrecognized name. Note that an old name
--- 1709,1748 ----
NULL, assign_custom_variable_classes, NULL
},
+ {
+ {"pgdata", PGC_POSTMASTER, 0, gettext_noop("Sets the location of the data directory"), NULL},
+ &guc_pgdata,
+ NULL, NULL, NULL
+ },
+
+ {
+ {"hba_conf", PGC_SIGHUP, 0, gettext_noop("Sets the location of the \"hba\" configuration file"), NULL},
+ &guc_hbafile,
+ NULL, NULL, NULL
+ },
+
+ {
+ {"ident_conf", PGC_SIGHUP, 0, gettext_noop("Sets the location of the \"ident\" configuration file"), NULL},
+ &guc_identfile,
+ NULL, NULL, NULL
+ },
+
+ {
+ {"external_pidfile", PGC_POSTMASTER, 0, gettext_noop("Writes the postmaster PID to the specified file"),
NULL},
+ &external_pidfile,
+ NULL, NULL, NULL
+ },
+
/* End-of-list marker */
{
{NULL, 0, 0, NULL, NULL}, NULL, NULL, NULL, NULL
}
};
+ /******** end of options list ********/
+
/*
* To allow continued support of obsolete names for GUC variables, we apply
* the following mappings to any unrecognized name. Note that an old name
Index: src/backend/utils/misc/postgresql.conf.sample
===================================================================
RCS file: /cvsroot/pgsql-server/src/backend/utils/misc/postgresql.conf.sample,v
retrieving revision 1.113
diff -c -c -r1.113 postgresql.conf.sample
*** src/backend/utils/misc/postgresql.conf.sample 7 Apr 2004 05:05:50 -0000 1.113
--- src/backend/utils/misc/postgresql.conf.sample 11 Jul 2004 00:07:09 -0000
***************
*** 22,27 ****
--- 22,37 ----
#---------------------------------------------------------------------------
+ # CONFIGURATION FILES
+ #---------------------------------------------------------------------------
+
+ # pgdata = '/usr/local/pgsql/data' # use data in another directory
+ # hba_conf = '/etc/pgsql/pg_hba.conf' # use hba info in another directory
+ # ident_conf = '/etc/pgsql/pg_ident.conf' # use ident info in another directory
+ # external_pidfile= '/var/run/postgresql.pid' # write an extra pid file
+
+
+ #---------------------------------------------------------------------------
# CONNECTIONS AND AUTHENTICATION
#---------------------------------------------------------------------------
Index: src/include/utils/guc.h
===================================================================
RCS file: /cvsroot/pgsql-server/src/include/utils/guc.h,v
retrieving revision 1.48
diff -c -c -r1.48 guc.h
*** src/include/utils/guc.h 1 Jul 2004 00:51:44 -0000 1.48
--- src/include/utils/guc.h 11 Jul 2004 00:07:10 -0000
***************
*** 135,140 ****
--- 135,147 ----
extern int client_min_messages;
extern int log_min_duration_statement;
+ extern char *guc_pgdata;
+ extern char *guc_hbafile;
+ extern char *guc_identfile;
+ extern char *external_pidfile;
+
+ extern char *user_pgconfig;
+ extern bool user_pgconfig_is_dir;
extern void SetConfigOption(const char *name, const char *value,
GucContext context, GucSource source);
Oh, also, I was not able to put a name on the patch because I only have a 'pgsql' email address for the submitter. Hope that is OK. --------------------------------------------------------------------------- pgsql@mohawksoft.com wrote: > > Bruce Momjian <pgman@candle.pha.pa.us> writes: > >>> This patch incorporates a number of changes suggested by the group. The > >>> purpose of this patch is to move postgresql to a position where all > >>> configuration options are specified in one place. The postgresql.conf > >>> file > >>> could completely document a postgresql environment. > > > > AFAICS this patch breaks standalone backends, since the smarts involved > > in dealing with the new flavors of directory layouts were not taught to > > postgres.c. > > Hmm, I guess its time to get a CVS version of PG. This was originally done > in 7.3 and migrated to 7.4. 7.5 is substantially different? > > > > > The documentation is rather lacking as well. "include" is not really a > > variable and should not be documented as if it were --- for one thing, > > that leaves the reader wondering if he can only specify it once. The > > other added variables are insufficiently doc'd because there is no > > explanation of the defaults. Also I should think that somewhere in the > > admin guide there should be an explanation of the different ways you can > > lay out the files and why you might choose different ones. It's also > > highly unclear how you get such a setup established, when there's been > > no change in the behavior of initdb. > > I can write the docs. The primary purpose of this patch is to enable the > functionality for those who need it. I was sure it would be impractical to > get a concensus on changing PostgreSQL's default behavior, but this > functionality can be used by OS vendors and consultants alike. > > As for include not being a variable, no it isn't. It is a new class of GUC > parameter. Would you like a better syntax? > > FWIW: This is exactly the same syntax that was discussed, and no one > brought up that it was a problem. You even said you liked the idea of > "include." > > > > > ProcessConfigFile will dump core on out-of-memory (test for malloc > > failure is in the wrong place). I also think some memory leaks have > > been introduced in ReadConfigFile. > > I will double check. The test for malloc failure may have drifted over > time. There should be no memory leaks, but again, I'll double check. > > > > The whole concept of a "function" GUC variable seems very ill-advised to > > me; for one thing, what will "show include" or "set include" do? Can a > > user do ALTER USER SET include = foo? I think it would have been better > > to hard-wire the handling of 'include' in the guc file reader, and not > > try to make it act like a variable. > > I wanted to create a generic facility for "smarter" configuration. Being > able to create functions and pass parameters should allow smaller more > focused configuration parsing. > > I'm open to suggestions. Would you prefer stating the function parameters > with a special character? '#' is taken, how about '!' ? is in: > > !include ... > > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 359-1001 + If your life is a hard drive, | 13 Roberts Road + Christ can be your backup. | Newtown Square, Pennsylvania 19073