public inbox for [email protected]
help / color / mirror / Atom feedFrom: [email protected] <[email protected]>
To: Bharath Rupireddy <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: RE: Fdw batch insert error out when set batch_size > 65535
Date: Tue, 25 May 2021 07:38:23 +0000
Message-ID: <OS0PR01MB57160ADFD5010254C8D1CE6594259@OS0PR01MB5716.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CALj2ACV=qgpccTEip7NHJWDe3Set8wXwDmNnRzy=GYmx9x94XQ@mail.gmail.com>
References: <OS0PR01MB571603973C0AC2874AD6BF2594299@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CALj2ACXg3tPAChzFd94_k_tP3x61QTVpuqKZke3M4ef78xEcuA@mail.gmail.com>
<OS0PR01MB5716BFBD760428859058FCBD94299@OS0PR01MB5716.jpnprd01.prod.outlook.com>
<CALj2ACV=qgpccTEip7NHJWDe3Set8wXwDmNnRzy=GYmx9x94XQ@mail.gmail.com>
From: Bharath Rupireddy <[email protected]>
Sent: Friday, May 21, 2021 5:03 PM
> On Fri, May 21, 2021 at 1:19 PM [email protected]
> <[email protected]> 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
Attachments:
[application/octet-stream] v3-0001-limit-the-fdw-batch-size.patch (4.9K, ../OS0PR01MB57160ADFD5010254C8D1CE6594259@OS0PR01MB5716.jpnprd01.prod.outlook.com/2-v3-0001-limit-the-fdw-batch-size.patch)
download | inline diff:
From 711bebfcc0f59d9905492d3d8680c02ca38c23f2 Mon Sep 17 00:00:00 2001
From: houzj <[email protected]>
Date: Tue, 25 May 2021 15:30:37 +0800
Subject: [PATCH] limit-the-fdw-batch-size
---
contrib/postgres_fdw/postgres_fdw.c | 10 +++++++++-
doc/src/sgml/postgres-fdw.sgml | 13 +++++++++++--
src/interfaces/libpq/fe-exec.c | 21 ++++++++++++---------
src/interfaces/libpq/libpq-fe.h | 2 ++
4 files changed, 34 insertions(+), 12 deletions(-)
diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index c48a421..11b50de 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -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. */
+ /*
+ * Adjust the batch_size to make sure the number of parameters in a batch
+ * does not exceed maximum number of parameters supported by the FE/BE
+ * protocol.
+ */
+ 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;
+
return batch_size;
}
diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index fb87372..0be2e69 100644
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -367,9 +367,18 @@ OPTIONS (ADD password_required 'false');
<listitem>
<para>
This option specifies the number of rows <filename>postgres_fdw</filename>
- 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 <filename>postgres_fdw</filename> inserts in a batch actually
+ depends on the number of columns and the provided <literal>batch_size</literal>
+ value. This is because of the limit the libpq protocol (which
+ <filename>postgres_fdw</filename> 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 * <literal>batch_size</literal>
+ is more than the limit, then the libpq emits an error. But
+ <filename>postgres_fdw</filename> adjusts the <literal>batch_size</literal>
+ to avoid this error.
The default is <literal>1</literal>.
</para>
</listitem>
diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 03592bd..832d61c 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 227adde..113ab52 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.7.2.windows.1
view thread (29+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected]
Subject: RE: Fdw batch insert error out when set batch_size > 65535
In-Reply-To: <OS0PR01MB57160ADFD5010254C8D1CE6594259@OS0PR01MB5716.jpnprd01.prod.outlook.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox