Re: POC: postgres_fdw insert batching

Zhihong Yu <zyu@yugabyte.com>

From: Zhihong Yu <zyu@yugabyte.com>
To: Tomas Vondra <tomas.vondra@enterprisedb.com>
Cc: Amit Langote <amitlangote09@gmail.com>, "tsunakawa.takay@fujitsu.com" <tsunakawa.takay@fujitsu.com>, "Andrey V. Lepikhov" <a.lepikhov@postgrespro.ru>, Tomas Vondra <tomas.vondra@2ndquadrant.com>, "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Date: 2021-01-21T01:02:56Z
Lists: pgsql-hackers
Hi, Tomas:
In my opinion, my patch is a little better.
Suppose one of the conditions in the if block changes in between the start
of loop and the end of the loop:

     * Determine if the FDW supports batch insert and determine the batch
     * size (a FDW may support batching, but it may be disabled for the
     * server/table).

My patch would reflect that change. I guess this was the reason the if /
else block was placed there in the first place.

Cheers

On Wed, Jan 20, 2021 at 4:56 PM Tomas Vondra <tomas.vondra@enterprisedb.com>
wrote:

>
>
> On 1/21/21 1:17 AM, Zhihong Yu wrote:
> > Hi,
> > The assignment to resultRelInfo is done when junk_filter_needed is true:
> >
> >          if (junk_filter_needed)
> >          {
> >              resultRelInfo = mtstate->resultRelInfo;
> >
> > Should the code for determining batch size access mtstate->resultRelInfo
> > directly ?
> >
>
> IMO the issue is that code iterates over all plans and moves to the next
> for each one:
>
>      resultRelInfo++;
>
> so it ends up pointing past the last element, hence the failures. So
> yeah, either the code needs to move before the loop (per my patch), or
> we need to access mtstate->resultRelInfo directly.
>
> I'm pretty amazed this did not crash during any of the many regression
> runs I did recently.
>
> regards
>
> --
> Tomas Vondra
> EnterpriseDB: http://www.enterprisedb.com
> The Enterprise PostgreSQL Company
>

Commits

  1. Fix tuple routing to initialize batching only for inserts

  2. Fix pointer type in ExecForeignBatchInsert SGML docs

  3. Fix initialization of FDW batching in ExecInitModifyTable

  4. Implement support for bulk inserts in postgres_fdw