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