RE: Fdw batch insert error out when set batch_size > 65535

From: Bharath Rupireddy <bharath(dot)rupireddyforpostgres(at)gmail(dot)com>
Sent: Wednesday, May 26, 2021 9:56 PM
> On Wed, May 26, 2021 at 6:36 PM Tomas Vondra
> <tomas(dot)vondra(at)enterprisedb(dot)com> wrote:
> >
> > On 5/26/21 8:57 AM, Bharath Rupireddy wrote:
> > > On Tue, May 25, 2021 at 2:47 PM Bharath Rupireddy
> > > <bharath(dot)rupireddyforpostgres(at)gmail(dot)com> wrote:
> > >>
> > >> On Tue, May 25, 2021 at 1:08 PM houzj(dot)fnst(at)fujitsu(dot)com
> > >> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> > >>> Thanks for the comments. I have addressed all comments to the v3
> patch.
> > >>
> > >> Thanks! The patch basically looks good to me except that it is
> > >> missing a commit message. I think it can be added now.
> > >
> > > With v3 patch, I observed failure in postgres_fdw test cases with
> > > insert query in prepared statements. Root cause is that in
> > > postgresGetForeignModifyBatchSize, fmstate can be null (see the
> > > existing code which handles for fmstate null cases). I fixed this,
> > > and added a commit message. PSA v4 patch.
> > >
> >
> > Thanks. In what situation is the fmstate NULL? If it is NULL, the
> > current code simply skips the line adjusting it. Doesn't that mean we
> > may not actually fix the bug in that case?
>
> fmstate i.e. resultRelInfo->ri_FdwState is NULL for EXPLAIN without ANALYZE
> cases, below comment says it and we can't get the bug because we don't
> actually execute the insert statement. The bug occurs on the remote server
> when the insert query with those many query parameters is submitted to the
> remote server.

Agreed.
The "ri_FdwState" is initialized in postgresBeginForeignInsert or postgresBeginForeignModify.
I think the above functions are always invoked before getting the batch_size.

Only in EXPLAIN mode, it will not initialize the ri_FdwState.

/*
* Do nothing in EXPLAIN (no ANALYZE) case. resultRelInfo->ri_FdwState
* stays NULL.
*/
if (eflags & EXEC_FLAG_EXPLAIN_ONLY)
return;

Best regards,
houzj