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

2023-07-03 Thread Tomas Vondra
On 7/2/23 15:50, Tomas Vondra wrote: > On 7/2/23 15:23, Tom Lane wrote: >> Andres Freund writes: >>> On 2021-06-11 18:44:28 -0400, Tom Lane wrote: I suggest what we do is leave it in place for long enough to get a round of reports from those slow animals, and then (assuming those

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

2023-07-02 Thread Tomas Vondra
On 7/2/23 15:23, Tom Lane wrote: > Andres Freund writes: >> On 2021-06-11 18:44:28 -0400, Tom Lane wrote: >>> I suggest what we do is leave it in place for long enough to get >>> a round of reports from those slow animals, and then (assuming >>> those reports are positive) drop the test. > >> I

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

2023-07-02 Thread Tom Lane
Andres Freund writes: > On 2021-06-11 18:44:28 -0400, Tom Lane wrote: >> I suggest what we do is leave it in place for long enough to get >> a round of reports from those slow animals, and then (assuming >> those reports are positive) drop the test. > I think two years later is long enough to

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

2023-07-02 Thread Andres Freund
Hi, On 2021-06-11 18:44:28 -0400, Tom Lane wrote: > Tomas Vondra writes: > > There's one caveat, though - for regular builds the slowdown is pretty > > much eliminated. But with valgrind it's still considerably slower. For > > postgres_fdw the "make check" used to take ~5 minutes for me, now it

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

2021-06-14 Thread Bharath Rupireddy
On Mon, Jun 14, 2021, 5:33 PM Bharath Rupireddy < bharath.rupireddyforpostg...@gmail.com> wrote: > Okay. Here are the readings on my dev system: > 1) on master with the existing test case with inserting 70K rows: > 4263200 ms (71.05 min) > 2) with Tomas's patch with the test case modified with

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

2021-06-14 Thread Bharath Rupireddy
On Sun, Jun 13, 2021 at 9:28 PM Tomas Vondra wrote: > On 6/13/21 5:25 PM, Bharath Rupireddy wrote: > > On Sun, Jun 13, 2021 at 6:10 AM Alvaro Herrera > > wrote: > >> > >> On 2021-Jun-12, Tomas Vondra wrote: > >> > >>> There's one caveat, though - for regular builds the slowdown is pretty > >>>

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

2021-06-13 Thread Tomas Vondra
On 6/13/21 5:25 PM, Bharath Rupireddy wrote: > On Sun, Jun 13, 2021 at 6:10 AM Alvaro Herrera > wrote: >> >> On 2021-Jun-12, Tomas Vondra wrote: >> >>> There's one caveat, though - for regular builds the slowdown is pretty >>> much eliminated. But with valgrind it's still considerably slower.

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

2021-06-13 Thread Bharath Rupireddy
On Sun, Jun 13, 2021 at 6:10 AM Alvaro Herrera wrote: > > On 2021-Jun-12, Tomas Vondra wrote: > > > There's one caveat, though - for regular builds the slowdown is pretty > > much eliminated. But with valgrind it's still considerably slower. For > > postgres_fdw the "make check" used to take ~5

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

2021-06-13 Thread Tomas Vondra
On 6/13/21 2:40 AM, Alvaro Herrera wrote: > On 2021-Jun-12, Tomas Vondra wrote: > >> There's one caveat, though - for regular builds the slowdown is pretty >> much eliminated. But with valgrind it's still considerably slower. For >> postgres_fdw the "make check" used to take ~5 minutes for me,

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

2021-06-12 Thread Alvaro Herrera
On 2021-Jun-12, Tomas Vondra wrote: > There's one caveat, though - for regular builds the slowdown is pretty > much eliminated. But with valgrind it's still considerably slower. For > postgres_fdw the "make check" used to take ~5 minutes for me, now it > takes >1h. And yes, this is entirely due

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

2021-06-11 Thread Tom Lane
Tomas Vondra writes: > There's one caveat, though - for regular builds the slowdown is pretty > much eliminated. But with valgrind it's still considerably slower. For > postgres_fdw the "make check" used to take ~5 minutes for me, now it > takes >1h. And yes, this is entirely due to the new test

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

