Subject: Re: POC: postgres_fdw insert batching
From: Tomas Vondra <tomas.vondra@enterprisedb.com>
To: Amit Langote <amitlangote09@gmail.com>,
 PostgreSQL-development <pgsql-hackers@postgresql.org>,
 "Tsunakawa, Takayuki" <tsunakawa.takay@fujitsu.com>
References: <20200628151002.7x5laxwpgvkyiu3q@development>
 <ebbd67c6-054a-f5eb-39a9-95e088f1c985@enterprisedb.com>
 <TYAPR01MB29903DF617C2B868C2477738FEA10@TYAPR01MB2990.jpnprd01.prod.outlook.com>
 <0fc809f6-7c3c-6d06-7ea3-f10765dfdf08@enterprisedb.com>
 <CA+HiwqHTnfTG52XxNZriFTr+1r1VyrLm_u-xU5cUuXW-XuLUcA@mail.gmail.com>
 <067ddf09-225c-e3c5-ea47-4dd39b34ebb0@enterprisedb.com>
 <CA+HiwqHrGA0HB5cjBL_0=Gk-u-dH=K8beAPibhOCFepU0kqndw@mail.gmail.com>
 <TYAPR01MB2990E3D10A122358419482B2FEBD0@TYAPR01MB2990.jpnprd01.prod.outlook.com>
 <CA+HiwqGjMVLEn8JoNT+XCeXA9SFECVvTEMawbzJ-RN_M+fxMTw@mail.gmail.com>
 <7ffcee1f-d55e-cbdf-5f3f-98f1c30537ef@enterprisedb.com>
 <CA+HiwqF4cB5AbJRW2RwqQfeOXwg0KjLB5t3Gh9JCiR7J-N8HwA@mail.gmail.com>
 <803d5507-299d-f109-7b71-b6bbbb17a053@enterprisedb.com>
 <CA+HiwqEPWAqheRFpyfQf2uO4cyhDEvhEwsc04_7GAxDuRGdBQg@mail.gmail.com>
 <CA+HiwqGyr_WKQ+-eR+hEHUwm4Frm==spQrdZtZU1S8WKT3YBaw@mail.gmail.com>
 <6929d485-2d2a-da46-3681-4a400a3d794f@enterprisedb.com>
Message-ID: <f44047f8-3bc9-51e1-61d8-7aaae56aee5a@enterprisedb.com>
Date: Thu, 18 Feb 2021 00:38:08 +0100
User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:78.0) Gecko/20100101
 Thunderbird/78.6.0
MIME-Version: 1.0
In-Reply-To: <6929d485-2d2a-da46-3681-4a400a3d794f@enterprisedb.com>
Content-Type: text/plain; charset=utf-8
Content-Language: en-US
Content-Transfer-Encoding: 7bit
Precedence: bulk

On 2/17/21 8:36 PM, Tomas Vondra wrote:
>
> ...
> 
> Thanks. The patch seems reasonable, but it's a bit too large for a fix,
> so I'll go ahead and push one of the previous fixes restricting batching
> to plain INSERT commands. But this seems useful, so I suggest adding it
> to the next commitfest.

I've pushed the v4 fix, adding the CMD_INSERT to execPartition.

I think we may need to revise the relationship between FDW and places
that (may) call GetForeignModifyBatchSize. Currently, these places need
to be quite well synchronized - in a way, the issue was (partially) due
to postgres_fdw and core not agreeing on the details.

In particular, create_foreign_modify sets batch_size for CMD_INSERT and
leaves it 0 otherwise. So GetForeignModifyBatchSize() returned 0 later,
triggering an assert.

It's probably better to require GetForeignModifyBatchSize() to always
return a valid batch size (>= 1). If batching is not supported, just
return 1.


regards

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