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
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:
>
> -
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
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 c
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?
> By
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 postgresGetForeignMod
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 Ope
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. I
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 th
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 c
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 houzj.f...
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
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))
> {
> bat
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
>
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 hav
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 t
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
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 * fmsta
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 bat
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
> - wi
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 tha
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 execu
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 because
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, as
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 (a
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 eliminat
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 c
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 to
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, n
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 mi
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.
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
> >>> m
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 150
33 matches
Mail list logo