| From: | Corey Huinker <corey(dot)huinker(at)gmail(dot)com> |
|---|---|
| To: | Rafia Sabih <rafia(dot)pghackers(at)gmail(dot)com> |
| Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: postgres_fdw: Emit message when batch_size is reduced |
| Date: | 2026-06-22 20:40:16 |
| Message-ID: | CADkLM=ezMQxu10rSbDY77MZLkezFEhPcQuPGA7T3aAnxi8wczQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
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"));
I'd like to hear other people's opinions on what the proper conforming
error message would be.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Peter Geoghegan | 2026-06-22 20:48:10 | Re: index prefetching |
| Previous Message | Bruce Momjian | 2026-06-22 20:37:45 | Re: The PostgreSQL C Dialect |