From: Bharath Rupireddy <bharath.rupireddyforpostg...@gmail.com>
Sent: Friday, May 21, 2021 5:03 PM
> On Fri, May 21, 2021 at 1:19 PM houzj.f...@fujitsu.com
> <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 can have something
> like
> below, since we are using it in the division.
> 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;
> }
> Also, please remove the { and } for the above if condition, because for 1 line
> statements we don't normally use { and }
> 2) An empty line after the macro definition will be good.
> +#define PQ_QUERY_PARAM_MAX_LIMIT 65535
> extern int PQsendQuery(PGconn *conn, const char *query);
> 3) I think we use:
> <filename>postgres_fdw</filename> not postgres_fdw
> <literal>batch_size</literal> not batch_size the number of columns *
> <literal>batch_size</literal> not the number of columns * batch_size
> + overrides an option specified for the server. Note the final number
> + of rows postgres_fdw inserts in a batch actually depends on the
> + number of columns and the provided batch_size value. This is because
> + of the limit the libpq protocol (which postgres_fdw uses to connect
> + to a remote server) has on the number of query parameters that can
> + be specified per query. For instance, if the number of columns
> * batch_size
> + is more than the limit, then the libpq emits an error. But
> postgres_fdw
> + adjusts the batch_size to avoid this error.Thanks for the comments. I have addressed all comments to the v3 patch. BTW, Is the batch_size issue here an Open Item of PG14 ? Best regards, houzj
v3-0001-limit-the-fdw-batch-size.patch
Description: v3-0001-limit-the-fdw-batch-size.patch
