Re: [PATCHES] Configuration patch

From: Bruce Momjian <pgman(at)candle(dot)pha(dot)pa(dot)us>
To: pgsql(at)mohawksoft(dot)com
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCHES] Configuration patch
Date: 2004-06-24 11:00:59
Message-ID: 200406241100.i5OB0xg07696@candle.pha.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers pgsql-patches


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

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Merlin Moncure 2004-06-24 12:51:32 Re: PREPARE and transactions
Previous Message Gavin Sherry 2004-06-24 10:38:36 Re: pg_largeobject and tablespaces

Browse pgsql-patches by date

  From Date Subject
Next Message Kris Jurka 2004-06-24 12:27:38 pg_dump --clean w/ <= 7.2 server
Previous Message Gavin Sherry 2004-06-24 10:31:51 Include tablespace information in psql \d footers