Adjust batch size in postgres_fdw to not use too many parameters

The FE/BE protocol identifies parameters with an Int16 index, which
limits the maximum number of parameters per query to 65535. With
batching added to postges_fdw this limit is much easier to hit, as
the whole batch is essentially a single query, making this error much
easier to hit.

The failures are a bit unpredictable, because it also depends on the
number of columns in the query. So instead of just failing, this patch
tweaks the batch_size to not exceed the maximum number of parameters.

Reported-by: Hou Zhijie <houzj.fnst@cn.fujitsu.com>
Reviewed-by: Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>
Discussion: https://postgr.es/m/OS0PR01MB571603973C0AC2874AD6BF2594299%40OS0PR01MB5716.jpnprd01.prod.outlook.com

diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
index 7b7c0db16cffcf9d7c01e5ce8459ee17a7a55389..1fb26639fcb9126c296a6442265cbfbc89ff9fee 100644 (file)
--- a/contrib/postgres_fdw/expected/postgres_fdw.out
+++ b/contrib/postgres_fdw/expected/postgres_fdw.out
@@ -9680,6 +9680,17 @@ SELECT COUNT(*) FROM ftable;
     34
 (1 row)
 
+TRUNCATE batch_table;
+DROP FOREIGN TABLE ftable;
+-- try if large batches exceed max number of bind parameters
+CREATE FOREIGN TABLE ftable ( x int ) SERVER loopback OPTIONS ( table_name 'batch_table', batch_size '100000' );
+INSERT INTO ftable SELECT * FROM generate_series(1, 70000) i;
+SELECT COUNT(*) FROM ftable;
+ count 
+-------
+ 70000
+(1 row)
+
 TRUNCATE batch_table;
 DROP FOREIGN TABLE ftable;
 -- Disable batch insert

diff --git a/contrib/postgres_fdw/postgres_fdw.c b/contrib/postgres_fdw/postgres_fdw.c
index 0843ed9dba2a2d52647439564504dd86fd148cc9..fafbab6b024d484b845f50ad3f309c707be0734b 100644 (file)
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -2030,7 +2030,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.
     */
@@ -2045,7 +2045,14 @@ 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. The number of
+    * parameters in a batch is limited to 65535 (uint16), so make sure we
+    * don't exceed this limit by using the maximum batch_size possible.
+    */
+   if (fmstate && fmstate->p_nums > 0)
+       batch_size = Min(batch_size, PQ_QUERY_PARAM_MAX_LIMIT / fmstate->p_nums);
+
    return batch_size;
 }
 

diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
index 191efbf7c248eb90e45b74ccc5fbe45cfb5116b8..8cb2148f1f6aa991c648602b45745f437dfc17e3 100644 (file)
--- a/contrib/postgres_fdw/sql/postgres_fdw.sql
+++ b/contrib/postgres_fdw/sql/postgres_fdw.sql
@@ -3026,6 +3026,13 @@ SELECT COUNT(*) FROM ftable;
 TRUNCATE batch_table;
 DROP FOREIGN TABLE ftable;
 
+-- try if large batches exceed max number of bind parameters
+CREATE FOREIGN TABLE ftable ( x int ) SERVER loopback OPTIONS ( table_name 'batch_table', batch_size '100000' );
+INSERT INTO ftable SELECT * FROM generate_series(1, 70000) i;
+SELECT COUNT(*) FROM ftable;
+TRUNCATE batch_table;
+DROP FOREIGN TABLE ftable;
+
 -- Disable batch insert
 CREATE FOREIGN TABLE ftable ( x int ) SERVER loopback OPTIONS ( table_name 'batch_table', batch_size '1' );
 EXPLAIN (VERBOSE, COSTS OFF) INSERT INTO ftable VALUES (1), (2);

diff --git a/doc/src/sgml/postgres-fdw.sgml b/doc/src/sgml/postgres-fdw.sgml
index 5aced083e9eb0d134aedf02fc7721ad485acf0dd..d96c3d0f0cd3be00cea6311c1238c0d9896a765e 100644 (file)
--- a/doc/src/sgml/postgres-fdw.sgml
+++ b/doc/src/sgml/postgres-fdw.sgml
@@ -372,6 +372,17 @@ OPTIONS (ADD password_required 'false');
        overrides an option specified for the server.
        The default is <literal>1</literal>.
       </para>
+
+      <para>
+       Note the actual number of rows <filename>postgres_fdw</filename> inserts at
+       once depends on the number of columns and the provided
+       <literal>batch_size</literal> value. The batch is executed as a single
+       query, and the libpq protocol (which <filename>postgres_fdw</filename>
+       uses to connect to a remote server) limits the number of parameters in a
+       single query to 65535. When the number of columns * <literal>batch_size</literal>
+       exceeds the limit, the <literal>batch_size</literal> will be adjusted to
+       avoid an error.
+      </para>
      </listitem>
     </varlistentry>
 

diff --git a/src/interfaces/libpq/fe-exec.c b/src/interfaces/libpq/fe-exec.c
index 03592bdce9fc9c0bebff75b282279993d0daf215..832d61c544f38173ecbd67254743c0d81608d9cd 100644 (file)
--- 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 845b4c04c9c2f5159ee47fdc61022530a3c2f5d4..ca733a2004835887332043ee4ea69efe42e414d8 100644 (file)
--- 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,