Re: units in postgresql.conf comments - Mailing list pgsql-hackers

From Magnus Hagander
Subject Re: units in postgresql.conf comments
Date
Msg-id CABUevExopB4ZmMNdaG3B-x+1M=dHwmEhwV=x37h3OJCYQ0sFyw@mail.gmail.com
Whole thread Raw
In response to Re: units in postgresql.conf comments  ("Joshua D. Drake" <jd@commandprompt.com>)
Responses Re: units in postgresql.conf comments  ("Joshua D. Drake" <jd@commandprompt.com>)
List pgsql-hackers
On Thu, May 30, 2013 at 3:52 AM, Joshua D. Drake <jd@commandprompt.com> wrote:
>
> On 05/30/2013 12:01 AM, Heikki Linnakangas wrote:
>
>> We could make it mandatory to specify the unit in the value. Ie. throw
>> an error on "wal_sender_timeout = 50":
>>
>> ERROR: unit required for option "wal_sender_timeout"
>> HINT:  Valid units for this parameter are "ms", "s", "min", "h", and "d".
>>
>> Then you wouldn't need a comment to explain what the unit of a naked
>> value is. The only problem I see with that is backwards-compatibility.
>> Old postgresql.conf files containing naked values would no longer work.
>> But all you'd need to do is to add in the units, which would work on
>> older versions too, and would be good for readability anyway.

In general, I like this. Requiring "full specification" is never
wrong. Except possibly for thje backwards compatible thing.


> I like this idea with one addition. We should have a default unit for each.
> For wal_sender_timeout seconds makes sense, but for checkpoint_timeout
> minutes makes sense (for example).

This sounds like a good way to make things even more confusing. Right
now the confusion is only in the comments - this would make it
confusing in the actual values.

Requiring a unit seems like a much better idea. That way, there is no
way for confusion.

--Magnus HaganderMe: http://www.hagander.net/Work: http://www.redpill-linpro.com/



pgsql-hackers by date:

Previous
From: Craig Ringer
Date:
Subject: Re: [PATCH] add --throttle to pgbench (submission 3)
Next
From: Andres Freund
Date:
Subject: Re: [PATCH] add --throttle to pgbench (submission 3)