Re: Specifying the unit in storage parameter

On Thu, Aug 21, 2014 at 4:20 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On Fri, Aug 8, 2014 at 12:32 PM, Fujii Masao <masao.fujii@gmail.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