public inbox for [email protected]  
help / color / mirror / Atom feed
From: [email protected] <[email protected]>
To: Bharath Rupireddy <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Tomas Vondra <[email protected]>
Subject: RE: Fdw batch insert error out when set batch_size > 65535
Date: Thu, 27 May 2021 03:12:16 +0000
Message-ID: <OS0PR01MB571674E988B480E82066EA7794239@OS0PR01MB5716.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CALj2ACXq8a=FdrPDa=_wMrvOwcZjrNfCdeyfG-2dt17LyBEc=g@mail.gmail.com>
References: <OS0PR01MB571603973C0AC2874AD6BF2594299@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<CALj2ACXg3tPAChzFd94_k_tP3x61QTVpuqKZke3M4ef78xEcuA@mail.gmail.com>
	<OS0PR01MB5716BFBD760428859058FCBD94299@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<CALj2ACV=qgpccTEip7NHJWDe3Set8wXwDmNnRzy=GYmx9x94XQ@mail.gmail.com>
	<OS0PR01MB57160ADFD5010254C8D1CE6594259@OS0PR01MB5716.jpnprd01.prod.outlook.com>
	<CALj2ACVvrd6LGga6UzOu17GzW0cmaS-F30KMBSxKsA+Zw8Z=BQ@mail.gmail.com>
	<CALj2ACXBJ3a7MLbCAGA=5dneYsD720KPp1Pot-kKVM1YFPkESA@mail.gmail.com>
	<[email protected]>
	<CALj2ACXq8a=FdrPDa=_wMrvOwcZjrNfCdeyfG-2dt17LyBEc=g@mail.gmail.com>

From: Bharath Rupireddy <[email protected]>
Sent: Wednesday, May 26, 2021 9:56 PM
> On Wed, May 26, 2021 at 6:36 PM Tomas Vondra
> <[email protected]> wrote:
> >
> > On 5/26/21 8:57 AM, Bharath Rupireddy wrote:
> > > On Tue, May 25, 2021 at 2:47 PM Bharath Rupireddy
> > > <[email protected]> wrote:
> > >>
> > >> On Tue, May 25, 2021 at 1:08 PM [email protected]
> > >> <[email protected]> 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
 



view thread (29+ messages)  latest in thread

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: [email protected]
  Cc: [email protected], [email protected], [email protected], [email protected]
  Subject: RE: Fdw batch insert error out when set batch_size > 65535
  In-Reply-To: <OS0PR01MB571674E988B480E82066EA7794239@OS0PR01MB5716.jpnprd01.prod.outlook.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox