Skip site navigation (1) Skip section navigation (2)

Re: PATCH: optimized DROP of multiple tables within a transaction

From: Shigeru Hanada <shigeru(dot)hanada(at)gmail(dot)com>
To: Tomas Vondra <tv(at)fuzzy(dot)cz>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: PATCH: optimized DROP of multiple tables within a transaction
Date: 2013-01-08 02:47:34
Message-ID: CAEZqfEcrVrW=Em2_K0-HmLsb3jLr0EkJrJh0D3YF=LWTL7hq6g@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackers
Hi Tomas,

On Tue, Jan 8, 2013 at 7:08 AM, Tomas Vondra <tv(at)fuzzy(dot)cz> wrote:
>> * I found another extra space after asterisk.
>>
>> +     RelFileNode * nodes;
>
> Thanks, fixed.

check

>> * Curly bracket which starts code block should be at the head of next line.
>>
>> +                             /* extend the array if needed (double the size) */
>> +                             if (maxrels <= nrels) {
>
> Fixed.

check

>> * There are several trailing tabs in src/backend/catalog/storage.c.
>
> Fixed (I balieve).

check

>> * naming of DROP_RELATIONS_BSEARCH_LIMIT (or off-by-one bug?)
>> IIUC bsearch is used when # of relations to be dropped is *more* than
>> the value of DROP_RELATIONS_BSEARCH_LIMIT (10).  IMO this behavior is
>> not what the macro name implies; I thought that bsearch is used for 10
>> relations.  Besides, the word "LIMIT" is used as *upper limit* in
>> other macros.  How about MIN_DROP_REL[ATION]S_BSEARCH or
>> DROP_REL[ATION]S_LINEAR_SEARCH_LIMIT?
>> # using RELS instead of RELATIONS would be better to shorten the name
>>
>
> I've changed the name to DROP_RELS_BSEARCH_THRESHOLD. I believe this
> naming is consistent with options like "geqo_threshold" - when the
> number of relations reaches the specified value, the bsearch is used.
>
> I've also increased the value from 10 to 20, in accordance with the
> previous discussion.

New name sounds good to me, but the #define says that the value is 15.
Should it be fixed to 20?

>> * +1 for Alvaro's suggestion about avoiding palloc traffic by starting
>> with 8 elements or so.
>>
>
> Done.

Not yet.  Initial size of srels array is still 1 element.

Regards,
-- 
Shigeru HANADA


In response to

Responses

pgsql-hackers by date

Next:From: Noah MischDate: 2013-01-08 02:49:57
Subject: lazy_vacuum_heap()'s removal of HEAPTUPLE_DEAD tuples
Previous:From: Bruce MomjianDate: 2013-01-08 00:48:44
Subject: Re: A very small typo in the comment

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group