RE: [HACKERS] Proposal to add work_mem option to postgres_fdw module

>I have committed just the libpq change. The documentation change was redundant, because the documentation already stated that all libpq options are accepted. (Arguably, the documentation was wrong before.) Also, the proposed test change didn't >seem to add much. It just checked that the foreign server option is accepted, but not whether it does anything. If you want to develop a more substantive test, we could consider it, but I feel that since this all just goes to libpq, we don't need to test it >further.

Thanks !

Regards.

Noriyoshi Shinoda

-----Original Message-----
From: Peter Eisentraut [mailto:peter(dot)eisentraut(at)2ndquadrant(dot)com]
Sent: Friday, September 7, 2018 10:17 PM
To: Shinoda, Noriyoshi (PN Japan GCS Delivery) <noriyoshi(dot)shinoda(at)hpe(dot)com>; fabriziomello(at)gmail(dot)com; [pgdg] Robert Haas <robertmhaas(at)gmail(dot)com>; michael(at)paquier(dot)xyz
Cc: Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [HACKERS] Proposal to add work_mem option to postgres_fdw module

On 05/09/2018 18:46, Peter Eisentraut wrote:
> On 01/09/2018 06:33, Shinoda, Noriyoshi (PN Japan GCS Delivery) wrote:
>> Certainly the PQconndefaults function specifies Debug flag for the "options" option.
>> I think that eliminating the Debug flag is the simplest solution.
>> For attached patches, GUC can be specified with the following syntax.
>>
>> CREATE SERVER remsvr1 FOREIGN DATA WRAPPER postgres_fdw OPTIONS (host
>> 'host 1', ..., options '-c work_mem=64MB -c geqo=off'); ALTER SERVER
>> remsv11 OPTIONS (SET options '-c work_mem=64MB -c geqo=off');
>>
>> However, I am afraid of the effect that this patch will change the behavior of official API PQconndefaults.
>
> It doesn't change the behavior of the API, it just changes the result
> of the API function, which is legitimate and the reason we have the
> API function in the first place.
>
> I think this patch is fine. I'll work on committing it.

I have committed just the libpq change. The documentation change was redundant, because the documentation already stated that all libpq options are accepted. (Arguably, the documentation was wrong before.) Also, the proposed test change didn't seem to add much. It just checked that the foreign server option is accepted, but not whether it does anything. If you want to develop a more substantive test, we could consider it, but I feel that since this all just goes to libpq, we don't need to test it further.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services