Re: postgres_fdw: Emit message when batch_size is reduced

On Tue, 21 Jul 2026 at 20:47, Nurlan Tulemisov <nurlan(dot)tulemisov(at)gmail(dot)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(dot)pghackers(at)gmail(dot)com>
> wrote:
>
>>
>>
>> On Fri, 17 Jul 2026 at 18:39, Nurlan Tulemisov <
>> nurlan(dot)tulemisov(at)gmail(dot)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(dot)pghackers(at)gmail(dot)com>
>>> wrote:
>>>
>>>>
>>>>
>>>> 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
>>>>
>>>
>>
>> --
>> Regards,
>> Rafia Sabih
>> CYBERTEC PostgreSQL International GmbH
>>
>
>
> --
> Regards,
> Nurlan
>

--
Regards,
Rafia Sabih
CYBERTEC PostgreSQL International GmbH