Re: postgres_fdw: Emit message when batch_size is reduced

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(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