From 9be99b0a18ce34b7538bf53179057b2717686b34 Mon Sep 17 00:00:00 2001 From: Bharath Rupireddy Date: Wed, 26 May 2021 19:24:35 +0530 Subject: [PATCH v5] Adjust batch_size to not exceed max param limit of libpq In postgres_fdw, when batch_size is set too high (> 65535) and more than 65535 rows are inserted into the foreign table, then the insert query fails. This is because of the libpq protocol maximum limit (65535) on number of query parameters. Instead of failing in this scenario, it is better to adjust the batch_size so that the number of parameters in a batch (number of columns * batch_size) does not exceed maximum number of parameters (65535) supported by the FE/BE protocol. While on this, change the maximum number of parameters limit of libpq to a macro instead of hard coded value. --- contrib/postgres_fdw/postgres_fdw.c | 12 ++++++++++-- doc/src/sgml/postgres-fdw.sgml | 13 +++++++++++-- src/interfaces/libpq/fe-exec.c | 21 ++++++++++++--------- src/interfaces/libpq/libpq-fe.h | 2 ++ 4 files changed, 35 insertions(+), 13 deletions(-) diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c index c48a421e88..fb82a0070a 100644 --- a/contrib/postgres_fdw/postgres_fdw.c +++ b/contrib/postgres_fdw/postgres_fdw.c @@ -1979,7 +1979,7 @@ postgresGetForeignModifyBatchSize(ResultRelInfo *resultRelInfo) Assert(fmstate == NULL || fmstate->aux_fmstate == NULL); /* - * In EXPLAIN without ANALYZE, ri_fdwstate is NULL, so we have to lookup + * In EXPLAIN without ANALYZE, ri_FdwState is NULL, so we have to lookup * the option directly in server/table options. Otherwise just use the * value we determined earlier. */ @@ -1994,7 +1994,15 @@ postgresGetForeignModifyBatchSize(ResultRelInfo *resultRelInfo) resultRelInfo->ri_TrigDesc->trig_insert_after_row)) return 1; - /* Otherwise use the batch size specified for server/table. */ + /* + * Otherwise use the batch size specified for server/table. If the number + * of parameters in a batch exceeds maximum number of parameters supported + * by the FE/BE protocol, then adjust the specified batch size accordingly. + */ + if (fmstate && fmstate->p_nums > 0 && + batch_size * fmstate->p_nums > PQ_QUERY_PARAM_MAX_LIMIT) + batch_size = PQ_QUERY_PARAM_MAX_LIMIT / fmstate->p_nums; + return batch_size; } diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml index fb87372bde..0be2e693a8 100644 --- a/doc/src/sgml/postgres-fdw.sgml +++ b/doc/src/sgml/postgres-fdw.sgml @@ -367,9 +367,18 @@ OPTIONS (ADD password_required 'false'); This option specifies the number of rows postgres_fdw - should insert in each insert operation. It can be specified for a + inserts in each insert operation. It can be specified for a foreign table or a foreign server. The option specified on a table - overrides an option specified for the server. + 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. The default is 1. diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c index 03592bdce9..832d61c544 100644 --- a/src/interfaces/libpq/fe-exec.c +++ b/src/interfaces/libpq/fe-exec.c @@ -1403,10 +1403,11 @@ PQsendQueryParams(PGconn *conn, libpq_gettext("command string is a null pointer\n")); return 0; } - if (nParams < 0 || nParams > 65535) + if (nParams < 0 || nParams > PQ_QUERY_PARAM_MAX_LIMIT) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("number of parameters must be between 0 and 65535\n")); + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("number of parameters must be between 0 and %d\n"), + PQ_QUERY_PARAM_MAX_LIMIT); return 0; } @@ -1451,10 +1452,11 @@ PQsendPrepare(PGconn *conn, libpq_gettext("command string is a null pointer\n")); return 0; } - if (nParams < 0 || nParams > 65535) + if (nParams < 0 || nParams > PQ_QUERY_PARAM_MAX_LIMIT) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("number of parameters must be between 0 and 65535\n")); + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("number of parameters must be between 0 and %d\n"), + PQ_QUERY_PARAM_MAX_LIMIT); return 0; } @@ -1548,10 +1550,11 @@ PQsendQueryPrepared(PGconn *conn, libpq_gettext("statement name is a null pointer\n")); return 0; } - if (nParams < 0 || nParams > 65535) + if (nParams < 0 || nParams > PQ_QUERY_PARAM_MAX_LIMIT) { - appendPQExpBufferStr(&conn->errorMessage, - libpq_gettext("number of parameters must be between 0 and 65535\n")); + appendPQExpBuffer(&conn->errorMessage, + libpq_gettext("number of parameters must be between 0 and %d\n"), + PQ_QUERY_PARAM_MAX_LIMIT); return 0; } diff --git a/src/interfaces/libpq/libpq-fe.h b/src/interfaces/libpq/libpq-fe.h index 227adde5a5..113ab52ada 100644 --- a/src/interfaces/libpq/libpq-fe.h +++ b/src/interfaces/libpq/libpq-fe.h @@ -429,6 +429,8 @@ extern PGresult *PQexecPrepared(PGconn *conn, int resultFormat); /* Interface for multiple-result or asynchronous queries */ +#define PQ_QUERY_PARAM_MAX_LIMIT 65535 + extern int PQsendQuery(PGconn *conn, const char *query); extern int PQsendQueryParams(PGconn *conn, const char *command, -- 2.25.1