Horton

Thread

Commits

  1. ecpg: Refactor ecpg_log() to skip unnecessary calls to ECPGget_sqlca().

  1. ECPG Refactor: move sqlca variable in ecpg_log()

    Yuto Sasaki (Fujitsu) <sasaki.yuto-00@fujitsu.com> — 2024-10-10T02:04:28Z

    The sqlca variable in the ecpg_log() was declared with an unnecessarily wide
scope, so I moved to the appropriate place.
Please see attached.

  2. Re: ECPG Refactor: move sqlca variable in ecpg_log()

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2024-10-10T16:13:18Z

    On 2024-Oct-10, Yuto Sasaki (Fujitsu) wrote:

> The sqlca variable in the ecpg_log() was declared with an unnecessarily wide
> scope, so I moved to the appropriate place.

Hmm, I'm not sure we want that, because if we do this, then
ECPGget_sqlca() gets run with the debug_mutex held.  In the original
coding, it's run without the mutex.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
Voy a acabar con todos los humanos / con los humanos yo acabaré
voy a acabar con todos (bis) / con todos los humanos acabaré ¡acabaré! (Bender)

  3. Re: ECPG Refactor: move sqlca variable in ecpg_log()

    Yuto Sasaki (Fujitsu) <sasaki.yuto-00@fujitsu.com> — 2024-10-16T06:04:23Z

    >Hmm, I'm not sure we want that, because if we do this, then
>ECPGget_sqlca() gets run with the debug_mutex held.  In the original
>coding, it's run without the mutex.

Thank you for pointing that out. I agree with your observation. But there is
another room for improvement - we can avoid unnecessary calls to ECPGget_sqlca()
by moving just before mutex lock. PSA a patch.

Best regards,

________________________________
差出人: Alvaro Herrera <alvherre@alvh.no-ip.org>
送信日時: 2024年10月11日 1:13
宛先: Sasaki, Yuto/佐佐木 悠人 <sasaki.yuto-00@fujitsu.com>
CC: pgsql-hackers@lists.postgresql.org <pgsql-hackers@lists.postgresql.org>
件名: Re: ECPG Refactor: move sqlca variable in ecpg_log()

On 2024-Oct-10, Yuto Sasaki (Fujitsu) wrote:

> The sqlca variable in the ecpg_log() was declared with an unnecessarily wide
> scope, so I moved to the appropriate place.

Hmm, I'm not sure we want that, because if we do this, then
ECPGget_sqlca() gets run with the debug_mutex held.  In the original
coding, it's run without the mutex.

--
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/
Voy a acabar con todos los humanos / con los humanos yo acabaré
voy a acabar con todos (bis) / con todos los humanos acabaré ¡acabaré! (Bender)

  4. Re: ECPG Refactor: move sqlca variable in ecpg_log()

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2024-10-16T09:52:40Z

    
On 2024/10/16 15:04, Yuto Sasaki (Fujitsu) wrote:
>  >Hmm, I'm not sure we want that, because if we do this, then
>  >ECPGget_sqlca() gets run with the debug_mutex held.  In the original
>  >coding, it's run without the mutex.
> 
> Thank you for pointing that out. I agree with your observation. But there is
> another room for improvement - we can avoid unnecessary calls to ECPGget_sqlca()
> by moving just before mutex lock.

+1

> PSA a patch.

The patch looks good to me.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  5. Re: ECPG Refactor: move sqlca variable in ecpg_log()

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2024-10-18T05:45:41Z

    
On 2024/10/16 18:52, Fujii Masao wrote:
> 
> 
> On 2024/10/16 15:04, Yuto Sasaki (Fujitsu) wrote:
>>  >Hmm, I'm not sure we want that, because if we do this, then
>>  >ECPGget_sqlca() gets run with the debug_mutex held.  In the original
>>  >coding, it's run without the mutex.
>>
>> Thank you for pointing that out. I agree with your observation. But there is
>> another room for improvement - we can avoid unnecessary calls to ECPGget_sqlca()
>> by moving just before mutex lock.
> 
> +1
> 
>> PSA a patch.
> 
> The patch looks good to me.

I've attached the latest version of the patch, now including the commit log.
Unless there are any objections, I'll proceed with committing it.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  6. Re: ECPG Refactor: move sqlca variable in ecpg_log()

    Tom Lane <tgl@sss.pgh.pa.us> — 2024-10-18T17:43:45Z

    Fujii Masao <masao.fujii@oss.nttdata.com> writes:
> I've attached the latest version of the patch, now including the commit log.
> Unless there are any objections, I'll proceed with committing it.

LGTM.  Maybe move down the sqlca variable declaration, so that the
declarations still match the order in which the variables are
initialized?  That's just cosmetic of course.

			regards, tom lane

  7. Re: ECPG Refactor: move sqlca variable in ecpg_log()

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2024-10-21T14:25:18Z

    
On 2024/10/19 2:43, Tom Lane wrote:
> Fujii Masao <masao.fujii@oss.nttdata.com> writes:
>> I've attached the latest version of the patch, now including the commit log.
>> Unless there are any objections, I'll proceed with committing it.
> 
> LGTM.  Maybe move down the sqlca variable declaration, so that the
> declarations still match the order in which the variables are
> initialized?  That's just cosmetic of course.

Thanks for the review! I've updated the patch accordingly, i.e.,
moved the declaration of the sqlca variable right after fmt.
The v3 patch is attached.

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION

  8. Re: ECPG Refactor: move sqlca variable in ecpg_log()

    Fujii Masao <masao.fujii@oss.nttdata.com> — 2024-10-22T15:02:55Z

    
On 2024/10/21 23:25, Fujii Masao wrote:
> 
> 
> On 2024/10/19 2:43, Tom Lane wrote:
>> Fujii Masao <masao.fujii@oss.nttdata.com> writes:
>>> I've attached the latest version of the patch, now including the commit log.
>>> Unless there are any objections, I'll proceed with committing it.
>>
>> LGTM.  Maybe move down the sqlca variable declaration, so that the
>> declarations still match the order in which the variables are
>> initialized?  That's just cosmetic of course.
> 
> Thanks for the review! I've updated the patch accordingly, i.e.,
> moved the declaration of the sqlca variable right after fmt.
> The v3 patch is attached.

Pushed. Thanks!

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION