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

From: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Sent: Friday, May 21, 2021 5:03 PM
> On Fri, May 21, 2021 at 1:19 PM houzj.fnst@fujitsu.com
> <houzj.fnst@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