Re: [HACKERS] Configuration patch

From: pgsql(at)mohawksoft(dot)com
To: "Tom Lane" <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: "Bruce Momjian" <pgman(at)candle(dot)pha(dot)pa(dot)us>, "PostgreSQL-patches" <pgsql-patches(at)postgresql(dot)org>
Subject: Re: [HACKERS] Configuration patch
Date: 2004-06-03 17:46:09
Message-ID: 7418.64.119.142.34.1086284769.squirrel@mail.mohawksoft.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches

> 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 ...

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2004-06-03 18:19:45 Re: [HACKERS] Configuration patch
Previous Message Tom Lane 2004-06-03 16:47:34 Re: [HACKERS] Configuration patch

Browse pgsql-patches by date

  From Date Subject
Next Message Tom Lane 2004-06-03 17:58:54 Re: Add error-checking to timestamp_recv
Previous Message Alvaro Herrera 2004-06-03 17:00:51 Re: fix oid casting inconsistency