postgres_fdw: Emit message when batch_size is reduced

On Mon, Jun 1, 2026 at 7:55 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

Hello hackers,

I noticed that when configured batch_size is reduced because of libpq
limit, in postgresGetForeignModifyBatchSize, there is no message about it
that gets to the user. I am thinking that to keep the transparency it makes
sense to add a debug message at least, as per the attached patch.

What do you think?

I'm iffy on this one.

The desire for instrumentation is generally good, and if a database was
having a problem where the tuned parameter wasn't having the desired effect
then it's understandable to want to know that.

But I'm also not certain what different action someone would take if they
knew that this situation was happening. Do you anticipate some sort of
action on the user or DBA's part from this information beyond understanding
that's why you're not getting the full batch size?

Thoughts on the code change itself:

- The new variable (configured) should be declared in the new if/then
block, which is the narrowest scope possible.

- The ereport() uses the old style with errmsg inside parens, and that's no
longer necessary. Existing ereport calls were left unchanged to avoid code
churn, but new ones should be in the simpler style.

- This error is debuggy-enough that it might be an elog() rather than an
ereport()

- I don't have good guidance on whether it should be DEBUG1 vs DEBUG2 or a
higher number. Curious if we have that guidance documented anywhere.

- In the test case, choosing a batch_size > PQ_QUERY_PARAM_MAX_LIMIT seems
a bit artificial, in that it's a value that can't work for any query with
parameters, and someone might add sanity checks to batch_size in option.c,
which would in turn invalidate the test case. A test case with a 2-column
table should be able to get the same effect any batch_size greater than
half of 65535, yes?

On Fri, 5 Jun 2026 at 07:43, Corey Huinker <corey.huinker@gmail.com> wrote:

On Mon, Jun 1, 2026 at 7:55 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

Hello hackers,

I noticed that when configured batch_size is reduced because of libpq
limit, in postgresGetForeignModifyBatchSize, there is no message about it
that gets to the user. I am thinking that to keep the transparency it makes
sense to add a debug message at least, as per the attached patch.

What do you think?

I'm iffy on this one.

The desire for instrumentation is generally good, and if a database was
having a problem where the tuned parameter wasn't having the desired effect
then it's understandable to want to know that.

But I'm also not certain what different action someone would take if they
knew that this situation was happening. Do you anticipate some sort of
action on the user or DBA's part from this information beyond understanding
that's why you're not getting the full batch size?

Hey Corey,

Thanks for your opinion. As I said, it feels important from the
transparency point to know when and why the batch_size changed for a case.

Thoughts on the code change itself:

- The new variable (configured) should be declared in the new if/then
block, which is the narrowest scope possible.

- The ereport() uses the old style with errmsg inside parens, and that's
no longer necessary. Existing ereport calls were left unchanged to avoid
code churn, but new ones should be in the simpler style.

- This error is debuggy-enough that it might be an elog() rather than an
ereport()

- I don't have good guidance on whether it should be DEBUG1 vs DEBUG2 or a
higher number. Curious if we have that guidance documented anywhere.

- In the test case, choosing a batch_size > PQ_QUERY_PARAM_MAX_LIMIT seems
a bit artificial, in that it's a value that can't work for any query with
parameters, and someone might add sanity checks to batch_size in option.c,
which would in turn invalidate the test case. A test case with a 2-column
table should be able to get the same effect any batch_size greater than
half of 65535, yes?

I reworked the patch with your suggestions.
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

On Fri, Jun 5, 2026 at 7:02 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Fri, 5 Jun 2026 at 07:43, Corey Huinker <corey.huinker@gmail.com>
wrote:

On Mon, Jun 1, 2026 at 7:55 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

Hello hackers,

I noticed that when configured batch_size is reduced because of libpq
limit, in postgresGetForeignModifyBatchSize, there is no message about it
that gets to the user. I am thinking that to keep the transparency it makes
sense to add a debug message at least, as per the attached patch.

What do you think?

I'm iffy on this one.

The desire for instrumentation is generally good, and if a database was
having a problem where the tuned parameter wasn't having the desired effect
then it's understandable to want to know that.

But I'm also not certain what different action someone would take if they
knew that this situation was happening. Do you anticipate some sort of
action on the user or DBA's part from this information beyond understanding
that's why you're not getting the full batch size?

Hey Corey,

Thanks for your opinion. As I said, it feels important from the
transparency point to know when and why the batch_size changed for a case.

Thoughts on the code change itself:

- The new variable (configured) should be declared in the new if/then
block, which is the narrowest scope possible.

- The ereport() uses the old style with errmsg inside parens, and that's
no longer necessary. Existing ereport calls were left unchanged to avoid
code churn, but new ones should be in the simpler style.

- This error is debuggy-enough that it might be an elog() rather than an
ereport()

- I don't have good guidance on whether it should be DEBUG1 vs DEBUG2 or
a higher number. Curious if we have that guidance documented anywhere.

- In the test case, choosing a batch_size > PQ_QUERY_PARAM_MAX_LIMIT
seems a bit artificial, in that it's a value that can't work for any query
with parameters, and someone might add sanity checks to batch_size in
option.c, which would in turn invalidate the test case. A test case with a
2-column table should be able to get the same effect any batch_size greater
than half of 65535, yes?

I reworked the patch with your suggestions.
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

If you can, run pgindent on your changes before submitting a patch.

