Re: Refactoring postgres_fdw/connection.c - Mailing list pgsql-hackers

From Fujii Masao
Subject Re: Refactoring postgres_fdw/connection.c
Date
Msg-id 7e71d7a9-54e1-79c0-f07c-943a0849b3b8@oss.nttdata.com
Whole thread Raw
In response to Re: Refactoring postgres_fdw/connection.c  (Kyotaro Horiguchi <horikyota.ntt@gmail.com>)
Responses Re: Refactoring postgres_fdw/connection.c
List pgsql-hackers

On 2022/07/26 16:25, Kyotaro Horiguchi wrote:
> Agree to that refactoring.  And it looks fine to me.

Thanks for reviewing the patches!


> I'm not sure the two are similar with each other.  The new function
> pgfdw_exec_pre_commit() looks like a merger of two isolated code paths
> intended to share a seven-line codelet.  I feel the code gets a bit
> harder to understand after the change.  I mildly oppose to this part.

If so, we can pgfdw_exec_pre_commit() into two, one is the common
function that sends or executes the command (i.e., calls
do_sql_command_begin() or do_sql_command()), and another is
the function only for toplevel. The latter function calls
the common function and then executes DEALLOCATE ALL things.

But this is not the way that other functions like pgfdw_abort_cleanup()
is implemented. Those functions have both codes for toplevel and
!toplevel (i.e., subxact), and run the processings depending
on the argument "toplevel". So I'm thinking that
pgfdw_exec_pre_commit() implemented in the same way is better.


> It gives the same feeling with 0002.  Considering that
> pending_deallocate becomes non-NIL only when toplevel, 38 lines out of
> 66 lines of the function are the toplevel-dedicated stuff.

Same as above.

Regards,

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



pgsql-hackers by date:

Previous
From: "shiy.fnst@fujitsu.com"
Date:
Subject: RE: [BUG] Logical replication failure "ERROR: could not map filenode "base/13237/442428" to relation OID" with catalog modifying txns
Next
From: Dilip Kumar
Date:
Subject: Re: Perform streaming logical transactions by background workers and parallel apply