Thread: postgresql.auto.conf comments

postgresql.auto.conf comments

From
Thom Brown
Date:
Hi,

I haven't seen this mentioned anywhere (although it may have as I haven't read through the entire history of it), but would others find it useful to have ALTER SYSTEM support comments?

e.g.

ALTER SYSTEM SET autovacuum_vacuum_scale_factor = 0.01
WITH [ INLINE | HEADLINE ] COMMENT $$As most of the tables on the system are so big, we're setting this parameter lower than the default to keep bloat under more control.$$;

I just made up inline and headline to suggest that we could allow either:

autovacuum_vacuum_scale_factor = 0.01 # As most of the tables...

and

# As most of the tables on the system are so big, we're setting this parameter
# lower than the default to keep bloat under more control.
autovacuum_vacuum_scale_factor = 0.01


The rationale being that it's often the case one wants to document the reason for a parameter being configured so, but there's no way of doing this for settings in postgresql.auto.conf as they'll be wiped out if added manually.

Thom

Re: postgresql.auto.conf comments

From
Stephen Frost
Date:
* Thom Brown (thom@linux.com) wrote:
> I haven't seen this mentioned anywhere (although it may have as I haven't
> read through the entire history of it), but would others find it useful to
> have ALTER SYSTEM support comments?

I do think it'd be useful.  I don't think 'inline' deserves inclusion
and just complicates it more than necessary (my 2c at least).  I'd just
do them all as 'headline' and wrap at 80 chars.

I will point out that this use of COMMENT is novel though, no?  Comments
are normally handled as "COMMENT ON blah IS 'whatever';"  ALTER SYSTEM
is certainly special but I'm not sure I like the idea of having some
commands which support in-command COMMENT while others don't.

> The rationale being that it's often the case one wants to document the
> reason for a parameter being configured so, but there's no way of doing
> this for settings in postgresql.auto.conf as they'll be wiped out if added
> manually.

<<Comments about postgresql.auto.conf redacted>>
Thanks,
    Stephen

Re: postgresql.auto.conf comments

From
Thom Brown
Date:
On 24 November 2014 at 20:40, Stephen Frost <sfrost@snowman.net> wrote:
* Thom Brown (thom@linux.com) wrote:
> I haven't seen this mentioned anywhere (although it may have as I haven't
> read through the entire history of it), but would others find it useful to
> have ALTER SYSTEM support comments?

I do think it'd be useful.  I don't think 'inline' deserves inclusion
and just complicates it more than necessary (my 2c at least).  I'd just
do them all as 'headline' and wrap at 80 chars.
 
I guess it would ensure consistency.

I will point out that this use of COMMENT is novel though, no?  Comments
are normally handled as "COMMENT ON blah IS 'whatever';"  ALTER SYSTEM
is certainly special but I'm not sure I like the idea of having some
commands which support in-command COMMENT while others don't.

I typed that out in my original email, thought about it, then removed it because I decided that perhaps it isn't the same class as comment as COMMENT ON uses.  That affects objects, whereas this would apply to individual config parameters within a file.  Also bear in mind that if someone runs:

SHOW maintenance_work_mem;

And sees "4GB", they may decide to add a comment based on that, even though the source of that setting isn't postgresql.auto.conf.

Thom

Re: postgresql.auto.conf comments

From
Robert Haas
Date:
On Mon, Nov 24, 2014 at 3:26 PM, Thom Brown <thom@linux.com> wrote:
> I haven't seen this mentioned anywhere (although it may have as I haven't
> read through the entire history of it), but would others find it useful to
> have ALTER SYSTEM support comments?

Oh, please no.

The main thing that caused us to have no way of modifying
postgresql.conf via SQL for so many years is that it's not clear how
you can sensibly rewrite a file with comments in it.  For example, the
default postgresql.conf file has stuff like this in it:

#variable = someval

If variable gets set to a non-default value, you might want to
uncomment that line, but now you have to parse the comments, which
will be tedious and error-prone and sometimes make stupid decisions:

#Back in days of yore when dinosaurs ruled the earth, we had
#autovacuum_naptime=1h, but that turned out to be a bad idea.
#
#autovacuum_naptime=1min

It's hard for a non-human to know that the second one is the one that
you should uncomment.  There are lots of other problems that arise,
too; this is just an example.

It would perhaps be OK to have comments in postgresql.conf.auto if
they were designated in some way that told us which specific comment
was associated with which specific setting.  But we need to be very
careful not to design something that requires us to write a parser
that can ferret out human intent from context clues.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: postgresql.auto.conf comments

From
Thom Brown
Date:
On 24 November 2014 at 21:04, Robert Haas <robertmhaas@gmail.com> wrote:
On Mon, Nov 24, 2014 at 3:26 PM, Thom Brown <thom@linux.com> wrote:
> I haven't seen this mentioned anywhere (although it may have as I haven't
> read through the entire history of it), but would others find it useful to
> have ALTER SYSTEM support comments?

Oh, please no.

The main thing that caused us to have no way of modifying
postgresql.conf via SQL for so many years is that it's not clear how
you can sensibly rewrite a file with comments in it.  For example, the
default postgresql.conf file has stuff like this in it:

#variable = someval

If variable gets set to a non-default value, you might want to
uncomment that line, but now you have to parse the comments, which
will be tedious and error-prone and sometimes make stupid decisions:

#Back in days of yore when dinosaurs ruled the earth, we had
#autovacuum_naptime=1h, but that turned out to be a bad idea.
#
#autovacuum_naptime=1min