The lack of a tab after "int" and the lack of a blank line after the var
declaration were easy to spot, but just in case I ran pgindent myself and
got:

$ git diff
diff --git a/contrib/postgres_fdw/postgres_fdw.c
b/contrib/postgres_fdw/postgres_fdw.c
index 633451973fb..2e1bc47a737 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -2305,12 +2305,13 @@ postgresGetForeignModifyBatchSize(ResultRelInfo
*resultRelInfo)
     */
    if (fmstate && fmstate->p_nums > 0)
    {
-       int configured = batch_size;
+       int         configured = batch_size;
+
        batch_size = Min(batch_size, PQ_QUERY_PARAM_MAX_LIMIT /
fmstate->p_nums);
        if (batch_size < configured)
-           elog(DEBUG1,"postgres_fdw: batch_size reduced from %d to %d "
-                           "to stay within the libpq %d-parameter limit",
-                           configured, batch_size,
PQ_QUERY_PARAM_MAX_LIMIT);
+           elog(DEBUG1, "postgres_fdw: batch_size reduced from %d to %d "
+                "to stay within the libpq %d-parameter limit",
+                configured, batch_size, PQ_QUERY_PARAM_MAX_LIMIT);
    }

The elog may get changed back into an ereport. If it does, it'll come under
our wording guidelines scrutiny, but we'll worry about that later.

Next, the test case still does "INSERT INTO ftable VALUES(1)", which
surprised me that it works even though ftable now has 2 columns. I think we
should change that to

INSERT INTO ftable(x) VALUES(1):

As this will remote the cognitive dissonance I experienced when looking at
the test case, and it has the bonus of demonstrating that the parameters
are generated in the SQL that postgres_fdw generates, not the SQL sent by
the user.

As for the "should we even do this" question, the overhead seems pretty
small and it will only come up in corner cases, and even then only for
those who have signed up for the firehose of DEBUG1 messages, so I'm at
least a +0.5 on this now.

- In the test case, choosing a batch_size > PQ_QUERY_PARAM_MAX_LIMIT
seems a bit artificial, in that it's a value that can't work for any query
with parameters, and someone might add sanity checks to batch_size in
option.c, which would in turn invalidate the test case. A test case with a
2-column table should be able to get the same effect any batch_size greater
than half of 65535, yes?

Thinking some more on this part, I think it would be worthwhile to check on
assignments to batch_size, and issue a NOTICE or WARNING that the set value
exceeds PQ_QUERY_PARAM_MAX_LIMIT, but does not reject the change because
this value could theoretically (but very unlikely) get smaller in the
future and we don't want to mess up any pg_restores or pg_upgrades.

On Fri, 5 Jun 2026 at 19:44, Corey Huinker <corey.huinker@gmail.com> wrote:

On Fri, Jun 5, 2026 at 7:02 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Fri, 5 Jun 2026 at 07:43, Corey Huinker <corey.huinker@gmail.com>
wrote:

On Mon, Jun 1, 2026 at 7:55 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

Hello hackers,

I noticed that when configured batch_size is reduced because of libpq
limit, in postgresGetForeignModifyBatchSize, there is no message about it
that gets to the user. I am thinking that to keep the transparency it makes
sense to add a debug message at least, as per the attached patch.

What do you think?

I'm iffy on this one.

The desire for instrumentation is generally good, and if a database was
having a problem where the tuned parameter wasn't having the desired effect
then it's understandable to want to know that.

But I'm also not certain what different action someone would take if
they knew that this situation was happening. Do you anticipate some sort of
action on the user or DBA's part from this information beyond understanding
that's why you're not getting the full batch size?

Hey Corey,

Thanks for your opinion. As I said, it feels important from the
transparency point to know when and why the batch_size changed for a case.

Thoughts on the code change itself:

- The new variable (configured) should be declared in the new if/then
block, which is the narrowest scope possible.

- The ereport() uses the old style with errmsg inside parens, and that's
no longer necessary. Existing ereport calls were left unchanged to avoid
code churn, but new ones should be in the simpler style.

- This error is debuggy-enough that it might be an elog() rather than an
ereport()

- I don't have good guidance on whether it should be DEBUG1 vs DEBUG2 or
a higher number. Curious if we have that guidance documented anywhere.

- In the test case, choosing a batch_size > PQ_QUERY_PARAM_MAX_LIMIT
seems a bit artificial, in that it's a value that can't work for any query
with parameters, and someone might add sanity checks to batch_size in
option.c, which would in turn invalidate the test case. A test case with a
2-column table should be able to get the same effect any batch_size greater
than half of 65535, yes?

I reworked the patch with your suggestions.
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

If you can, run pgindent on your changes before submitting a patch.

The lack of a tab after "int" and the lack of a blank line after the var
declaration were easy to spot, but just in case I ran pgindent myself and
got:

$ git diff
diff --git a/contrib/postgres_fdw/postgres_fdw.c
b/contrib/postgres_fdw/postgres_fdw.c
index 633451973fb..2e1bc47a737 100644
--- a/contrib/postgres_fdw/postgres_fdw.c
+++ b/contrib/postgres_fdw/postgres_fdw.c
@@ -2305,12 +2305,13 @@ postgresGetForeignModifyBatchSize(ResultRelInfo
*resultRelInfo)
*/
if (fmstate && fmstate->p_nums > 0)
{
-       int configured = batch_size;
+       int         configured = batch_size;
+
batch_size = Min(batch_size, PQ_QUERY_PARAM_MAX_LIMIT /
fmstate->p_nums);
if (batch_size < configured)
-           elog(DEBUG1,"postgres_fdw: batch_size reduced from %d to %d "
-                           "to stay within the libpq %d-parameter limit",
-                           configured, batch_size,
PQ_QUERY_PARAM_MAX_LIMIT);
+           elog(DEBUG1, "postgres_fdw: batch_size reduced from %d to %d "
+                "to stay within the libpq %d-parameter limit",
+                configured, batch_size, PQ_QUERY_PARAM_MAX_LIMIT);
}