2021-06-11 Thread Tomas Vondra
On 6/9/21 4:05 PM, Tomas Vondra wrote: > On 6/9/21 3:28 PM, Tom Lane wrote: >> Tomas Vondra writes: >>> Note that the problem here is [1] - we're creating a lot of slots >>> referencing the same tuple descriptor, which inflates the duration. >>> There's a fix in the other thread, which

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

2021-06-09 Thread Tomas Vondra
On 6/9/21 3:28 PM, Tom Lane wrote: Tomas Vondra writes: Note that the problem here is [1] - we're creating a lot of slots referencing the same tuple descriptor, which inflates the duration. There's a fix in the other thread, which eliminates ~99% of the overhead. I plan to push that fix soon

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

2021-06-09 Thread Tom Lane
Tomas Vondra writes: > Note that the problem here is [1] - we're creating a lot of slots > referencing the same tuple descriptor, which inflates the duration. > There's a fix in the other thread, which eliminates ~99% of the > overhead. I plan to push that fix soon (a day or two). Oh, okay,

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

2021-06-09 Thread Tomas Vondra
On 6/9/21 12:22 PM, Tomas Vondra wrote: On 6/9/21 8:28 AM, Tom Lane wrote: I wrote: Bharath Rupireddy writes: I've added a simple regression test to postgres_fdw, testing that batch sizes > 65535 work fine, and pushed the fix. I was earlier thinking of adding one, but stopped

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

2021-06-09 Thread Tomas Vondra
On 6/9/21 8:28 AM, Tom Lane wrote: I wrote: Bharath Rupireddy writes: I've added a simple regression test to postgres_fdw, testing that batch sizes > 65535 work fine, and pushed the fix. I was earlier thinking of adding one, but stopped because it might increase the regression test

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

2021-06-09 Thread Tom Lane
I wrote: > Bharath Rupireddy writes: >>> I've added a simple regression test to postgres_fdw, testing that batch >>> sizes > 65535 work fine, and pushed the fix. >> I was earlier thinking of adding one, but stopped because it might >> increase the regression test execution time. It looks like

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

2021-06-09 Thread Tom Lane
Bharath Rupireddy writes: >> I've added a simple regression test to postgres_fdw, testing that batch >> sizes > 65535 work fine, and pushed the fix. > I was earlier thinking of adding one, but stopped because it might > increase the regression test execution time. It looks like that's true > -

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

2021-06-08 Thread Bharath Rupireddy
On Wed, Jun 9, 2021 at 12:04 AM Tomas Vondra wrote: > No, the "Int16" refers to the FE/BE documentation, where we use Int16. > But in the C code we interpret it as uint16. Hm. I see that in protocol.sgml Int16 is being used. > I've added a simple regression test to postgres_fdw, testing that

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

