From: | Zhihong Yu <zyu(at)yugabyte(dot)com> |
---|---|
To: | "tsunakawa(dot)takay(at)fujitsu(dot)com" <tsunakawa(dot)takay(at)fujitsu(dot)com> |
Cc: | Tomas Vondra <tomas(dot)vondra(at)enterprisedb(dot)com>, Amit Langote <amitlangote09(at)gmail(dot)com>, "Andrey V(dot) Lepikhov" <a(dot)lepikhov(at)postgrespro(dot)ru>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: POC: postgres_fdw insert batching |
Date: | 2021-01-18 15:27:54 |
Message-ID: | CALNJ-vRJOC5Sbt7LwFbyP4d9bPJurEyrxTz01EaxypdniG8PiA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
Hi, Takayuki-san:
+ if (batch_size <= 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("%s requires a non-negative integer value",
It seems the message doesn't match the check w.r.t. the batch size of 0.
+ int numInserted = numSlots;
Since numInserted is filled by ExecForeignBatchInsert(), the initialization
can be done with 0.
Cheers
On Sun, Jan 17, 2021 at 10:52 PM tsunakawa(dot)takay(at)fujitsu(dot)com <
tsunakawa(dot)takay(at)fujitsu(dot)com> wrote:
> Tomas-san,
>
> From: Amit Langote <amitlangote09(at)gmail(dot)com>
> > Good thing you reminded me that this is about inserts, and in that
> > case no, ExecInitModifyTable() doesn't know all leaf partitions, it
> > only sees the root table whose batch_size doesn't really matter. So
> > it's really ExecInitRoutingInfo() that I would recommend to set
> > ri_BatchSize; right after this block:
> >
> > /*
> > * If the partition is a foreign table, let the FDW init itself for
> > * routing tuples to the partition.
> > */
> > if (partRelInfo->ri_FdwRoutine != NULL &&
> > partRelInfo->ri_FdwRoutine->BeginForeignInsert != NULL)
> > partRelInfo->ri_FdwRoutine->BeginForeignInsert(mtstate, partRelInfo);
> >
> > Note that ExecInitRoutingInfo() is called only once for a partition
> > when it is initialized after being inserted into for the first time.
> >
> > For a non-partitioned targets, I'd still say set ri_BatchSize in
> > ExecInitModifyTable().
>
> Attached is the patch that added call to GetModifyBatchSize() to the above
> two places. The regression test passes.
>
> (FWIW, frankly, I prefer the previous version because the code is a bit
> smaller... Maybe we should refactor the code someday to reduce similar
> processings in both the partitioned case and non-partitioned case.)
>
>
> Regards
> Takayuki Tsunakawa
>
>
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2021-01-18 15:34:10 | Re: popcount |
Previous Message | Alvaro Herrera | 2021-01-18 15:21:31 | Re: support for MERGE |