Re: POC: postgres_fdw insert batching

On 1/18/21 7:51 AM, tsunakawa.takay@fujitsu.com wrote:
> Tomas-san,
> 
> From: Amit Langote <amitlangote09@gmail.com>
>> Good thing you reminded me that this is about inserts, and in that 
>> case no, ExecInitModifyTable() doesn't know all leaf partitions,
>> it only sees the root table whose batch_size doesn't really matter.
>> So it's really ExecInitRoutingInfo() that I would recommend to set 
>> ri_BatchSize; right after this block:
>> 
>> /* * If the partition is a foreign table, let the FDW init itself
>> for * routing tuples to the partition. */ if
>> (partRelInfo->ri_FdwRoutine != NULL && 
>> partRelInfo->ri_FdwRoutine->BeginForeignInsert != NULL) 
>> partRelInfo->ri_FdwRoutine->BeginForeignInsert(mtstate,
>> partRelInfo);
>> 
>> Note that ExecInitRoutingInfo() is called only once for a
>> partition when it is initialized after being inserted into for the
>> first time.
>> 
>> For a non-partitioned targets, I'd still say set ri_BatchSize in 
>> ExecInitModifyTable().
> 
> Attached is the patch that added call to GetModifyBatchSize() to the
> above two places.  The regression test passes.
> 
> (FWIW, frankly, I prefer the previous version because the code is a
> bit smaller...  Maybe we should refactor the code someday to reduce
> similar processings in both the partitioned case and non-partitioned
> case.)
> 

Less code would be nice, but it's not always the right thing to do, 
unfortunately :-(

I took a look at this - there's a bit of bitrot due to 708d165ddb92c, so 
attached is a rebased patch (0001) fixing that.

0002 adds a couple comments and minor tweaks

0003 addresses a couple shortcomings related to explain - we haven't 
been showing the batch size for EXPLAIN (VERBOSE), because there'd be no 
FdwState, so this tries to fix that. Furthermore, there were no tests 
for EXPLAIN output with batch size, so I added a couple.


regards

-- 
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company