2021-06-08 Thread Tomas Vondra
On 5/31/21 6:01 AM, Bharath Rupireddy wrote: > On Mon, May 31, 2021 at 1:21 AM Tomas Vondra > wrote: >> >> Hi, >> >> I took at this patch today. I did some minor changes, mostly: >> >> 1) change the code limiting batch_size from >> >> if (fmstate->p_nums > 0 && >>(batch_size *

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

2021-05-30 Thread Bharath Rupireddy
On Mon, May 31, 2021 at 1:21 AM Tomas Vondra wrote: > > Hi, > > I took at this patch today. I did some minor changes, mostly: > > 1) change the code limiting batch_size from > > if (fmstate->p_nums > 0 && >(batch_size * fmstate->p_nums > PQ_QUERY_PARAM_MAX_LIMIT)) > { >

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

2021-05-30 Thread Tomas Vondra
Hi, I took at this patch today. I did some minor changes, mostly: 1) change the code limiting batch_size from if (fmstate->p_nums > 0 && (batch_size * fmstate->p_nums > PQ_QUERY_PARAM_MAX_LIMIT)) { batch_size = PQ_QUERY_PARAM_MAX_LIMIT / fmstate->p_nums; } to if

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

2021-05-26 Thread houzj.f...@fujitsu.com
From: Bharath Rupireddy Sent: Wednesday, May 26, 2021 9:56 PM > On Wed, May 26, 2021 at 6:36 PM Tomas Vondra > wrote: > > > > On 5/26/21 8:57 AM, Bharath Rupireddy wrote: > > > On Tue, May 25, 2021 at 2:47 PM Bharath Rupireddy > > > wrote: > > >> > > >> On Tue, May 25, 2021 at 1:08 PM

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

2021-05-26 Thread Bharath Rupireddy
On Wed, May 26, 2021 at 6:36 PM Tomas Vondra wrote: > > On 5/26/21 8:57 AM, Bharath Rupireddy wrote: > > On Tue, May 25, 2021 at 2:47 PM Bharath Rupireddy > > wrote: > >> > >> On Tue, May 25, 2021 at 1:08 PM houzj.f...@fujitsu.com > >> wrote: > >>> Thanks for the comments. I have addressed all

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

2021-05-26 Thread Tomas Vondra
On 5/26/21 8:57 AM, Bharath Rupireddy wrote: On Tue, May 25, 2021 at 2:47 PM Bharath Rupireddy wrote: On Tue, May 25, 2021 at 1:08 PM houzj.f...@fujitsu.com wrote: Thanks for the comments. I have addressed all comments to the v3 patch. Thanks! The patch basically looks good to me except

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

2021-05-26 Thread Bharath Rupireddy
On Tue, May 25, 2021 at 2:47 PM Bharath Rupireddy wrote: > > On Tue, May 25, 2021 at 1:08 PM houzj.f...@fujitsu.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.

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

2021-05-25 Thread Bharath Rupireddy
On Tue, May 25, 2021 at 1:08 PM houzj.f...@fujitsu.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. > BTW, Is the batch_size issue here an

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

2021-05-25 Thread houzj.f...@fujitsu.com
From: Bharath Rupireddy Sent: Friday, May 21, 2021 5:03 PM > On Fri, May 21, 2021 at 1:19 PM houzj.f...@fujitsu.com > wrote: > > Attaching V2 patch. Please consider it for further review. > > Thanks for the patch. Some more comments: > > 1) Can fmstate->p_nums ever be 0 in

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

2021-05-21 Thread Andrew Dunstan
On 5/21/21 5:03 AM, Bharath Rupireddy wrote: > On Fri, May 21, 2021 at 1:19 PM houzj.f...@fujitsu.com > wrote: >> Attaching V2 patch. Please consider it for further review. > Thanks for the patch. Some more comments: > > 1) Can fmstate->p_nums ever be 0 in postgresGetForeignModifyBatchSize? >

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

2021-05-21 Thread Bharath Rupireddy
On Fri, May 21, 2021 at 1:19 PM houzj.f...@fujitsu.com wrote: > Attaching V2 patch. Please consider it for further review. Thanks for the patch. Some more comments: 1) Can fmstate->p_nums ever be 0 in postgresGetForeignModifyBatchSize? By any chance, if it can, I think instead of an assert, we

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

2021-05-21 Thread houzj.f...@fujitsu.com
From: Bharath Rupireddy Sent: Friday, May 21, 2021 1:42 PM > On Fri, May 21, 2021 at 8:18 AM houzj.f...@fujitsu.com > wrote: > > Actually, I think if the (number of columns) * (number of rows) > > > 65535, then we can get this error. But, I think almost no one will set > > such a large value, so

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

2021-05-20 Thread Bharath Rupireddy
On Fri, May 21, 2021 at 8:18 AM houzj.f...@fujitsu.com wrote: > > Hi, > > When reading the code, I noticed some possible issue about fdw batch insert. > When I set batch_size > 65535 and insert more than 65535 rows into foreign > table, > It will throw an error: > > For example: > >

Fdw batch insert error out when set batch_size > 65535

2021-05-20 Thread houzj.f...@fujitsu.com
Hi, When reading the code, I noticed some possible issue about fdw batch insert. When I set batch_size > 65535 and insert more than 65535 rows into foreign table, It will throw an error: For example: -- CREATE FOREIGN TABLE vzdalena_tabulka2(a int, b varchar)   SERVER omega_db