Re: postgres_fdw: Emit message when batch_size is reduced

On Sat, 11 Jul 2026 at 23:56, Nurlan Tulemisov <nurlan(dot)tulemisov(at)gmail(dot)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(dot)pghackers(at)gmail(dot)com>:
>
>>
>>
>> On Mon, 22 Jun 2026 at 22:40, Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
>> wrote:
>>
>>> On Fri, Jun 19, 2026 at 8:38 AM Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com>
>>> wrote:
>>>
>>>>
>>>>
>>>> On Tue, 16 Jun 2026 at 22:20, Corey Huinker <corey(dot)huinker(at)gmail(dot)com>
>>>> wrote:
>>>>
>>>>> On Wed, Jun 10, 2026 at 5:09 AM Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com>
>>>>> wrote:
>>>>>
>>>>>>
>>>>>>
>>>>>> On Tue, 9 Jun 2026 at 22:22, Corey Huinker <corey(dot)huinker(at)gmail(dot)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