I'm not sure this is an argument against supporting comments to postgresql.auto.conf since it's specifically intended not to be edited by humans, and commenting out a parameter will remove it from the file upon further ALTER SYSTEM invocations anyway.

It would perhaps be OK to have comments in postgresql.conf.auto if
they were designated in some way that told us which specific comment
was associated with which specific setting.  But we need to be very
careful not to design something that requires us to write a parser
that can ferret out human intent from context clues. 

Perhaps the parser could automatically remove any comment blocks which are followed by a blank/empty line.

So if we had:

---------------------
work_mem = '4MB'
# Set this lower as we're using a really fast disk interace
#seq_page_cost = '0.5'

# Set random_page_cost lower as we're using an SSD
random_page_cost = '1.0'
# This line has been manually added by a human without a newline
maintenance_work_mem = '1GB'

# This is an orphaned comment
---------------------

I would expect the next modification to the file to cause reduce it to:

---------------------
work_mem = '4MB'

# Set random_page_cost lower as we're using an SSD
random_page_cost = 1.0

# This line has been manually added
maintenance_work_mem = '1GB'
---------------------
 
Thom

Re: postgresql.auto.conf comments

From
Robert Haas
Date:
On Mon, Nov 24, 2014 at 4:22 PM, Thom Brown <thom@linux.com> wrote:
> Perhaps the parser could automatically remove any comment blocks which are
> followed by a blank/empty line.

Well, if we can agree on something, it doesn't bother me any.  I'm
just saying we spent years arguing about it, because we couldn't agree
on anything.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



Re: postgresql.auto.conf comments

From
Stephen Frost
Date:
* Thom Brown (thom@linux.com) wrote:
> On 24 November 2014 at 20:40, Stephen Frost <sfrost@snowman.net> wrote:
> > I will point out that this use of COMMENT is novel though, no?  Comments
> > are normally handled as "COMMENT ON blah IS 'whatever';"  ALTER SYSTEM
> > is certainly special but I'm not sure I like the idea of having some
> > commands which support in-command COMMENT while others don't.
>
> I typed that out in my original email, thought about it, then removed it
> because I decided that perhaps it isn't the same class as comment as
> COMMENT ON uses.  That affects objects, whereas this would apply to
> individual config parameters within a file.  Also bear in mind that if
> someone runs:
>
> SHOW maintenance_work_mem;
>
> And sees "4GB", they may decide to add a comment based on that, even though
> the source of that setting isn't postgresql.auto.conf.

I'd be fine with supporting two distinct object type or perhaps one
object type and two sub types to allow those to be different (no clue
what the actual syntax would be..).

I'd actually prefer that these comments be represented in both
pg_description as well as being in the actual config file- otherwise how
are you going to be able to view what's there from the database before
you go and change it?  The fact that the information is also persisted
into the actual .auto.conf file is a convenience for admins who happen
to be looking there but I'd consider what's in pg_description to be the
canonical source.
Thanks,
    Stephen

Re: postgresql.auto.conf comments

From
Stephen Frost
Date:
* Robert Haas (robertmhaas@gmail.com) wrote:
> On Mon, Nov 24, 2014 at 4:22 PM, Thom Brown <thom@linux.com> wrote:
> > Perhaps the parser could automatically remove any comment blocks which are
> > followed by a blank/empty line.
>
> Well, if we can agree on something, it doesn't bother me any.  I'm
> just saying we spent years arguing about it, because we couldn't agree
> on anything.

I'm mystified by this whole discussion.

For my 2c (and, yes, it's far too late to change most likely, but too
bad) is that the entirety of postgresql.auto.conf should be generated,
and generated with the catalog as the canonical source.  No one should
ever be modifying it directly and if they do then we act just as badly
as if they go hand-modifying heap files and screw it up.

We shouldn't have ever allowed postgresql.auto.conf to be the canonical
source of anything.  We didn't do that with pg_shadow back when we
required that and I don't see any good reason to that now.
Thanks,
    Stephen

Re: postgresql.auto.conf comments

From
Amit Kapila
Date:
On Tue, Nov 25, 2014 at 1:56 AM, Thom Brown <thom@linux.com> wrote:
>
> Hi,
>
> I haven't seen this mentioned anywhere (although it may have as I haven't read through the entire history of it), but would others find it useful to have ALTER SYSTEM support comments?
>
> e.g.
>
> ALTER SYSTEM SET autovacuum_vacuum_scale_factor = 0.01
> WITH [ INLINE | HEADLINE ] COMMENT $As most of the tables on the system are so big, we're setting this parameter lower than the default to keep bloat under more control.$;
>
> I just made up inline and headline to suggest that we could allow either:
>
> autovacuum_vacuum_scale_factor = 0.01 # As most of the tables...
>
> and
>
> # As most of the tables on the system are so big, we're setting this parameter
> # lower than the default to keep bloat under more control.
> autovacuum_vacuum_scale_factor = 0.01
>

I think the biggest hurdle to achieve this is to enhance/implement
the parser for it, right now the function to parse config file
(ParseConfigFp()) gives us the list of name-value pair item's.  I think
if we could improve it or write a different function which can return
name-value-comment item's, then it won't be too difficult for
Alter System to support it.

>
> The rationale being that it's often the case one wants to document the reason for a parameter being configured so, but there's no way of doing this for settings in postgresql.auto.conf as they'll be wiped out if added manually.
>

I think this is completely genuine request and I have seen that other
systems which support Alter System does support comments along
with each entry.

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