Re: [PATCH] Cleanup of GUC units code

From: Simon Riggs <simon(at)2ndQuadrant(dot)com>
To: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Cc: Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, Marko Kreen <markokr(at)gmail(dot)com>, Peter Eisentraut <peter_e(at)gmx(dot)net>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Asko Oja <ascoja(at)gmail(dot)com>
Subject: Re: [PATCH] Cleanup of GUC units code
Date: 2008-09-08 11:08:26
Message-ID: 1220872106.3913.120.camel@ebony.2ndQuadrant
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers


On Wed, 2008-09-03 at 11:58 +0300, Asko Oja wrote:
> On Wed, Sep 3, 2008 at 11:20 AM, Heikki Linnakangas
> <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> Marko Kreen wrote:
> On 9/2/08, Peter Eisentraut <peter_e(at)gmx(dot)net> wrote:
> Marko Kreen wrote:
> In the meantime, here is simple patch
> for case-insensivity.
> You might be able to talk me into accepting
> various unambiguous, common
> alternative spellings of various units. But
> for instance allowing MB and Mb
> to mean the same thing is insane.
>
> How would the docs for that look like? And anyway,
> what is wrong with
> Mb for megabytes?
> From infamous wikipedia: A megabit is a unit of information or
> computer storage, abbreviated Mbit (or Mb).
> To me playing with case of acronyms and even depending on it seems
> more insane. It would make much more sense to have case insensitive
> set of acronyms and (thanks Tom for pointing out) some sanity checks
> when configuration is loaded to notify user when wrong ones are used
> for some context.

There is a patch on the CommitFest, so we must either accept it or
reject it (with/without comments).

Peter's objection is reasonable, as far as most people have replied.
Marko's proposal is also reasonable to most people, since they do not
wish fat fingers to cause any amount of downtime. ISTM that if you've
done this, you appreciate the feature, if not it seems less important.

I would point out that Marko's patch is about what values get accepted
in postgresql.conf, not how the units are represented when you perform
SHOW, look at pg_settings or read the docs. So Marko's proposal does not
ignore the important distinction between units in *all* places, just at
the time of input. With that in mind, the proposal seems to be
acceptable to the majority, based upon my assessment of the comments.

In terms of the patch, ISTM that relaxing the comparison logic also
appears to relax the warnings given and that is not acceptable, given
concerns raised.

So my recommendation to everybody is
* we allow case insensitive matches of units in postgresql.conf
* Marko should change patch to put WARNINGs in place so people know they
got it wrong
* we make sure the case is always shown correctly in all other aspects
of Postgres server and docs (no relaxation at all there)
* in the longer term, we look for the solution to be a config checker

--
Simon Riggs www.2ndQuadrant.com
PostgreSQL Training, Services and Support

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Simon Riggs 2008-09-08 11:11:31 Re: Our CLUSTER implementation is pessimal
Previous Message Heikki Linnakangas 2008-09-08 10:52:26 Re: Our CLUSTER implementation is pessimal