Re: [HACKERS] 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.

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(at)mohawksoft(dot)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(at)candle(dot)pha(dot)pa(dot)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(at)mohawksoft(dot)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(at)candle(dot)pha(dot)pa(dot)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(at)mohawksoft(dot)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(at)candle(dot)pha(dot)pa(dot)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(at)mohawksoft(dot)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(at)candle(dot)pha(dot)pa(dot)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(at)mohawksoft(dot)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(at)candle(dot)pha(dot)pa(dot)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(at)candle(dot)pha(dot)pa(dot)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(at)candle(dot)pha(dot)pa(dot)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(at)mohawksoft(dot)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(at)mohawksoft(dot)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(at)mohawksoft(dot)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(at)candle(dot)pha(dot)pa(dot)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(at)candle(dot)pha(dot)pa(dot)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(at)candle(dot)pha(dot)pa(dot)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

Where are we on this?

---------------------------------------------------------------------------

Tom Lane wrote:
> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)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
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

>
> Where are we on this?

That's a good question.

Tom doesn't like the syntax of "include" and there are a couple bugs he is
concered it.

I'm pretty agnostic about the syntax, but I wouldn't get overly worried
about the metaphor presented either.

"include='...'" doesn't bother me at all, but some people have a problem
with it.

Then there is the design of using a callable function for a configuration
parameter, personally, I think this feature is useful for the future, Tom
seems to have a problem it it.

After that, the discussion sort of ends.

I'm willing to adress the bugs.
I don't think the syntax is a huge deal, IMHO at most it is a
documentation problem.

>
> ---------------------------------------------------------------------------
>
> Tom Lane wrote:
>> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)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
>>
>
> --
> Bruce Momjian | http://candle.pha.pa.us
> pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
> + If your life is a hard drive, | 13 Roberts Road
> + Christ can be your backup. | Newtown Square, Pennsylvania
> 19073
>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: the planner will ignore your desire to choose an index scan if your
> joining column's datatypes do not match
>

pgsql(at)mohawksoft(dot)com wrote:
> >
> > Where are we on this?
>
> That's a good question.
>
> Tom doesn't like the syntax of "include" and there are a couple bugs he is
> concered it.
>
> I'm pretty agnostic about the syntax, but I wouldn't get overly worried
> about the metaphor presented either.
>
> "include='...'" doesn't bother me at all, but some people have a problem
> with it.
>
> Then there is the design of using a callable function for a configuration
> parameter, personally, I think this feature is useful for the future, Tom
> seems to have a problem it it.
>
> After that, the discussion sort of ends.
>
>
> I'm willing to adress the bugs.
> I don't think the syntax is a huge deal, IMHO at most it is a
> documentation problem.

Well, it seems pretty clear were we need to go on this. First, we could
just add the documentation to the non-include part of the patch based on
the version I posted and apply that to be sure it gets into 7.5.

Then, for "include", it needs to be an operation and not a variable,
probably something in guc-file.l.

It should not use an equals and not be something you can say SHOW with.

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

>> Tom doesn't like the syntax of "include"

I said more than once that I didn't care about the syntax; it's the
implementation I was objecting to.

However, given that we are going to push it into guc-file.l, it'll
be easier all around if we choose a syntax that doesn't look exactly
like a variable assignment.
include 'file'
with no equal sign would probably work as well as anything else.

regards, tom lane

Mark (see I remembered your name), where are we on this patch? It needs
docs and include has to be redone. Should I remove the include part of
the patch, add docs, and apply it?

---------------------------------------------------------------------------

pgsql(at)mohawksoft(dot)com wrote:
> >
> > Where are we on this?
>
> That's a good question.
>
> Tom doesn't like the syntax of "include" and there are a couple bugs he is
> concered it.
>
> I'm pretty agnostic about the syntax, but I wouldn't get overly worried
> about the metaphor presented either.
>
> "include='...'" doesn't bother me at all, but some people have a problem
> with it.
>
> Then there is the design of using a callable function for a configuration
> parameter, personally, I think this feature is useful for the future, Tom
> seems to have a problem it it.
>
> After that, the discussion sort of ends.
>
>
> I'm willing to adress the bugs.
> I don't think the syntax is a huge deal, IMHO at most it is a
> documentation problem.
>
> >
> > ---------------------------------------------------------------------------
> >
> > Tom Lane wrote:
> >> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)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
> >>
> >
> > --
> > Bruce Momjian | http://candle.pha.pa.us
> > pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
> > + If your life is a hard drive, | 13 Roberts Road
> > + Christ can be your backup. | Newtown Square, Pennsylvania
> > 19073
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 9: the planner will ignore your desire to choose an index scan if your
> > joining column's datatypes do not match
> >
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: the planner will ignore your desire to choose an index scan if your
> joining column's datatypes do not match
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

I have gotten no reply to my request to either move the include
functionality into the guc-file.l or remove it and just add docs for the
config location part of the patch. I now would like someone else to
take the patch and make those changes to get it applied before feature
freeze. If not, I can do it but it will be after the feature freeze:

ftp://candle.pha.pa.us/pub/postgresql/mypatches/guc

---------------------------------------------------------------------------

pgsql(at)mohawksoft(dot)com wrote:
> >
> > Where are we on this?
>
> That's a good question.
>
> Tom doesn't like the syntax of "include" and there are a couple bugs he is
> concered it.
>
> I'm pretty agnostic about the syntax, but I wouldn't get overly worried
> about the metaphor presented either.
>
> "include='...'" doesn't bother me at all, but some people have a problem
> with it.
>
> Then there is the design of using a callable function for a configuration
> parameter, personally, I think this feature is useful for the future, Tom
> seems to have a problem it it.
>
> After that, the discussion sort of ends.
>
>
> I'm willing to adress the bugs.
> I don't think the syntax is a huge deal, IMHO at most it is a
> documentation problem.
>
> >
> > ---------------------------------------------------------------------------
> >
> > Tom Lane wrote:
> >> Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)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
> >>
> >
> > --
> > Bruce Momjian | http://candle.pha.pa.us
> > pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
> > + If your life is a hard drive, | 13 Roberts Road
> > + Christ can be your backup. | Newtown Square, Pennsylvania
> > 19073
> >
> > ---------------------------(end of broadcast)---------------------------
> > TIP 9: the planner will ignore your desire to choose an index scan if your
> > joining column's datatypes do not match
> >
>
>
> ---------------------------(end of broadcast)---------------------------
> TIP 9: the planner will ignore your desire to choose an index scan if your
> joining column's datatypes do not match
>

--
Bruce Momjian | http://candle.pha.pa.us
pgman(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

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(at)mohawksoft(dot)com wrote:
> > pgsql(at)mohawksoft(dot)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(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073

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(at)mohawksoft(dot)com wrote:
> > Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)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(at)candle(dot)pha(dot)pa(dot)us | (610) 359-1001
+ If your life is a hard drive, | 13 Roberts Road
+ Christ can be your backup. | Newtown Square, Pennsylvania 19073