On Mon, Nov 10, 2014 at 4:15 PM, Etsuro Fujita
<fujita.etsuro@lab.ntt.co.jp> wrote:
> (2014/11/06 23:38), Fujii Masao wrote:
>>
>> On Tue, Nov 4, 2014 at 12:04 PM, Etsuro Fujita
>> <fujita.etsuro@lab.ntt.co.jp> wrote:
>>>
>>> IIUC, I think that min = 0 disables fast update, so ISTM that it'd be
>>> appropriate to set min to some positive value. And ISTM that the idea of
>>> using the min value of work_mem is not so bad.
>>
>>
>> OK. I changed the min value to 64kB.
>>
>>> *** 356,361 **** CREATE [ UNIQUE ] INDEX [ CONCURRENTLY ] [ <replaceable
>>> class="parameter">name</
>>> --- 356,372 ----
>>> </listitem>
>>> </varlistentry>
>>> </variablelist>
>>> + <variablelist>
>>> + <varlistentry>
>>> + <term><literal>PENDING_LIST_CLEANUP_SIZE</></term>
>>>
>>> The above is still in uppercse.
>>
>>
>> Fixed.
>>
>> Attached is the updated version of the patch. Thanks for the review!
>
>
> Thanks for the updating the patch!
>
> The patch looks good to me except for the following point:
Thanks for the review again!
>
> *** a/src/backend/access/gin/ginfast.c
> --- b/src/backend/access/gin/ginfast.c
> ***************
> *** 25,30 ****
> --- 25,32 ----
> #include "utils/memutils.h"
> #include "utils/rel.h"
>
> + /* GUC parameter */
> + int pending_list_cleanup_size = 0;
>
> I think we need to initialize the GUC to boot_val, 4096 in this case.
No, IIUC basically the variable for GUC doesn't need to be initialized
to its default value. OTOH, it's also harmless to initialize it to the default.
I like the current code a bit because we don't need to change the initial
value again when we decide to change the default value of GUC.
I have no strong opinion about this, though.
Regards,
--
Fujii Masao