On Mon, Oct 25, 2021 at 12:00 PM Fujii Masao
<masao.fujii@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.