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.
With Regards,
Bharath Rupireddy.
EnterpriseDB: http://www.enterprisedb.com
