Re: Specifying the unit in storage parameter

On Thu, Aug 21, 2014 at 4:20 PM, Michael Paquier
<michael(dot)paquier(at)gmail(dot)com> wrote:
> On Fri, Aug 8, 2014 at 12:32 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> This is not user-friendly. I'd like to propose the attached patch which
>> introduces the infrastructure which allows us to specify the unit when
>> setting INTEGER storage parameter like autovacuum_vacuum_cost_delay.
>> Comment? Review?
> This patch makes autovacuum_vacuum_cost_delay more consistent with
> what is at server level. So +1.

Thanks for reviewing the patch!

> Looking at the patch, the parameter "fillfactor" in the category
> RELOPT_KIND_HEAP (the first element in intRelOpts of reloptions.c) is
> not updated with the new field. It is only a one-line change.
> @@ -97,7 +97,7 @@ static relopt_int intRelOpts[] =
> "Packs table pages only to this percentage",
> RELOPT_KIND_HEAP
> },
> - HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100
> + HEAP_DEFAULT_FILLFACTOR, HEAP_MIN_FILLFACTOR, 100, 0
> },

Oh, good catch. I wonder why I did such a mistake...
Attached is the updated version of the patch.

> Except that, I tested as well the patch and it works as expected. The
> documentation, as well as the regression tests remain untouched, but I
> guess that this is fine (not seeing similar tests in regressions, and
> documentation does not specify the unit for a given parameter).

I think that it's worth adding the regression test for this feature.
Attached patch updates the regression test.

Regards,

--
Fujii Masao