The elog may get changed back into an ereport. If it does, it'll come
under our wording guidelines scrutiny, but we'll worry about that later.

Next, the test case still does "INSERT INTO ftable VALUES(1)", which
surprised me that it works even though ftable now has 2 columns. I think we
should change that to

INSERT INTO ftable(x) VALUES(1):

As this will remote the cognitive dissonance I experienced when looking at
the test case, and it has the bonus of demonstrating that the parameters
are generated in the SQL that postgres_fdw generates, not the SQL sent by
the user.

As for the "should we even do this" question, the overhead seems pretty
small and it will only come up in corner cases, and even then only for
those who have signed up for the firehose of DEBUG1 messages, so I'm at
least a +0.5 on this now.

Thanks for your inputs. Reworked patch is attached.
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

On Fri, 5 Jun 2026 at 19:52, Corey Huinker <corey.huinker@gmail.com> wrote:

- In the test case, choosing a batch_size > PQ_QUERY_PARAM_MAX_LIMIT
seems a bit artificial, in that it's a value that can't work for any query
with parameters, and someone might add sanity checks to batch_size in
option.c, which would in turn invalidate the test case. A test case with a
2-column table should be able to get the same effect any batch_size greater
than half of 65535, yes?

Thinking some more on this part, I think it would be worthwhile to check
on assignments to batch_size, and issue a NOTICE or WARNING that the set
value exceeds PQ_QUERY_PARAM_MAX_LIMIT, but does not reject the change
because this value could theoretically (but very unlikely) get smaller in
the future and we don't want to mess up any pg_restores or pg_upgrades.

I am not able to understand this clearly, are you saying there should be a
new check for this NOTICE/WARNING or should the DEBUG1 message (in the
patch upthread) be a NOTICE/WARNING?
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

On Tue, Jun 9, 2026 at 7:38 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Fri, 5 Jun 2026 at 19:52, Corey Huinker <corey.huinker@gmail.com>
wrote:

- In the test case, choosing a batch_size > PQ_QUERY_PARAM_MAX_LIMIT
seems a bit artificial, in that it's a value that can't work for any query
with parameters, and someone might add sanity checks to batch_size in
option.c, which would in turn invalidate the test case. A test case with a
2-column table should be able to get the same effect any batch_size greater
than half of 65535, yes?

Thinking some more on this part, I think it would be worthwhile to check
on assignments to batch_size, and issue a NOTICE or WARNING that the set
value exceeds PQ_QUERY_PARAM_MAX_LIMIT, but does not reject the change
because this value could theoretically (but very unlikely) get smaller in
the future and we don't want to mess up any pg_restores or pg_upgrades.

I am not able to understand this clearly, are you saying there should be a
new check for this NOTICE/WARNING or should the DEBUG1 message (in the
patch upthread) be a NOTICE/WARNING?
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

I'm saying that we may want a separate check when the batch_size option is
SET (via CREATE/ALTER TABLE/SERVER) to test the value against
PQ_QUERY_PARAM_MAX_LIMIT
and issue a NOTICE/WARNING that the chosen value will always have to be set
at or below $PQ_QUERY_PARAM_MAX_LIMIT - but still let them store the values
as-is.

Thanks for your inputs. Reworked patch is attached.
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

You've addressed all my concerns, aside from the desire for the check on
the set/update of the value. Do you have a commitfest entry? I didn't find
one.

On Tue, 9 Jun 2026 at 22:22, Corey Huinker <corey.huinker@gmail.com> wrote:

Thanks for your inputs. Reworked patch is attached.
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

You've addressed all my concerns, aside from the desire for the check on
the set/update of the value. Do you have a commitfest entry? I didn't find
one.

There is commitfest entry now -->
https://commitfest.postgresql.org/patch/6873/

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

On Wed, Jun 10, 2026 at 5:09 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Tue, 9 Jun 2026 at 22:22, Corey Huinker <corey.huinker@gmail.com>
wrote:

Thanks for your inputs. Reworked patch is attached.
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

You've addressed all my concerns, aside from the desire for the check on
the set/update of the value. Do you have a commitfest entry? I didn't find
one.

There is commitfest entry now -->
https://commitfest.postgresql.org/patch/6873/

I've added myself as a reviewer. Did you want to try adding the check at
time of the option being set? If not, I can make an attempt at that.

On Tue, 16 Jun 2026 at 22:20, Corey Huinker <corey.huinker@gmail.com> wrote:

On Wed, Jun 10, 2026 at 5:09 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Tue, 9 Jun 2026 at 22:22, Corey Huinker <corey.huinker@gmail.com>
wrote:

Thanks for your inputs. Reworked patch is attached.
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

You've addressed all my concerns, aside from the desire for the check on
the set/update of the value. Do you have a commitfest entry? I didn't find
one.

There is commitfest entry now -->
https://commitfest.postgresql.org/patch/6873/

