Re: POC: postgres_fdw insert batching

From: Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>
To: Amit Langote <amitlangote09(at)gmail(dot)com>, "tsunakawa(dot)takay(at)fujitsu(dot)com" <tsunakawa(dot)takay(at)fujitsu(dot)com>
Cc: "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-19 16:01:05
Message-ID: 9d94c863-ce7c-ea7f-6b84-834c98929e2f@enterprisedb.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 1/19/21 7:23 AM, Amit Langote wrote:
> On Tue, Jan 19, 2021 at 2:06 PM tsunakawa(dot)takay(at)fujitsu(dot)com
> <tsunakawa(dot)takay(at)fujitsu(dot)com> wrote:
>> From: Amit Langote <amitlangote09(at)gmail(dot)com>
>>> I apologize in advance for being maybe overly pedantic, but I noticed
>>> that, in ExecInitModifyTable(), you decided to place the call outside
>>> the loop that goes over resultRelations (shown below), although my
>>> intent was to ask to place it next to the BeginForeignModify() in that
>>> loop.
>>
>> Actually, I tried to do it (adding the GetModifyBatchSize() call after BeginForeignModify()), but it failed. Because postgresfdwGetModifyBatchSize() wants to know if RETURNING is specified, and ResultRelInfo->projectReturning is created after the above part. Considering the context where GetModifyBatchSize() implementations may want to know the environment, I placed the call as late as possible in the initialization phase. As for the future(?) multi-target DML statements, I think we can change this together with other many(?) parts that assume a single target table.
>
> Okay, sometime later then.
>
> I wasn't sure if bringing it up here would be appropriate, but there's
> a patch by me to refactor ModfiyTable result relation allocation that
> will have to remember to move this code along to an appropriate place
> [1]. Thanks for the tip about the dependency on how RETURNING is
> handled. I will remember it when rebasing my patch over this.
>

Thanks. The last version (v12) should be addressing all the comments and
seems fine to me, so barring objections I'll get that pushed shortly.

One thing that seems a bit annoying is that with the partitioned table
the explain (verbose) looks like this:

QUERY PLAN
-----------------------------------------------------
Insert on public.batch_table
-> Function Scan on pg_catalog.generate_series i
Output: i.i
Function Call: generate_series(1, 66)
(4 rows)

That is, there's no information about the batch size :-( But AFAICS
that's due to how explain shows (or rather does not) partitions in this
type of plan.

regards

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Dilip Kumar 2021-01-19 16:02:31 Re: Is Recovery actually paused?
Previous Message Tom Lane 2021-01-19 15:57:25 Re: [PATCH 1/1] Fix detection of pwritev support for OSX.