Doc: Improve note about copying into postgres_fdw foreign tables in batch

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