Thread: Refactoring postgres_fdw code to rollback remote transaction
Hi, In postgres_fdw, pgfdw_xact_callback() and pgfdw_subxact_callback() do almost the same thing to rollback remote toplevel- and sub-transaction. But their such rollback logics are implemented separately and in different way. Which would decrease the readability and maintainability, I think. So how about making the common function so that those callback functions can just use it? Patch attached. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
Attachment
On Thu, Sep 16, 2021 at 7:31 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
Hi,
In postgres_fdw, pgfdw_xact_callback() and pgfdw_subxact_callback() do
almost the same thing to rollback remote toplevel- and sub-transaction.
But their such rollback logics are implemented separately and
in different way. Which would decrease the readability and maintainability,
I think. So how about making the common function so that those callback
functions can just use it? Patch attached.
Regards,
--
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION
Hi,
+ goto fail; /* Trouble clearing prepared statements */
The label fail can be removed. Under the above condition, entry->changing_xact_state is still true. You can directly return.
Cheers
On 2021/09/17 11:40, Zhihong Yu wrote: > + goto fail; /* Trouble clearing prepared statements */ > > The label fail can be removed. Under the above condition, entry->changing_xact_state is still true. You can directly return. Thanks for the review! Yes, you're right. Attached the updated version of the patch. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
Attachment
On Fri, Sep 17, 2021 at 8:28 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote: > > On 2021/09/17 11:40, Zhihong Yu wrote: > > + goto fail; /* Trouble clearing prepared statements */ > > > > The label fail can be removed. Under the above condition, entry->changing_xact_state is still true. You can directlyreturn. > > Thanks for the review! Yes, you're right. Attached the updated version of the patch. +1 for the code refactoring (1 file changed, 75 insertions(+), 102 deletions(-)). The v2 patch looks good to me as is. Regards, Bharath Rupireddy.
On 2021/09/17 15:33, Bharath Rupireddy wrote: > On Fri, Sep 17, 2021 at 8:28 AM Fujii Masao <masao.fujii@oss.nttdata.com> wrote: >> >> On 2021/09/17 11:40, Zhihong Yu wrote: >>> + goto fail; /* Trouble clearing prepared statements */ >>> >>> The label fail can be removed. Under the above condition, entry->changing_xact_state is still true. You can directlyreturn. >> >> Thanks for the review! Yes, you're right. Attached the updated version of the patch. > > +1 for the code refactoring (1 file changed, 75 insertions(+), 102 > deletions(-)). > > The v2 patch looks good to me as is. Thanks for the review! Barring any objection, I will commit the patch. Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION
On 2021/09/22 0:16, Fujii Masao wrote: > Thanks for the review! Barring any objection, I will commit the patch. Pushed. Thanks! Regards, -- Fujii Masao Advanced Computing Technology Center Research and Development Headquarters NTT DATA CORPORATION