Обсуждение: Doc: Improve note about copying into postgres_fdw foreign tables in batch

Hi,

Here is a small patch to improve the note, which was added by commit
97da48246 ("Allow batch insertion during COPY into a foreign table."),
by adding an explanation about how the actual number of rows
postgres_fdw inserts at once is determined in the COPY case, including
a limitation that does not apply to the INSERT case.

I will add this to the next CF.

Best regards,
Etsuro Fujita

On Fri, Feb 17, 2023 at 5:45 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
> Here is a small patch to improve the note, which was added by commit
> 97da48246 ("Allow batch insertion during COPY into a foreign table."),
> by adding an explanation about how the actual number of rows
> postgres_fdw inserts at once is determined in the COPY case, including
> a limitation that does not apply to the INSERT case.

Does anyone want to comment on this?

Best regards,
Etsuro Fujita

> On Fri, Feb 17, 2023 at 5:45 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>> Here is a small patch to improve the note, which was added by commit
>> 97da48246 ("Allow batch insertion during COPY into a foreign table."),
>> by adding an explanation about how the actual number of rows
>> postgres_fdw inserts at once is determined in the COPY case, including
>> a limitation that does not apply to the INSERT case.
> 
> Does anyone want to comment on this?

>        <para>
> -       This option also applies when copying into foreign tables.
> +       This option also applies when copying into foreign tables.  In that case
> +       the actual number of rows <filename>postgres_fdw</filename> copies at
> +       once is determined in a similar way to in the insert case, but it is

"similar way to in" should be "similar way to", maybe?

> +       limited to at most 1000 due to implementation restrictions of the
> +       <command>COPY</command> command.
>        </para>
>       </listitem>
>      </varlistentry>

Best reagards,
--
Tatsuo Ishii
SRA OSS LLC
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp

> On 22 Mar 2023, at 12:58, Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>
> On Fri, Feb 17, 2023 at 5:45 PM Etsuro Fujita <etsuro.fujita@gmail.com> wrote:
>> Here is a small patch to improve the note, which was added by commit
>> 97da48246 ("Allow batch insertion during COPY into a foreign table."),
>> by adding an explanation about how the actual number of rows
>> postgres_fdw inserts at once is determined in the COPY case, including
>> a limitation that does not apply to the INSERT case.
>
> Does anyone want to comment on this?

Patch looks good to me, but I agree with Tatsuo downthread that "similar way to
the insert case" reads better.  Theoretically the number could be different
from 1000 if MAX_BUFFERED_TUPLES was changed in the build, but that's a
non-default not worth spending time explaining.

+       the actual number of rows <filename>postgres_fdw</filename> copies at

While not the fault of this patch I find it confusing that we mix <filename>
and <literal> for marking up "postgres_fdw", the latter seemingly more correct
(and less commonly used) than <filename>.

--
Daniel Gustafsson

> While not the fault of this patch I find it confusing that we mix <filename>
> and <literal> for marking up "postgres_fdw", the latter seemingly more correct
> (and less commonly used) than <filename>.

I think we traditionally use <filename> for an extension module (file)
name. It seems the <literal> is used when we want to refer to objects
other than files.

Best reagards,
--
Tatsuo Ishii
SRA OSS LLC
English: http://www.sraoss.co.jp/index_en/
Japanese:http://www.sraoss.co.jp

On Wed, Mar 22, 2023 at 9:13 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> Patch looks good to me, but I agree with Tatsuo downthread that "similar way to
> the insert case" reads better.

Ok, I removed "in".

> Theoretically the number could be different
> from 1000 if MAX_BUFFERED_TUPLES was changed in the build, but that's a
> non-default not worth spending time explaining.

Agreed.

> +       the actual number of rows <filename>postgres_fdw</filename> copies at
>
> While not the fault of this patch I find it confusing that we mix <filename>
> and <literal> for marking up "postgres_fdw", the latter seemingly more correct
> (and less commonly used) than <filename>.

On Wed, Mar 22, 2023 at 9:32 PM Tatsuo Ishii <ishii@sraoss.co.jp> wrote:
> I think we traditionally use <filename> for an extension module (file)
> name. It seems the <literal> is used when we want to refer to objects
> other than files.

<filename> seems more appropriate to me as well in this context, so I
left it alone.

Attached is an updated version of the patch.

Thanks for looking, Daniel and Ishii-san!

Best regards,
Etsuro Fujita

> On 23 Mar 2023, at 10:51, Etsuro Fujita <etsuro.fujita@gmail.com> wrote:

> <filename> seems more appropriate to me as well in this context, so I
> left it alone.

And just to be clear, I think you are right in leaving it alone given the
context.

> Attached is an updated version of the patch.

LGTM.

--
Daniel Gustafsson

On Thu, Mar 23, 2023 at 6:55 PM Daniel Gustafsson <daniel@yesql.se> wrote:
> > <filename> seems more appropriate to me as well in this context, so I
> > left it alone.
>
> And just to be clear, I think you are right in leaving it alone given the
> context.
>
> > Attached is an updated version of the patch.
>
> LGTM.

Cool!  Pushed.

Thanks again!

Best regards,
Etsuro Fujita