Re: postgresql.auto.conf read from wrong directory

From: Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Abhijit Menon-Sen <ams(at)2ndquadrant(dot)com>
Cc: Christoph Berg <cb(at)df7cb(dot)de>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Andres Freund <andres(at)2ndquadrant(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: postgresql.auto.conf read from wrong directory
Date: 2014-06-17 04:19:35
Message-ID: CAA4eK1KuEY7KaEQg=3Gfm8-bya4V_8ShWbeFRS1GnCWxcKJ9hQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Jun 16, 2014 at 7:06 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> > Developer options are mainly for debugging information or might help in
one
> > of the situations, so I thought somebody might not want them to be part
of
> > server configuration once they are set. We already disallow parameters
like
> > config_file, transaction_isolation, etc. which are disallowed to be set
in
> > postgresql.conf. Could you please explain me a bit in which
> > situations/scenarios, do you think that allowing developer options via
Alter
> > System can be helpful?
>
> I think that's helpful. I've heard that some users enable the developer
option
> "trace_sort" in postgresql.conf in order to collect the information
> about sorting.
> They might want to set trace_sort via ALTER SYSTEM, for example.

Okay, if you have usecase where people use such parameters in
postegresql.conf, then it makes sense. I have removed that restriction
from patch.

> I suggest the following wording:
>
> This command may not be used to set
> <xref linkend="guc-data-directory">
> or any parameters that are not allowed in postgresql.conf.

I think we should not use *may* in this line, because in no
circumstance we will allow this command for the parameters
mentioned. I have changed it as per your suggestion.

> > + /*
> > + * Disallow parameter's that are excluded or disallowed in
> > + * postgresql.conf.
> > + */
>
> "parameters", no apostrophe.
okay.
I have changed above comment as earlier comment has
information about developer option which we don't need now.

> > if ((record->context == PGC_INTERNAL) ||
> > - (record->flags & GUC_DISALLOW_IN_FILE))
> > - ereport(ERROR,
> > -
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
> > - errmsg("parameter \"%s\" cannot be
changed",
> > - name)));
> > + (record->flags & GUC_DISALLOW_IN_FILE) ||
> > + (record->flags & GUC_DISALLOW_IN_AUTO_FILE) ||
> > + (record->flags & GUC_NOT_IN_SAMPLE))
> > + ereport(ERROR,
> > +
(errcode(ERRCODE_CANT_CHANGE_RUNTIME_PARAM),
> > + errmsg("parameter \"%s\" cannot be
changed",
> > + name)));
>
> I looked at the settings that are marked GUC_NOT_IN_SAMPLE but neither
> PGC_INTERNAL nor GUC_DISALLOW_IN_*FILE. I don't feel strongly about it,
> but I don't see any particularly good reason to exclude them here.

By above do you mean that the patch should allow GUC_NOT_IN_SAMPLE or
something else, if earlier then I have removed it as per comment from
Fujji-san.

With Regards,
Amit Kapila.
EnterpriseDB: http://www.enterprisedb.com

Attachment Content-Type Size
prohibit_data_dir_by_alter_system-v3.patch application/octet-stream 2.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Geoghegan 2014-06-17 04:21:57 Re: Doing better at HINTing an appropriate column within errorMissingColumn()
Previous Message Tom Lane 2014-06-17 03:56:39 Re: Doing better at HINTing an appropriate column within errorMissingColumn()