Re: Improve the HINT message of the ALTER command for postgres_fdw - Mailing list pgsql-hackers

From bt21masumurak
Subject Re: Improve the HINT message of the ALTER command for postgres_fdw
Date
Msg-id 46864d7a8ca6c2faeb2f9a7a401d47c0@oss.nttdata.com
Whole thread Raw
In response to Re: Improve the HINT message of the ALTER command for postgres_fdw  (Daniel Gustafsson <daniel@yesql.se>)
Responses Re: Improve the HINT message of the ALTER command for postgres_fdw
List pgsql-hackers
Hi
Thank you for your comments.

>> It seems like the change proposed for postgres_fdw_validator is 
>> similar to what
>> file_fdw is doing in file_fdw_validator.  I think we also need to do 
>> the same
>> change in dblink_fdw_validator and postgresql_fdw_validator as well.
> 
> Agreed.
> 
>> While on this, it's better to add test cases for the error message
>> "There are no valid options in this context." for all the three fdws
>> i.e. file_fdw, postgres_fdw and dblink_fdw may be in their respective
>> contrib modules test files, and for postgresql_fdw_validator in
>> src/test/regress/sql/foreign_data.sql.
> 
> +1.

I made new patch based on those comments.

Regards,
Kosei Masumura


2021-10-08 17:13 に Daniel Gustafsson さんは書きました:
>> On 8 Oct 2021, at 09:38, Bharath Rupireddy 
>> <bharath.rupireddyforpostgres@gmail.com> wrote:
>> 
>> On Fri, Oct 8, 2021 at 12:48 PM bt21masumurak
>> <bt21masumurak@oss.nttdata.com> wrote:
>>> 
>>> Hi
>>> 
>>> When 'ALTER FOREIGN DATA WRAPPER OPTIONS' is executed against
>>> postgres_fdw, the HINT message is printed as shown below, even though
>>> there are no valid options in this context.
>>> 
>>> =# ALTER FOREIGN DATA WRAPPER postgres_fdw OPTIONS (format 'csv');
>>> ERROR: invalid option "format"
>>> HINT: Valid options in this context are:
>>> 
>>> I made a patch for this problem.
>> 
>> Good catch.
> 
> +1
> 
>> It seems like the change proposed for postgres_fdw_validator is 
>> similar to what
>> file_fdw is doing in file_fdw_validator.  I think we also need to do 
>> the same
>> change in dblink_fdw_validator and postgresql_fdw_validator as well.
> 
> Agreed.
> 
>> While on this, it's better to add test cases for the error message
>> "There are no valid options in this context." for all the three fdws
>> i.e. file_fdw, postgres_fdw and dblink_fdw may be in their respective
>> contrib modules test files, and for postgresql_fdw_validator in
>> src/test/regress/sql/foreign_data.sql.
> 
> +1.
> 
> --
> Daniel Gustafsson        https://vmware.com/

Attachment

pgsql-hackers by date:

Previous
From: Alexander Kuzmenkov
Date:
Subject: preserve timestamps when installing headers
Next
From: Greg Nancarrow
Date:
Subject: Re: Skipping logical replication transactions on subscriber side