Re: POC: postgres_fdw insert batching

From: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
To: "tsunakawa(dot)takay(at)fujitsu(dot)com" <tsunakawa(dot)takay(at)fujitsu(dot)com>
Cc: 'Amit Langote' <amitlangote09(at)gmail(dot)com>, "Andrey V(dot) Lepikhov" <a(dot)lepikhov(at)postgrespro(dot)ru>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: POC: postgres_fdw insert batching
Date: 2021-01-18 17:56:23
Message-ID: 16fcfd4c-2d5e-67f7-fa69-9b5d119bac52@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 1/18/21 7:51 AM, tsunakawa(dot)takay(at)fujitsu(dot)com wrote:
> Tomas-san,
>
> From: Amit Langote <amitlangote09(at)gmail(dot)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

Attachment Content-Type Size
0001-Add-bulk-insert-for-foreign-tables-v11.patch text/x-patch 49.8 KB
0002-tweaks-v11.patch text/x-patch 3.8 KB
0003-explain-v11.patch text/x-patch 12.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2021-01-18 18:00:38 Re: simplifying foreign key/RI checks
Previous Message Zhihong Yu 2021-01-18 17:48:47 Re: simplifying foreign key/RI checks