Re: [Postgres-xc-developers] Patch for bug#3013984: Sequence scope

[mei le <lem...@ya...>]

Dears,

Thanks for your modifications and comments on this patch. I am sorry for my mistakes. I will be more careful during later work.

I checked the patch and tested it in our project. The result proved that it does work well.

Thanks again.

Regards,

                                       Benny
--- 10年11月12日，周五, Michael Paquier <mic...@gm...> 写道：


发件人: Michael Paquier <mic...@gm...>
主题: Re: [Postgres-xc-developers] Patch for bug#3013984: Sequence scope
收件人: "mei le" <lem...@ya...>
抄送: "pg" <pos...@li...>
日期: 2010年11月12日,周五,下午12:23


Hi Benny,

I had a look at your patch,
and it is really a nice feature you implemented.

With this functionnality, it is possible to drop sequences on GTM in a very smooth way when a database is dropped.
I have a couple of comments about it though.

1) in gtm_seq.c, function seq_start_with_nsp
You make a memory compare that will always return a true result because you compare the same string.
       return ((memcmp(nsp->gsk_key, nsp->gsk_key, nsp->gsk_keylen) == 0)      
                       && (seq->gsk_key[nsp->gsk_keylen] == '.'));
I believe this is the right code:
       return ((memcmp(nsp->gsk_key, seq->gsk_key, nsp->gsk_keylen) == 0)      
                       && (seq->gsk_key[nsp->gsk_keylen] == '.'));

2) in gtm_seq.c, function GTM_RmSeqWithNsp
When you encounter a sequence that is used by another process, you mark it for deletion and then you exit the process,
letting perhaps a lot of sequences not dropped.
I think priority should be given to deletion, as DROP DATABASE has to be forced.
Btw, a busy sequence cannot basically be found because that would mean that an instance is still using the database,
and in this case database drop is not possible.

3) in gtm_c.h, about the message type definition.
I made a modification in message type so as not to use a unique character.
Using an enumeration (typedef enum) makes it more consistent with Postgres-xc code, and it makes code maintenance easier.

4) Also I notice a couple of typo problems, take care when you use spaces and tabs! 

I corrected those points above in the patch attached.
I also changed a couple of function names to make it clearer.

You should check it and make a couple of additional tests also.
Perhaps you have additional ideas about this implementation, don't hesitate to share what you think.

Regards,

-- 
Michael Paquier
http://michaelpq.users.sourceforge.net