Re: POC: postgres_fdw insert batching

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