I've added myself as a reviewer. Did you want to try adding the check at
time of the option being set? If not, I can make an attempt at that.

Please find the attached file for the patch with the warning message at the
time of batch_size option addition. Looking forward to your inputs.

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

On Fri, Jun 19, 2026 at 8:38 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Tue, 16 Jun 2026 at 22:20, Corey Huinker <corey.huinker@gmail.com>
wrote:

On Wed, Jun 10, 2026 at 5:09 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Tue, 9 Jun 2026 at 22:22, Corey Huinker <corey.huinker@gmail.com>
wrote:

Thanks for your inputs. Reworked patch is attached.
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

You've addressed all my concerns, aside from the desire for the check
on the set/update of the value. Do you have a commitfest entry? I didn't
find one.

There is commitfest entry now -->
https://commitfest.postgresql.org/patch/6873/

I've added myself as a reviewer. Did you want to try adding the check at
time of the option being set? If not, I can make an attempt at that.

Please find the attached file for the patch with the warning message at
the time of batch_size option addition. Looking forward to your inputs.

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

Applies clean, passes.

I think we need to tweak the elog() below:

+ if (batch_size > PQ_QUERY_PARAM_MAX_LIMIT)
+ elog(WARNING, "postgres_fdw: batch_size %d is at or above the libpq "
+ "%d-parameter limit; the effective per-batch ceiling is "
+ "limit / number_of_columns and may be lower",
+ batch_size, PQ_QUERY_PARAM_MAX_LIMIT);

I think this should be an ereport() because it's the sort of error we'd
want the caller to see, and that means we need the message to conform the
guidelines at https://www.postgresql.org/docs/current/error-style-guide.html,
and I'm going to suggest this as a starting point:

