From: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com> |
---|---|
To: | Amit Langote <amitlangote09(at)gmail(dot)com> |
Cc: | "tsunakawa(dot)takay(at)fujitsu(dot)com" <tsunakawa(dot)takay(at)fujitsu(dot)com>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, "Andrey V(dot) Lepikhov" <a(dot)lepikhov(at)postgrespro(dot)ru>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: POC: postgres_fdw insert batching |
Date: | 2021-01-13 17:41:09 |
Message-ID: | d682fcf2-41db-8e8c-1f4c-a1cb4e481bab@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 1/13/21 3:43 PM, Tomas Vondra wrote:
>
> ...
>
> Thanks for the report. Yeah, I think there's a missing check in
> ExecInsert. Adding
>
> (!resultRelInfo->ri_TrigDesc->trig_insert_after_row)
>
> solves this. But now I'm wondering if this is the wrong place to make
> this decision. I mean, why should we make the decision here, when the
> decision whether to have a RETURNING clause is made in postgres_fdw in
> deparseReturningList? We don't really know what the other FDWs will do,
> for example.
>
> So I think we should just move all of this into GetModifyBatchSize. We
> can start with ri_BatchSize = 0. And then do
>
> if (resultRelInfo->ri_BatchSize == 0)
> resultRelInfo->ri_BatchSize =
> resultRelInfo->ri_FdwRoutine->GetModifyBatchSize(resultRelInfo);
>
> if (resultRelInfo->ri_BatchSize > 1)
> {
> ... do batching ...
> }
>
> The GetModifyBatchSize would always return value > 0, so either 1 (no
> batching) or >1 (batching).
>
FWIW the attached v8 patch does this - most of the conditions are moved
to the GetModifyBatchSize() callback. I've removed the check for the
BatchInsert callback, though - the FDW knows whether it supports that,
and it seems a bit pointless at the moment as there are no other batch
callbacks. Maybe we should add an Assert somewhere, though?
regards
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
Attachment | Content-Type | Size |
---|---|---|
0001-Add-bulk-insert-for-foreign-tables-v8.patch | text/x-patch | 48.8 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Geoghegan | 2021-01-13 17:45:52 | Re: Deleting older versions in unique indexes to avoid page splits |
Previous Message | Alvaro Herrera | 2021-01-13 17:33:43 | Re: [DOC] Document concurrent index builds waiting on each other |