On Mon, Nov 10, 2014 at 4:15 PM, Etsuro Fujita <fujita.ets...@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.ets...@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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