ereport(WARNING,
errmsg("%s of %d exceeds protocol limit of %d", "batch_size",
batch_size, PQ_QUERY_PARAM_MAX_LIMIT),
errdetail("The %s for a query will be reduced to protocol limit
divided by the number of columns in the query.", "batch_size"));

I'd like to hear other people's opinions on what the proper conforming
error message would be.

On Mon, 22 Jun 2026 at 22:40, Corey Huinker <corey.huinker@gmail.com> wrote:

On Fri, Jun 19, 2026 at 8:38 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Tue, 16 Jun 2026 at 22:20, Corey Huinker <corey.huinker@gmail.com>
wrote:

On Wed, Jun 10, 2026 at 5:09 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Tue, 9 Jun 2026 at 22:22, Corey Huinker <corey.huinker@gmail.com>
wrote:

Thanks for your inputs. Reworked patch is attached.
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

You've addressed all my concerns, aside from the desire for the check
on the set/update of the value. Do you have a commitfest entry? I didn't
find one.

There is commitfest entry now -->
https://commitfest.postgresql.org/patch/6873/

I've added myself as a reviewer. Did you want to try adding the check at
time of the option being set? If not, I can make an attempt at that.

Please find the attached file for the patch with the warning message at
the time of batch_size option addition. Looking forward to your inputs.

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

Applies clean, passes.

I think we need to tweak the elog() below:

+ if (batch_size > PQ_QUERY_PARAM_MAX_LIMIT)
+ elog(WARNING, "postgres_fdw: batch_size %d is at or above the libpq "
+ "%d-parameter limit; the effective per-batch ceiling is "
+ "limit / number_of_columns and may be lower",
+ batch_size, PQ_QUERY_PARAM_MAX_LIMIT);

I think this should be an ereport() because it's the sort of error we'd
want the caller to see, and that means we need the message to conform the
guidelines at
https://www.postgresql.org/docs/current/error-style-guide.html, and I'm
going to suggest this as a starting point:

ereport(WARNING,
errmsg("%s of %d exceeds protocol limit of %d", "batch_size",
batch_size, PQ_QUERY_PARAM_MAX_LIMIT),
errdetail("The %s for a query will be reduced to protocol limit
divided by the number of columns in the query.", "batch_size"));

Done.

I'd like to hear other people's opinions on what the proper conforming
error message would be.

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

Hi,
I reviewed v5, applied it to the current master, and ran the postgres_fdw
regression tests. The patch applies cleanly and the tests pass.

Would it also make sense to emit a NOTICE or WARNING from
postgres_fdw_validator() in option.c when batch_size is set to a value
greater than PQ_QUERY_PARAM_MAX_LIMIT?

That would notify the user immediately during CREATE or ALTER, while still
accepting the value and preserving the existing behavior.

The existing tests cover cases where the messages are emitted, but I think
it may also be useful to cover the exact boundaries and the cases where no
message should be emitted.

For a two-parameter foreign insert, batch_size = 32767 should not produce a
DEBUG1 message, while batch_size = 32768 should be reduced to 32767.
Similarly, for a single parameter, batch_size = 65535 should not produce a
warning, while 65536 should.

Regards,
Nurlan

сб, 11 июл. 2026 г. в 21:13, Rafia Sabih <rafia.pghackers@gmail.com>:

On Mon, 22 Jun 2026 at 22:40, Corey Huinker <corey.huinker@gmail.com>
wrote:

On Fri, Jun 19, 2026 at 8:38 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Tue, 16 Jun 2026 at 22:20, Corey Huinker <corey.huinker@gmail.com>
wrote:

On Wed, Jun 10, 2026 at 5:09 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Tue, 9 Jun 2026 at 22:22, Corey Huinker <corey.huinker@gmail.com>
wrote:

Thanks for your inputs. Reworked patch is attached.
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

You've addressed all my concerns, aside from the desire for the check
on the set/update of the value. Do you have a commitfest entry? I didn't
find one.

There is commitfest entry now -->
https://commitfest.postgresql.org/patch/6873/

I've added myself as a reviewer. Did you want to try adding the check
at time of the option being set? If not, I can make an attempt at that.

Please find the attached file for the patch with the warning message at
the time of batch_size option addition. Looking forward to your inputs.

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

Applies clean, passes.

I think we need to tweak the elog() below:

+ if (batch_size > PQ_QUERY_PARAM_MAX_LIMIT)
+ elog(WARNING, "postgres_fdw: batch_size %d is at or above the libpq "
+ "%d-parameter limit; the effective per-batch ceiling is "
+ "limit / number_of_columns and may be lower",
+ batch_size, PQ_QUERY_PARAM_MAX_LIMIT);

I think this should be an ereport() because it's the sort of error we'd
want the caller to see, and that means we need the message to conform the
guidelines at
https://www.postgresql.org/docs/current/error-style-guide.html, and I'm
going to suggest this as a starting point:

ereport(WARNING,
errmsg("%s of %d exceeds protocol limit of %d", "batch_size",
batch_size, PQ_QUERY_PARAM_MAX_LIMIT),
errdetail("The %s for a query will be reduced to protocol limit
divided by the number of columns in the query.", "batch_size"));

Done.

I'd like to hear other people's opinions on what the proper conforming
error message would be.

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

--
Regards,
Nurlan

On Sat, 11 Jul 2026 at 23:56, Nurlan Tulemisov <nurlan.tulemisov@gmail.com>
wrote:

Hi,
I reviewed v5, applied it to the current master, and ran the postgres_fdw
regression tests. The patch applies cleanly and the tests pass.

Thank you for the review.

Would it also make sense to emit a NOTICE or WARNING from
postgres_fdw_validator() in option.c when batch_size is set to a value
greater than PQ_QUERY_PARAM_MAX_LIMIT?

Wouldn't that give duplicate warnings with the changes already in the

patch...or you are suggesting to have it only in postgres_fdw_validator() ?

That would notify the user immediately during CREATE or ALTER, while still
accepting the value and preserving the existing behavior.

The existing tests cover cases where the messages are emitted, but I think
it may also be useful to cover the exact boundaries and the cases where no
message should be emitted.

For a two-parameter foreign insert, batch_size = 32767 should not produce
a DEBUG1 message, while batch_size = 32768 should be reduced to 32767.
Similarly, for a single parameter, batch_size = 65535 should not produce a
warning, while 65536 should.

Done.
Please find the updated patch attached.

Regards,
Nurlan

сб, 11 июл. 2026 г. в 21:13, Rafia Sabih <rafia.pghackers@gmail.com>:

On Mon, 22 Jun 2026 at 22:40, Corey Huinker <corey.huinker@gmail.com>
wrote:

On Fri, Jun 19, 2026 at 8:38 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Tue, 16 Jun 2026 at 22:20, Corey Huinker <corey.huinker@gmail.com>
wrote:

On Wed, Jun 10, 2026 at 5:09 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Tue, 9 Jun 2026 at 22:22, Corey Huinker <corey.huinker@gmail.com>
wrote:

Thanks for your inputs. Reworked patch is attached.
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

You've addressed all my concerns, aside from the desire for the
check on the set/update of the value. Do you have a commitfest entry? I
didn't find one.

There is commitfest entry now -->
https://commitfest.postgresql.org/patch/6873/

I've added myself as a reviewer. Did you want to try adding the check
at time of the option being set? If not, I can make an attempt at that.

Please find the attached file for the patch with the warning message at
the time of batch_size option addition. Looking forward to your inputs.

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

Applies clean, passes.

I think we need to tweak the elog() below:

+ if (batch_size > PQ_QUERY_PARAM_MAX_LIMIT)
+ elog(WARNING, "postgres_fdw: batch_size %d is at or above the libpq "
+ "%d-parameter limit; the effective per-batch ceiling is "
+ "limit / number_of_columns and may be lower",
+ batch_size, PQ_QUERY_PARAM_MAX_LIMIT);

I think this should be an ereport() because it's the sort of error we'd
want the caller to see, and that means we need the message to conform the
guidelines at
https://www.postgresql.org/docs/current/error-style-guide.html, and I'm
going to suggest this as a starting point:

ereport(WARNING,
errmsg("%s of %d exceeds protocol limit of %d", "batch_size",
batch_size, PQ_QUERY_PARAM_MAX_LIMIT),
errdetail("The %s for a query will be reduced to protocol limit
divided by the number of columns in the query.", "batch_size"));

Done.

I'd like to hear other people's opinions on what the proper conforming
error message would be.

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

--
Regards,
Nurlan

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

Hi Rafia,

Wouldn't that give duplicate warnings with the changes already in the

patch...or you are suggesting to have it only in postgres_fdw_validator() ?

Yes, after thinking about it some more, I think it would be better to emit
the WARNING only from postgres_fdw_validator() when the option is SET
(CREATE/ALTER), while still allowing values greater than
PQ_QUERY_PARAM_MAX_LIMIT to be stored.

This also seems consistent with Corey's earlier suggestion:

I'm saying that we may want a separate check when the batch_size option is
SET (via CREATE/ALTER TABLE/SERVER) to test the value against
PQ_QUERY_PARAM_MAX_LIMIT
and issue a NOTICE/WARNING that the chosen value will always have to be set
at or below $PQ_QUERY_PARAM_MAX_LIMIT - but still let them store the values
as-is.

The WARNING currently emitted during INSERT seems potentially noisy, since
it may be repeated every time the foreign table is used.

Nit: since this table is used to verify that no warning is emitted:

+-- Verify that no WARNING is emitted when batch_size is within the
+-- libpq 65535-parameter limit.
+CREATE TABLE batch_warn_table ( x int );

Would batch_no_warn_table be a clearer name? The current name may be
slightly confusing in this context.

On Thu, Jul 16, 2026 at 10:51 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Fri, 17 Jul 2026 at 18:39, Nurlan Tulemisov <nurlan.tulemisov@gmail.com>
wrote:

Hi Rafia,

Wouldn't that give duplicate warnings with the changes already in the

patch...or you are suggesting to have it only in postgres_fdw_validator() ?

Yes, after thinking about it some more, I think it would be better to emit
the WARNING only from postgres_fdw_validator() when the option is SET
(CREATE/ALTER), while still allowing values greater than
PQ_QUERY_PARAM_MAX_LIMIT to be stored.

This also seems consistent with Corey's earlier suggestion:

I'm saying that we may want a separate check when the batch_size option is
SET (via CREATE/ALTER TABLE/SERVER) to test the value against
PQ_QUERY_PARAM_MAX_LIMIT
and issue a NOTICE/WARNING that the chosen value will always have to be
set
at or below $PQ_QUERY_PARAM_MAX_LIMIT - but still let them store the
values
as-is.

The WARNING currently emitted during INSERT seems potentially noisy, since
it may be repeated every time the foreign table is used.

Nit: since this table is used to verify that no warning is emitted:

+-- Verify that no WARNING is emitted when batch_size is within the
+-- libpq 65535-parameter limit.
+CREATE TABLE batch_warn_table ( x int );

Would batch_no_warn_table be a clearer name? The current name may be
slightly confusing in this context.

Thank you for the review, I have made both the changes in the attached

version.

On Thu, Jul 16, 2026 at 10:51 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Sat, 11 Jul 2026 at 23:56, Nurlan Tulemisov <
nurlan.tulemisov@gmail.com> wrote:

Hi,
I reviewed v5, applied it to the current master, and ran the
postgres_fdw regression tests. The patch applies cleanly and the tests pass.

Thank you for the review.

Would it also make sense to emit a NOTICE or WARNING from
postgres_fdw_validator() in option.c when batch_size is set to a value
greater than PQ_QUERY_PARAM_MAX_LIMIT?

Wouldn't that give duplicate warnings with the changes already in the

patch...or you are suggesting to have it only in postgres_fdw_validator() ?

That would notify the user immediately during CREATE or ALTER, while
still accepting the value and preserving the existing behavior.

The existing tests cover cases where the messages are emitted, but I
think it may also be useful to cover the exact boundaries and the cases
where no message should be emitted.

For a two-parameter foreign insert, batch_size = 32767 should not
produce a DEBUG1 message, while batch_size = 32768 should be reduced to
32767. Similarly, for a single parameter, batch_size = 65535 should not
produce a warning, while 65536 should.

Done.
Please find the updated patch attached.

Regards,
Nurlan

сб, 11 июл. 2026 г. в 21:13, Rafia Sabih <rafia.pghackers@gmail.com>:

On Mon, 22 Jun 2026 at 22:40, Corey Huinker <corey.huinker@gmail.com>
wrote:

On Fri, Jun 19, 2026 at 8:38 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Tue, 16 Jun 2026 at 22:20, Corey Huinker <corey.huinker@gmail.com>
wrote:

On Wed, Jun 10, 2026 at 5:09 AM Rafia Sabih <
rafia.pghackers@gmail.com> wrote:

On Tue, 9 Jun 2026 at 22:22, Corey Huinker <corey.huinker@gmail.com>
wrote:

Thanks for your inputs. Reworked patch is attached.
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

You've addressed all my concerns, aside from the desire for the
check on the set/update of the value. Do you have a commitfest entry? I
didn't find one.

There is commitfest entry now -->
https://commitfest.postgresql.org/patch/6873/

I've added myself as a reviewer. Did you want to try adding the
check at time of the option being set? If not, I can make an attempt at
that.

Please find the attached file for the patch with the warning message
at the time of batch_size option addition. Looking forward to your inputs.

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

Applies clean, passes.

I think we need to tweak the elog() below:

+ if (batch_size > PQ_QUERY_PARAM_MAX_LIMIT)
+ elog(WARNING, "postgres_fdw: batch_size %d is at or above the libpq "
+ "%d-parameter limit; the effective per-batch ceiling is "
+ "limit / number_of_columns and may be lower",
+ batch_size, PQ_QUERY_PARAM_MAX_LIMIT);

I think this should be an ereport() because it's the sort of error
we'd want the caller to see, and that means we need the message to conform
the guidelines at
https://www.postgresql.org/docs/current/error-style-guide.html, and
I'm going to suggest this as a starting point:

ereport(WARNING,
errmsg("%s of %d exceeds protocol limit of %d", "batch_size",
batch_size, PQ_QUERY_PARAM_MAX_LIMIT),
errdetail("The %s for a query will be reduced to protocol
limit divided by the number of columns in the query.", "batch_size"));

Done.

I'd like to hear other people's opinions on what the proper conforming
error message would be.

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

--
Regards,
Nurlan

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

Hi Rafia,

I reviewed v7. I have no further concerns with the current version, and the
patch looks ready to me.

On Mon, Jul 20, 2026 at 12:16 PM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Fri, 17 Jul 2026 at 18:39, Nurlan Tulemisov <nurlan.tulemisov@gmail.com>
wrote:

Hi Rafia,

Wouldn't that give duplicate warnings with the changes already in the

patch...or you are suggesting to have it only in postgres_fdw_validator() ?

Yes, after thinking about it some more, I think it would be better to
emit the WARNING only from postgres_fdw_validator() when the option is SET
(CREATE/ALTER), while still allowing values greater than
PQ_QUERY_PARAM_MAX_LIMIT to be stored.

This also seems consistent with Corey's earlier suggestion:

I'm saying that we may want a separate check when the batch_size option
is
SET (via CREATE/ALTER TABLE/SERVER) to test the value against
PQ_QUERY_PARAM_MAX_LIMIT
and issue a NOTICE/WARNING that the chosen value will always have to be
set
at or below $PQ_QUERY_PARAM_MAX_LIMIT - but still let them store the
values
as-is.

The WARNING currently emitted during INSERT seems potentially noisy,
since it may be repeated every time the foreign table is used.

Nit: since this table is used to verify that no warning is emitted:

+-- Verify that no WARNING is emitted when batch_size is within the
+-- libpq 65535-parameter limit.
+CREATE TABLE batch_warn_table ( x int );

Would batch_no_warn_table be a clearer name? The current name may be
slightly confusing in this context.

Thank you for the review, I have made both the changes in the attached

version.

On Thu, Jul 16, 2026 at 10:51 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Sat, 11 Jul 2026 at 23:56, Nurlan Tulemisov <
nurlan.tulemisov@gmail.com> wrote:

Hi,
I reviewed v5, applied it to the current master, and ran the
postgres_fdw regression tests. The patch applies cleanly and the tests pass.

Thank you for the review.

Would it also make sense to emit a NOTICE or WARNING from
postgres_fdw_validator() in option.c when batch_size is set to a value
greater than PQ_QUERY_PARAM_MAX_LIMIT?

Wouldn't that give duplicate warnings with the changes already in the

patch...or you are suggesting to have it only in postgres_fdw_validator() ?

That would notify the user immediately during CREATE or ALTER, while
still accepting the value and preserving the existing behavior.

The existing tests cover cases where the messages are emitted, but I
think it may also be useful to cover the exact boundaries and the cases
where no message should be emitted.

For a two-parameter foreign insert, batch_size = 32767 should not
produce a DEBUG1 message, while batch_size = 32768 should be reduced to
32767. Similarly, for a single parameter, batch_size = 65535 should not
produce a warning, while 65536 should.

Done.
Please find the updated patch attached.

Regards,
Nurlan

сб, 11 июл. 2026 г. в 21:13, Rafia Sabih <rafia.pghackers@gmail.com>:

On Mon, 22 Jun 2026 at 22:40, Corey Huinker <corey.huinker@gmail.com>
wrote:

On Fri, Jun 19, 2026 at 8:38 AM Rafia Sabih <
rafia.pghackers@gmail.com> wrote:

On Tue, 16 Jun 2026 at 22:20, Corey Huinker <corey.huinker@gmail.com>
wrote:

On Wed, Jun 10, 2026 at 5:09 AM Rafia Sabih <
rafia.pghackers@gmail.com> wrote:

On Tue, 9 Jun 2026 at 22:22, Corey Huinker <
corey.huinker@gmail.com> wrote:

Thanks for your inputs. Reworked patch is attached.
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

You've addressed all my concerns, aside from the desire for the
check on the set/update of the value. Do you have a commitfest entry? I
didn't find one.

There is commitfest entry now -->
https://commitfest.postgresql.org/patch/6873/

I've added myself as a reviewer. Did you want to try adding the
check at time of the option being set? If not, I can make an attempt at
that.

Please find the attached file for the patch with the warning message
at the time of batch_size option addition. Looking forward to your inputs.

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

Applies clean, passes.

I think we need to tweak the elog() below:

+ if (batch_size > PQ_QUERY_PARAM_MAX_LIMIT)
+ elog(WARNING, "postgres_fdw: batch_size %d is at or above the libpq
"
+ "%d-parameter limit; the effective per-batch ceiling is "
+ "limit / number_of_columns and may be lower",
+ batch_size, PQ_QUERY_PARAM_MAX_LIMIT);

I think this should be an ereport() because it's the sort of error
we'd want the caller to see, and that means we need the message to conform
the guidelines at
https://www.postgresql.org/docs/current/error-style-guide.html, and
I'm going to suggest this as a starting point:

ereport(WARNING,
errmsg("%s of %d exceeds protocol limit of %d", "batch_size",
batch_size, PQ_QUERY_PARAM_MAX_LIMIT),
errdetail("The %s for a query will be reduced to protocol
limit divided by the number of columns in the query.", "batch_size"));

Done.

I'd like to hear other people's opinions on what the proper
conforming error message would be.

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

--
Regards,
Nurlan

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

--
Regards,
Nurlan

On Tue, 21 Jul 2026 at 20:47, Nurlan Tulemisov <nurlan.tulemisov@gmail.com>
wrote:

Hi Rafia,

I reviewed v7. I have no further concerns with the current version, and
the patch looks ready to me.

Thanks for your time on this. Please mark it ready for committer then.

On Mon, Jul 20, 2026 at 12:16 PM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Fri, 17 Jul 2026 at 18:39, Nurlan Tulemisov <
nurlan.tulemisov@gmail.com> wrote:

Hi Rafia,

Wouldn't that give duplicate warnings with the changes already in the

patch...or you are suggesting to have it only in postgres_fdw_validator() ?

Yes, after thinking about it some more, I think it would be better to
emit the WARNING only from postgres_fdw_validator() when the option is SET
(CREATE/ALTER), while still allowing values greater than
PQ_QUERY_PARAM_MAX_LIMIT to be stored.

This also seems consistent with Corey's earlier suggestion:

I'm saying that we may want a separate check when the batch_size option
is
SET (via CREATE/ALTER TABLE/SERVER) to test the value against
PQ_QUERY_PARAM_MAX_LIMIT
and issue a NOTICE/WARNING that the chosen value will always have to be
set
at or below $PQ_QUERY_PARAM_MAX_LIMIT - but still let them store the
values
as-is.

The WARNING currently emitted during INSERT seems potentially noisy,
since it may be repeated every time the foreign table is used.

Nit: since this table is used to verify that no warning is emitted:

+-- Verify that no WARNING is emitted when batch_size is within the
+-- libpq 65535-parameter limit.
+CREATE TABLE batch_warn_table ( x int );

Would batch_no_warn_table be a clearer name? The current name may be
slightly confusing in this context.

Thank you for the review, I have made both the changes in the attached

version.

On Thu, Jul 16, 2026 at 10:51 AM Rafia Sabih <rafia.pghackers@gmail.com>
wrote:

On Sat, 11 Jul 2026 at 23:56, Nurlan Tulemisov <
nurlan.tulemisov@gmail.com> wrote:

Hi,
I reviewed v5, applied it to the current master, and ran the
postgres_fdw regression tests. The patch applies cleanly and the tests pass.

Thank you for the review.

Would it also make sense to emit a NOTICE or WARNING from
postgres_fdw_validator() in option.c when batch_size is set to a value
greater than PQ_QUERY_PARAM_MAX_LIMIT?

Wouldn't that give duplicate warnings with the changes already in the

patch...or you are suggesting to have it only in postgres_fdw_validator() ?

That would notify the user immediately during CREATE or ALTER, while
still accepting the value and preserving the existing behavior.

The existing tests cover cases where the messages are emitted, but I
think it may also be useful to cover the exact boundaries and the cases
where no message should be emitted.

For a two-parameter foreign insert, batch_size = 32767 should not
produce a DEBUG1 message, while batch_size = 32768 should be reduced to
32767. Similarly, for a single parameter, batch_size = 65535 should not
produce a warning, while 65536 should.

Done.
Please find the updated patch attached.

Regards,
Nurlan

сб, 11 июл. 2026 г. в 21:13, Rafia Sabih <rafia.pghackers@gmail.com>:

On Mon, 22 Jun 2026 at 22:40, Corey Huinker <corey.huinker@gmail.com>
wrote:

On Fri, Jun 19, 2026 at 8:38 AM Rafia Sabih <
rafia.pghackers@gmail.com> wrote:

On Tue, 16 Jun 2026 at 22:20, Corey Huinker <
corey.huinker@gmail.com> wrote:

On Wed, Jun 10, 2026 at 5:09 AM Rafia Sabih <
rafia.pghackers@gmail.com> wrote:

On Tue, 9 Jun 2026 at 22:22, Corey Huinker <
corey.huinker@gmail.com> wrote:

Thanks for your inputs. Reworked patch is attached.
--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

You've addressed all my concerns, aside from the desire for the
check on the set/update of the value. Do you have a commitfest entry? I
didn't find one.

There is commitfest entry now -->
https://commitfest.postgresql.org/patch/6873/

I've added myself as a reviewer. Did you want to try adding the
check at time of the option being set? If not, I can make an attempt at
that.

Please find the attached file for the patch with the warning
message at the time of batch_size option addition. Looking forward to your
inputs.

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

Applies clean, passes.

I think we need to tweak the elog() below:

+ if (batch_size > PQ_QUERY_PARAM_MAX_LIMIT)
+ elog(WARNING, "postgres_fdw: batch_size %d is at or above the
libpq "
+ "%d-parameter limit; the effective per-batch ceiling is "
+ "limit / number_of_columns and may be lower",
+ batch_size, PQ_QUERY_PARAM_MAX_LIMIT);

I think this should be an ereport() because it's the sort of error
we'd want the caller to see, and that means we need the message to conform
the guidelines at
https://www.postgresql.org/docs/current/error-style-guide.html, and
I'm going to suggest this as a starting point:

ereport(WARNING,
errmsg("%s of %d exceeds protocol limit of %d",
"batch_size", batch_size, PQ_QUERY_PARAM_MAX_LIMIT),
errdetail("The %s for a query will be reduced to protocol
limit divided by the number of columns in the query.", "batch_size"));

Done.

I'd like to hear other people's opinions on what the proper
conforming error message would be.

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

--
Regards,
Nurlan

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH

--
Regards,
Nurlan

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH