Request a Demo
Home   >   mailing lists

Re: postgres_fdw: Emit message when batch_size is reduced - Mailing list pgsql-hackers

From Nurlan Tulemisov
Subject Re: postgres_fdw: Emit message when batch_size is reduced
Date
Msg-id CALCiY5M6fwMN2J1V_P1SSSESUMux8-tfhk_eU1sY4GZd2JUJEg@mail.gmail.com
Whole thread
In response to Re: postgres_fdw: Emit message when batch_size is reduced  (Rafia Sabih <rafia.pghackers@gmail.com>)
Responses Re: postgres_fdw: Emit message when batch_size is reduced
List pgsql-hackers
Tree view

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

pgsql-hackers by date:

Previous
From: Fujii Masao
Date:
Subject: Clarify or fix SIGINT handling in data checksums launcher
Next
From: Fujii Masao
Date:
Subject: Re: allow spread checkpoints when changing checksums online