Thread: 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)
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
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/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