Re: Improve the HINT message of the ALTER command for postgres_fdw

On 2021/10/12 19:57, bt21masumurak wrote:
> I made new patch based on those comments.

Thanks for updating the patch!

-                                        errhint("HOGEHOGEValid options in this context are: %s",
-                                                        buf.data)));

The patch contains the garbage "HOGEHOGE" in the above,
which causes the compiler to fail. Attached is the updated version
of the patch. I got rid of the garbage.


+--HINT test
+ALTER FOREIGN DATA WRAPPER file_fdw OPTIONS (format 'csv');

file_fdw already has the test for ALTER FOREIGN DATA WRAPPER .. OPTIONS,
so you don't need to add new test for the command into file_fdw.
I removed that test from file_fdw.


Also I moved the tests for ALTER FOREIGN DATA WRAPPER .. OPTIONS,
in the tests of postgres_fdw, dblink, and foreign data, into more proper
places.


BTW, I found file_fdw.c, dblink.c, postgres_fdw/option.c and foreign.c
use different error codes for the same error message as follows.
They should use the same error code? If yes, ISTM that
ERRCODE_FDW_INVALID_OPTION_NAME is better because
the error message is "invalid option ...".

- ERRCODE_FDW_INVALID_OPTION_NAME (file_fdw.c)
- ERRCODE_FDW_OPTION_NAME_NOT_FOUND (dblink.c)
- ERRCODE_FDW_INVALID_OPTION_NAME (postgres_fdw/option.c)
- ERRCODE_SYNTAX_ERROR (foreign.c)

Regards,

-- 
Fujii Masao
Advanced Computing Technology Center
Research and Development Headquarters
NTT DATA CORPORATION