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

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.

With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com