Postgres Pro
Downloads
Home   >   mailing lists

Thread: [PATCH] pgbench - refactor some connection finish/null into commonfunction

[PATCH] pgbench - refactor some connection finish/null into commonfunction

From
"Rady, Doug"
Date:

This patch refactors all of the connection state PQfinish() and NULL’ing into a single function.

Excludes PQfinish() in doConnect().

This came out of earlier ppoll() work where this was felt to be worthwhile but not directly relevant to that change.

 

doug

 

 

Attachment

Re: [PATCH] pgbench - refactor some connection finish/null intocommon function

From
Fabien COELHO
Date:
Hello Doug,

> This patch refactors all of the connection state PQfinish() and NULL’ing into a single function.
> Excludes PQfinish() in doConnect().

My 0.02€:

The argument could be "PGconn **" instead of a "CState *"?
If so, it may be used in a few more places. What is your opinion?

I'm fine with this kind of factorization which takes out a three-line 
pattern, but I'm wondering whether it would please committers.

-- 
Fabien.

Re: [PATCH] pgbench - refactor some connection finish/null intocommon function

From
"Rady, Doug"
Date:
On 1/30/18, 03:41, "Fabien COELHO" <coelho@cri.ensmp.fr> wrote:    
    Hello Doug,
Hi Fabien,
    
    > This patch refactors all of the connection state PQfinish() and NULL’ing into a single function.
    > Excludes PQfinish() in doConnect().
    
    My 0.02€:
    
    The argument could be "PGconn **" instead of a "CState *"?
    If so, it may be used in a few more places. What is your opinion?

I should have named  finishCon()  as  finishCStateCon()  since it was specific to that use pattern.
I'll resubmit with that change if you think it helps.
    
    I'm fine with this kind of factorization which takes out a three-line 
    pattern, but I'm wondering whether it would please committers.

Guess we'll find out ...
    
    -- 
    Fabien.
Thanks!
doug

Re: [PATCH] pgbench - refactor some connection finish/null intocommon function

From
Andres Freund
Date:
On 2018-02-21 19:11:15 +0000, Rady, Doug wrote:
> 
> On 1/30/18, 03:41, "Fabien COELHO" <coelho@cri.ensmp.fr> wrote:    
>     Hello Doug,
> Hi Fabien,
>     
>     > This patch refactors all of the connection state PQfinish() and NULL’ing into a single function.
>     > Excludes PQfinish() in doConnect().
>     
>     My 0.02€:
>     
>     The argument could be "PGconn **" instead of a "CState *"?
>     If so, it may be used in a few more places. What is your opinion?
> 
> I should have named  finishCon()  as  finishCStateCon()  since it was specific to that use pattern.
> I'll resubmit with that change if you think it helps.

I think the current name works well enough, and I think the PGconn **
version would be more complicated.


>     I'm fine with this kind of factorization which takes out a three-line 
>     pattern, but I'm wondering whether it would please committers.
> 
> Guess we'll find out ...

I would never have bothered on my own, but ...

Greetings,

Andres Freund