On Mon, Oct 25, 2021 at 12:00 PM Fujii Masao <masao.fu...@oss.nttdata.com> wrote: > > On 2021/10/16 19:43, Bharath Rupireddy wrote: > > I'm fine with the distinction that's made, now I'm thinking about the > > appropriate areas where ERRCODE_FDW_INVALID_OPTION_NAME can be used. > > Is it correct to use ERRCODE_FDW_INVALID_OPTION_NAME in > > postgresImportForeignSchema where we don't check buffer length and > > option name length but throw the error when we don't find what's being > > expected for IMPORT FOREIGN SCHEMA command? Isn't it the > > ERRCODE_FDW_OPTION_NAME_NOT_FOUND right choice there? I've seen some > > of the option parsing logic(with the search text "stmt->options)" in > > the code base), they are mostly using "option \"%s\" not recognized" > > without an error code or "unrecognized XXXX option \"%s\"" with > > ERRCODE_SYNTAX_ERROR. I'm not sure which is right. If not in > > postgresImportForeignSchema, where else can > > ERRCODE_FDW_INVALID_OPTION_NAME be used? > > These are good questions. But TBH I don't know the answers and have not > found good articles describing more detail definitions of those error codes. > I'm tempted to improve the HINT message part at first because it has > obviously an issue. And then we can consider what error code should be > used in FDW layer if necessary.
Yeah, let's focus on fixing the hint message here and the alter_foreign_data_wrapper_options_v3.patch LGTM. Why didn't we have a test case for file_fdw? It looks like the file_fdw contrib module doesn't have any test cases in its sql directory. I'm not sure if the module code is being covered in someother tests. Regards, Bharath Rupireddy.
