On Thu, Feb 18, 2021 at 4:36 AM Tomas Vondra
wrote:
> On 2/17/21 9:51 AM, Amit Langote wrote:
> > On Wed, Feb 17, 2021 at 5:46 PM Amit Langote
> > wrote:
> >> Sorry, I hadn't shared enough details of my investigations when I
> >> originally ran into this. Such as that I had considered
On Thu, Feb 18, 2021 at 8:38 AM Tomas Vondra
wrote:
> On 2/17/21 8:36 PM, Tomas Vondra wrote:
> > Thanks. The patch seems reasonable, but it's a bit too large for a fix,
> > so I'll go ahead and push one of the previous fixes restricting batching
> > to plain INSERT commands. But this seems
On 2/17/21 8:36 PM, Tomas Vondra wrote:
>
> ...
>
> Thanks. The patch seems reasonable, but it's a bit too large for a fix,
> so I'll go ahead and push one of the previous fixes restricting batching
> to plain INSERT commands. But this seems useful, so I suggest adding it
> to the next
On 2/17/21 9:51 AM, Amit Langote wrote:
> On Wed, Feb 17, 2021 at 5:46 PM Amit Langote wrote:
>> On Wed, Feb 17, 2021 at 12:04 AM Tomas Vondra
>> wrote:
>>> On 2/16/21 10:25 AM, Amit Langote wrote:
On Tue, Feb 16, 2021 at 1:36 AM Tomas Vondra
> Thanks for the patch, it seems fine to me.
On Wed, Feb 17, 2021 at 5:46 PM Amit Langote wrote:
> On Wed, Feb 17, 2021 at 12:04 AM Tomas Vondra
> wrote:
> > On 2/16/21 10:25 AM, Amit Langote wrote:
> > > On Tue, Feb 16, 2021 at 1:36 AM Tomas Vondra
> > >> Thanks for the patch, it seems fine to me.
> > >
> > > Thanks for checking.
> > >
>
On 2/16/21 10:25 AM, Amit Langote wrote:
On Tue, Feb 16, 2021 at 1:36 AM Tomas Vondra
wrote:
On 2/5/21 3:52 AM, Amit Langote wrote:
Tsunakwa-san,
On Mon, Jan 25, 2021 at 1:21 PM tsunakawa.ta...@fujitsu.com
wrote:
From: Amit Langote
Yes, it can be simplified by using a local join to
On Tue, Feb 16, 2021 at 1:36 AM Tomas Vondra
wrote:
> On 2/5/21 3:52 AM, Amit Langote wrote:
> > Tsunakwa-san,
> >
> > On Mon, Jan 25, 2021 at 1:21 PM tsunakawa.ta...@fujitsu.com
> > wrote:
> >> From: Amit Langote
> >>> Yes, it can be simplified by using a local join to prevent the update of
>
On 2/5/21 3:52 AM, Amit Langote wrote:
> Tsunakwa-san,
>
> On Mon, Jan 25, 2021 at 1:21 PM tsunakawa.ta...@fujitsu.com
> wrote:
>> From: Amit Langote
>>> Yes, it can be simplified by using a local join to prevent the update of
>>> the foreign
>>> partition from being pushed to the remote
On 2/5/21 2:55 AM, Ian Lawrence Barwick wrote:
> ...
>
> There's a minor typo in the doc's version of the
> ExecForeignBatchInsert() declaration;
> is:
>
> TupleTableSlot **
> ExecForeignBatchInsert(EState *estate,
> ResultRelInfo *rinfo,
>
From: Amit Langote
> It appears I had missed your reply, sorry.
>
> > + PgFdwModifyState *fmstate = resultRelInfo->ri_FdwState ?
> > +
> (PgFdwModifyState *) resultRelInfo->ri_FdwState :
> > + NULL;
> >
> > This can be written as:
> >
>
Tsunakwa-san,
On Mon, Jan 25, 2021 at 1:21 PM tsunakawa.ta...@fujitsu.com
wrote:
> From: Amit Langote
> > Yes, it can be simplified by using a local join to prevent the update of
> > the foreign
> > partition from being pushed to the remote server, for which my example in
> > the
> > previous
2021年1月22日(金) 14:50 Ian Lawrence Barwick :
> Hi
>
> 2021年1月21日(木) 8:00 Tomas Vondra :
>
>> OK, pushed after a little bit of additional polishing (mostly comments).
>>
>> Thanks everyone!
>>
>
> There's a minor typo in the doc's version of the ExecForeignBatchInsert()
> declaration;
> is:
>
>
2021年1月22日(金) 14:50 Ian Lawrence Barwick :
> Hi
>
> 2021年1月21日(木) 8:00 Tomas Vondra :
>
>> OK, pushed after a little bit of additional polishing (mostly comments).
>>
>> Thanks everyone!
>>
>
> There's a minor typo in the doc's version of the ExecForeignBatchInsert()
> declaration;
> is:
>
>
From: Amit Langote
> Yes, it can be simplified by using a local join to prevent the update of the
> foreign
> partition from being pushed to the remote server, for which my example in the
> previous email used a local trigger. Note that the update of the foreign
> partition to be done locally
On Sun, Jan 24, 2021 at 2:17 AM Tomas Vondra
wrote:
> On 1/23/21 9:31 AM, Amit Langote wrote:
> > I was looking at this and it looks like we've got a problematic case
> > where postgresGetForeignModifyBatchSize() is called from
> > ExecInitRoutingInfo().
> >
> > That case is when the insert is
On 1/23/21 9:31 AM, Amit Langote wrote:
On Thu, Jan 21, 2021 at 11:36 AM Tomas Vondra
wrote:
On 1/21/21 3:09 AM, tsunakawa.ta...@fujitsu.com wrote:
From: Tomas Vondra
Right, that's pretty much what I ended up doing (without the CMD_INSERT
check it'd add batching info to explain for
Amit:
Good catch.
bq. ExecInitRoutingInfo() that is in the charge of initialing
Should be 'ExecInitRoutingInfo() that is in charge of initializing'
Cheers
On Sat, Jan 23, 2021 at 12:31 AM Amit Langote
wrote:
> On Thu, Jan 21, 2021 at 11:36 AM Tomas Vondra
> wrote:
> > On 1/21/21 3:09 AM,
On Thu, Jan 21, 2021 at 11:36 AM Tomas Vondra
wrote:
> On 1/21/21 3:09 AM, tsunakawa.ta...@fujitsu.com wrote:
> > From: Tomas Vondra
> >> Right, that's pretty much what I ended up doing (without the CMD_INSERT
> >> check it'd add batching info to explain for updates too, for example).
> >> I'll
Hi
2021年1月21日(木) 8:00 Tomas Vondra :
> OK, pushed after a little bit of additional polishing (mostly comments).
>
> Thanks everyone!
>
There's a minor typo in the doc's version of the ExecForeignBatchInsert()
declaration;
is:
TupleTableSlot **
ExecForeignBatchInsert(EState *estate,
On 1/21/21 3:09 AM, tsunakawa.ta...@fujitsu.com wrote:
From: Tomas Vondra
Right, that's pretty much what I ended up doing (without the CMD_INSERT
check it'd add batching info to explain for updates too, for example).
I'll do a bit more testing on the attached patch, but I think that's the
From: Tom Lane
> The "single target table" could be partitioned, in which case there'll be
> multiple resultrelinfos, some of which could be foreign tables.
Thank you. I thought so at first, but later I found that ExecInsert() only
handles one element in mtstate->resultRelInfo. So I thought
From: Tomas Vondra
> Right, that's pretty much what I ended up doing (without the CMD_INSERT
> check it'd add batching info to explain for updates too, for example).
> I'll do a bit more testing on the attached patch, but I think that's the
> right fix to
> push.
Thanks to the outer check for
"tsunakawa.ta...@fujitsu.com" writes:
> Just for learning, could anyone tell me what this loop for? I thought
> current Postgres's DML supports a single target table, so it's enough to
> handle the first element of mtstate->resultRelInfo.
The "single target table" could be partitioned, in
On 1/21/21 2:53 AM, Amit Langote wrote:
On Thu, Jan 21, 2021 at 10:42 AM Tomas Vondra
wrote:
On 1/21/21 2:24 AM, Amit Langote wrote:
On Thu, Jan 21, 2021 at 9:56 AM Tomas Vondra
wrote:
On 1/21/21 1:17 AM, Zhihong Yu wrote:
Hi,
The assignment to resultRelInfo is done when
From: Zhihong Yu
> My first name is Zhihong.
> You can call me Ted if you want to save some typing :-)
Ah, I'm very sorry. Thank you, let me call you Ted then. That can't be
mistaken.
Regards
Takayuki Tsunakawa
On Thu, Jan 21, 2021 at 10:42 AM Tomas Vondra
wrote:
> On 1/21/21 2:24 AM, Amit Langote wrote:
> > On Thu, Jan 21, 2021 at 9:56 AM Tomas Vondra
> > wrote:
> >> On 1/21/21 1:17 AM, Zhihong Yu wrote:
> >>> Hi,
> >>> The assignment to resultRelInfo is done when junk_filter_needed is true:
> >>>
>
From: Tomas Vondra
> Right. But I think Tom is right this should initialize ri_BatchSize for all
> the
> resultRelInfo elements, not just the first one. Per the attached patch, which
> resolves the issue both on x86_64 and armv7l for me.
I think Your patch is perfect in the sense that it's
Hi, Takayuki-san:
My first name is Zhihong.
You can call me Ted if you want to save some typing :-)
Cheers
On Wed, Jan 20, 2021 at 5:37 PM tsunakawa.ta...@fujitsu.com <
tsunakawa.ta...@fujitsu.com> wrote:
> From: Zhihong Yu
>
> > Do we need to consider how this part of code inside
>
On 1/21/21 2:22 AM, Tom Lane wrote:
Tomas Vondra writes:
I may be wrong, but the most likely explanation seems to be this is due
to the junk filter initialization, which simply moves past the end of
the mtstate->resultRelInfo array.
resultRelInfo is certainly pointing at garbage at that
On 1/21/21 2:24 AM, Amit Langote wrote:
On Thu, Jan 21, 2021 at 9:56 AM Tomas Vondra
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 =
From: Zhihong Yu
> Do we need to consider how this part of code inside ExecInitModifyTable()
> would evolve ?
> I think placing the compound condition toward the end of
> ExecInitModifyTable() is reasonable because it checks the latest information.
+1 for Zaihong-san's idea. But instead of
On Thu, Jan 21, 2021 at 9:56 AM Tomas Vondra
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;
> >
> >
Tomas Vondra writes:
> I may be wrong, but the most likely explanation seems to be this is due
> to the junk filter initialization, which simply moves past the end of
> the mtstate->resultRelInfo array.
resultRelInfo is certainly pointing at garbage at that point.
> It kinda seems the
Hi,
Do we need to consider how this part of code inside ExecInitModifyTable()
would evolve ?
I think placing the compound condition toward the end
of ExecInitModifyTable() is reasonable because it checks the latest
information.
Regards
On Wed, Jan 20, 2021 at 5:11 PM Tomas Vondra
wrote:
>
>
>
On 1/21/21 2:02 AM, Zhihong Yu wrote:
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
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
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
On 1/21/21 12:59 AM, Tom Lane wrote:
Tomas Vondra writes:
OK, pushed after a little bit of additional polishing (mostly comments).
Thanks everyone!
florican reports this is seriously broken on 32-bit hardware:
On 1/21/21 12:52 AM, Tomas Vondra wrote:
Hmm, seems that florican doesn't like this :-(
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=florican=2021-01-20%2023%3A08%3A15
It's a i386 machine running FreeBSD, so not sure what exactly it's picky
about. But when I tried running this
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 ?
diff --git
Tomas Vondra writes:
> OK, pushed after a little bit of additional polishing (mostly comments).
> Thanks everyone!
florican reports this is seriously broken on 32-bit hardware:
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=florican=2021-01-20%2023%3A08%3A15
First guess is incorrect
Hmm, seems that florican doesn't like this :-(
https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=florican=2021-01-20%2023%3A08%3A15
It's a i386 machine running FreeBSD, so not sure what exactly it's picky
about. But when I tried running this under valgrind, I get some strange
failures
OK, pushed after a little bit of additional polishing (mostly comments).
Thanks everyone!
--
Tomas Vondra
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
On Wed, Jan 20, 2021 at 1:01 AM Tomas Vondra
wrote:
> On 1/19/21 7:23 AM, Amit Langote wrote:
> > On Tue, Jan 19, 2021 at 2:06 PM tsunakawa.ta...@fujitsu.com
> >> Actually, I tried to do it (adding the GetModifyBatchSize() call after
> >> BeginForeignModify()), but it failed. Because
> >>
On 1/19/21 7:23 AM, Amit Langote wrote:
On Tue, Jan 19, 2021 at 2:06 PM tsunakawa.ta...@fujitsu.com
wrote:
From: Amit Langote
I apologize in advance for being maybe overly pedantic, but I noticed
that, in ExecInitModifyTable(), you decided to place the call outside
the loop that goes over
On Tue, Jan 19, 2021 at 2:06 PM tsunakawa.ta...@fujitsu.com
wrote:
> From: Amit Langote
> > I apologize in advance for being maybe overly pedantic, but I noticed
> > that, in ExecInitModifyTable(), you decided to place the call outside
> > the loop that goes over resultRelations (shown below),
From: Amit Langote
> I apologize in advance for being maybe overly pedantic, but I noticed
> that, in ExecInitModifyTable(), you decided to place the call outside
> the loop that goes over resultRelations (shown below), although my
> intent was to ask to place it next to the BeginForeignModify()
Tsunakawa-san,
On Tue, Jan 19, 2021 at 12:50 PM tsunakawa.ta...@fujitsu.com
wrote:
> From: Tomas Vondra
> > OK. Can you prepare a final patch, squashing all the commits into a
> > single one, and perhaps use the function in create_foreign_modify?
>
> Attached, including the message fix pointed
From: Tomas Vondra
> OK. Can you prepare a final patch, squashing all the commits into a
> single one, and perhaps use the function in create_foreign_modify?
Attached, including the message fix pointed by Zaihong-san.
Regards
Takayuki Tsunakawa
On 1/19/21 2:28 AM, tsunakawa.ta...@fujitsu.com wrote:
From: Tomas Vondra
I took a look at this - there's a bit of bitrot due to
708d165ddb92c, so attached is a rebased patch (0001) fixing that.
0002 adds a couple comments and minor tweaks
0003 addresses a couple shortcomings related to
Tomas-san, Zhihong-san,
From: Zhihong Yu
> + 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
From: Tomas Vondra
> I took a look at this - there's a bit of bitrot due to 708d165ddb92c, so
> attached is
> a rebased patch (0001) fixing that.
>
> 0002 adds a couple comments and minor tweaks
>
> 0003 addresses a couple shortcomings related to explain - we haven't been
> showing the batch
On 1/18/21 7:51 AM, tsunakawa.ta...@fujitsu.com wrote:
Tomas-san,
From: Amit Langote
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
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
Tomas-san,
From: Amit Langote
> 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
On Sat, Jan 16, 2021 at 12:00 AM tsunakawa.ta...@fujitsu.com
wrote:
> From: Amit Langote
> > Okay, so maybe not moving the whole logic into the FDW's
> > BeginForeignModify(), but at least if we move this...
> >
> > @@ -441,6 +449,72 @@ ExecInsert(ModifyTableState *mtstate,
> > + /*
> > +
From: Amit Langote
> Okay, so maybe not moving the whole logic into the FDW's
> BeginForeignModify(), but at least if we move this...
>
> @@ -441,6 +449,72 @@ ExecInsert(ModifyTableState *mtstate,
> + /*
> +* Determine if the FDW supports batch insert and determine the
> batch
> +
On Fri, Jan 15, 2021 at 12:05 AM Tomas Vondra
wrote:
> Attached is v9 with all of those tweaks,
Thanks.
> except for moving the BatchSize
> call to BeginForeignModify - I tried that, but it did not seem like an
> improvement, because we'd still need the checks for API callbacks in
> ExecInsert
From: Tomas Vondra
> Attached is v9 with all of those tweaks, except for moving the BatchSize call
> to
> BeginForeignModify - I tried that, but it did not seem like an improvement,
> because we'd still need the checks for API callbacks in ExecInsert for
> example.
> So I decided not to do
On 1/14/21 2:57 PM, Amit Langote wrote:
> On Thu, Jan 14, 2021 at 21:57 Tomas Vondra
> mailto:tomas.von...@enterprisedb.com>>
> wrote:
>
> On 1/14/21 9:58 AM, Amit Langote wrote:
> > Hi,
> >
> > On Thu, Jan 14, 2021 at 2:41 AM Tomas Vondra
> >
On Thu, Jan 14, 2021 at 21:57 Tomas Vondra
wrote:
> On 1/14/21 9:58 AM, Amit Langote wrote:
> > Hi,
> >
> > On Thu, Jan 14, 2021 at 2:41 AM Tomas Vondra
> > wrote:
> >> On 1/13/21 3:43 PM, Tomas Vondra wrote:
> >>> Thanks for the report. Yeah, I think there's a missing check in
> >>>
On 1/14/21 9:58 AM, Amit Langote wrote:
> Hi,
>
> On Thu, Jan 14, 2021 at 2:41 AM Tomas Vondra
> wrote:
>> 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
>>>
>>>
Hi,
On Thu, Jan 14, 2021 at 2:41 AM Tomas Vondra
wrote:
> 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
From: Tomas Vondra
> 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
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,
On 1/13/21 10:15 AM, Amit Langote wrote:
> Hi Tomas, Tsunakawa-san,
>
> Thanks for your work on this.
>
> On Tue, Jan 12, 2021 at 11:06 AM Tomas Vondra
> wrote:
>> AFAICS the discussions about making this use COPY and/or libpq
>> pipelining (neither of which is committed yet) ended with the
Hi Tomas, Tsunakawa-san,
Thanks for your work on this.
On Tue, Jan 12, 2021 at 11:06 AM Tomas Vondra
wrote:
> AFAICS the discussions about making this use COPY and/or libpq
> pipelining (neither of which is committed yet) ended with the conclusion
> that those changes are somewhat independent,
From: Tomas Vondra
> Attached is a v6 of this patch, rebased to current master and with some minor
> improvements (mostly comments and renaming the "end" struct field to
> "values_end" which I think is more descriptive).
Thank you very much. In fact, my initial patches used values_end, and I
Hi,
Attached is a v6 of this patch, rebased to current master and with some
minor improvements (mostly comments and renaming the "end" struct field
to "values_end" which I think is more descriptive).
The one thing that keeps bugging me is convert_prep_stmt_params - it
dies the right thing,
From: David Fetter
> Please pardon me for barging in late in this discussion, but if we're
> going to be using a bulk API here, wouldn't it make more sense to use
> COPY, except where RETURNING is specified, in place of INSERT?
Please do not hesitate. I mentioned earlier in this thread that I
From: Craig Ringer
> It was not my intention to hold this patch up or greatly expand its
> scope. I'll spend some time testing it out and have a closer read soon
> to see if I can help progress it.
Thank you, I'm relieved to hear that. Last weekend, I was scared of a possible
mood that's
On Wed, Nov 25, 2020 at 05:04:36AM +, tsunakawa.ta...@fujitsu.com wrote:
> From: Tomas Vondra
> > On 11/24/20 9:45 AM, tsunakawa.ta...@fujitsu.com wrote:
> > > OTOH, as for the name GetModifyBatchSize() you suggest, I think
> > GetInsertBatchSize may be better. That is, this API deals with
On Fri, 27 Nov 2020, 14:06 tsunakawa.ta...@fujitsu.com,
wrote:
>
>
Also, I'm afraid it requires major surgery or reform of executor. I
don't want it to delay the release of reasonably good (10x)
improvement with the synchronous interface.)
Totally sensible. If it isn't feasible without
On Sat, 28 Nov 2020, 10:10 Tomas Vondra,
wrote:
>
>
> On 11/27/20 7:05 AM, tsunakawa.ta...@fujitsu.com wrote:
>
> However, the FDW interface as it's implemented today is not designed to
> allow that, I believe (we pretty much just invoke the FWD callbacks as
> if it was a local AM). It assumes
On 11/27/20 7:05 AM, tsunakawa.ta...@fujitsu.com wrote:
> From: Craig Ringer
>> But in the libpq pipelining patch I demonstrated a 300 times
>> (3000%) performance improvement on a test workload...
>
> Wow, impressive number. I've just seen it in the beginning of the
> libpq pipelining
From: Craig Ringer
> But in the libpq pipelining patch I demonstrated a 300 times (3000%)
> performance improvement on a test workload...
Wow, impressive number. I've just seen it in the beginning of the libpq
pipelining thread (oh, already four years ago..!) Could you share the workload
On Fri, Nov 27, 2020 at 10:47 AM tsunakawa.ta...@fujitsu.com <
tsunakawa.ta...@fujitsu.com> wrote:
Covering this one first:
I expect postgresExecForeignBatchInsert() would be able to use the libpq
> batching API, because it receives an array of tuples and can generate and
> issue INSERT
From: Tomas Vondra
> Not sure how is this related to app developers? I think the idea was
> that the libpq features might be useful between the two PostgreSQL
> instances. I.e. the postgres_fdw would use the libpq batching to send
> chunks of data to the other side.
> Well, my point was that we
On Fri, Nov 27, 2020 at 3:34 AM Tomas Vondra
wrote:
> Not sure how is this related to app developers? I think the idea was
> that the libpq features might be useful between the two PostgreSQL
> instances. I.e. the postgres_fdw would use the libpq batching to send
> chunks of data to the other
On 11/26/20 2:48 AM, tsunakawa.ta...@fujitsu.com wrote:
> From: Tomas Vondra
>> Well, good that we all agree this is a useful feature to have (in
>> general). The question is whether postgres_fdw should be doing
>> batching on it's onw (per this thread) or rely on some other
>> feature
From: Tomas Vondra
> Well, good that we all agree this is a useful feature to have (in
> general). The question is whether postgres_fdw should be doing batching
> on it's onw (per this thread) or rely on some other feature (libpq
> pipelining). I haven't followed the other thread, so I don't have
On 11/25/20 7:31 AM, tsunakawa.ta...@fujitsu.com wrote:
> From: Craig Ringer
>> I suggest that when developing this, you keep in mind the ongoing
>> work on the libpq pipelining/batching enhancements, and also the
>> way many interfaces to foreign data sources support asynchronous,
>> concurrent
From: Craig Ringer
> I suggest that when developing this, you keep in mind the ongoing work on the
> libpq pipelining/batching enhancements, and also the way many interfaces to
> foreign data sources support asynchronous, concurrent operations.
Yes, thank you, I bear it in mind. I understand
From: Tomas Vondra
> On 11/24/20 9:45 AM, tsunakawa.ta...@fujitsu.com wrote:
> > OTOH, as for the name GetModifyBatchSize() you suggest, I think
> GetInsertBatchSize may be better. That is, this API deals with multiple
> records in a single INSERT statement. Your GetModifyBatchSize will be
>
On Thu, Oct 8, 2020 at 10:40 AM tsunakawa.ta...@fujitsu.com <
tsunakawa.ta...@fujitsu.com> wrote:
>
> Thank you for picking up this. I'm interested in this topic, too. (As an
> aside, we'd like to submit a bulk insert patch for ECPG in the near future.)
>
> As others referred, Andrey-san's fast
On 11/24/20 9:45 AM, tsunakawa.ta...@fujitsu.com wrote:
> From: Tomas Vondra
>> 1) We're calling it "batch_size" but the API function is named
>> postgresGetMaxBulkInsertTuples(). Perhaps we should rename the function
>> to postgresGetModifyBatchSize()? That has the advantage it'd work if we
From: Tomas Vondra
> 1) We're calling it "batch_size" but the API function is named
> postgresGetMaxBulkInsertTuples(). Perhaps we should rename the function
> to postgresGetModifyBatchSize()? That has the advantage it'd work if we
> ever add support for batching to UPDATE/DELETE.
Actually, I
On 11/23/20 3:17 AM, tsunakawa.ta...@fujitsu.com wrote:
> From: Tomas Vondra
>> I don't think this is usable in practice, because a single session
>> may be using multiple FDW servers, with different implementations,
>> latency to the data nodes, etc. It's unlikely a single GUC value
>> will be
From: Tomas Vondra
> I don't think this is usable in practice, because a single session may
> be using multiple FDW servers, with different implementations, latency
> to the data nodes, etc. It's unlikely a single GUC value will be
> suitable for all of them.
That makes sense. The row size
On 11/19/20 3:43 AM, tsunakawa.ta...@fujitsu.com wrote:
> From: Tomas Vondra
>> Unfortunately, this does not compile for me, because
>> nodeModifyTable calls ExecGetTouchedPartitions, which is not
>> defined anywhere. Not sure what's that about, so I simply
>> commented-out this. That probably
From: Tomas Vondra
> Unfortunately, this does not compile for me, because nodeModifyTable calls
> ExecGetTouchedPartitions, which is not defined anywhere. Not sure what's
> that about, so I simply commented-out this. That probably fails the
> partitioned
> cases, but it allowed me to do some
On 11/17/20 10:11 AM, tsunakawa.ta...@fujitsu.com wrote:
> Hello,
>
>
> Modified the patch as I talked with Tomas-san. The performance
> results of loading one million records into a hash-partitioned table
> with 8 partitions are as follows:
>
> unpatched, local: 8.6 seconds unpatched, fdw:
Hello,
Modified the patch as I talked with Tomas-san. The performance results of
loading one million records into a hash-partitioned table with 8 partitions are
as follows:
unpatched, local: 8.6 seconds
unpatched, fdw: 113.7 seconds
patched, fdw: 12.5 seconds (9x
From: t...@corona.is.ed.ac.uk On Behalf Of
> Does this patch affect trigger semantics on the base table?
>
> At the moment when I insert 1000 rows into a postgres_fdw table using a
> single insert statement (e.g. INSERT INTO fdw_foo SELECT ... FROM bar) I
> naively expect a "statement level"
On Wed, 11 Nov 2020, tsunakawa.ta...@fujitsu.com wrote:
This email was sent to you by someone outside of the University.
You should only click on links or attachments if you are certain that the email
is genuine and the content is safe.
From: Tomas Vondra
I see the patch builds the "bulk"
From: Tomas Vondra
> I see the patch builds the "bulk" query in execute_foreign_modify. IMO
> that's something we should do earlier, when we're building the simple
> query (for 1-row inserts). I'd understand if you were concerned about
> overhead in case of 1-row inserts, trying to not plan the
On 11/10/20 4:05 PM, Tomas Vondra wrote:
> Hi,
>
> Thanks for working on this!
>
> On 11/10/20 1:45 AM, tsunakawa.ta...@fujitsu.com wrote:
>> Hello,
>>
>>
>> The attached patch implements the new bulk insert routine for
>> postgres_fdw and the executor utilizing it. It passes make
>>
Hi,
Thanks for working on this!
On 11/10/20 1:45 AM, tsunakawa.ta...@fujitsu.com wrote:
> Hello,
>
>
> The attached patch implements the new bulk insert routine for
> postgres_fdw and the executor utilizing it. It passes make
> check-world.
>
I haven't done any testing yet, just a quick
Hello,
The attached patch implements the new bulk insert routine for postgres_fdw and
the executor utilizing it. It passes make check-world.
I measured performance in a basic non-partitioned case by modifying Tomas-san's
scripts. They perform an INSERT SELECT statement that copies one
From: Tomas Vondra
> I'm not sure when I'll have time to work on this again, so if you are
> interested and willing to work on it, please go ahead. I'll gladly do
> reviews and help you with it.
Thank you very much.
> I think transferring data to other databases is fine - interoperability
> is
1 - 100 of 115 matches
Mail list logo
