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

2018-03-01 Thread Andres Freund
On 2018-02-21 19:11:15 +, Rady, Doug wrote:
> 
> On 1/30/18, 03:41, "Fabien COELHO"  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



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

2018-02-21 Thread Rady, Doug

On 1/30/18, 03:41, "Fabien COELHO"  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 into common function

2018-01-30 Thread Fabien COELHO


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.