ECPG Refactor: move sqlca variable in ecpg_log()
The sqlca variable in the ecpg_log() was declared with an unnecessarily wide
scope, so I moved to the appropriate place.
Please see attached.
Attachments:
scope.diffapplication/octet-stream; name=scope.diffDownload+7-4
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)
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)
Attachments:
change_scope.diffapplication/octet-stream; name=change_scope.diffDownload+3-1
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
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
Attachments:
v2-0001-ecpg-Refactor-ecpg_log-to-skip-unnecessary-calls-.patchtext/plain; charset=UTF-8; name=v2-0001-ecpg-Refactor-ecpg_log-to-skip-unnecessary-calls-.patchDownload+3-2
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
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
Attachments:
v3-0001-ecpg-Refactor-ecpg_log-to-skip-unnecessary-calls-.patchtext/plain; charset=UTF-8; name=v3-0001-ecpg-Refactor-ecpg_log-to-skip-unnecessary-calls-.patchDownload+3-2
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