On Mon, Oct 7, 2024 at 10:16 AM Michael Paquier <mich...@paquier.xyz> wrote:
> On Thu, Aug 22, 2024 at 04:00:13PM +0530, Nishant Sharma wrote:
> > I may be wrong, but just wanted to share my thoughts on the differences.
> > So, it
> > can be considered a different issue/mistake and can be handled
> separately in
> > another email thread.
>
> + else if (strcmp(def->defname, "column_name") == 0)
> + {
> + char *col_name_opt = defGetString(def);
> +
> + /*
> + * PostgresSQL follows SQL syntax, so we do not allow empty
> + * column_name option.
> + */
> + if (col_name_opt && col_name_opt[0] == '\0')
> + ereport(ERROR,
> + (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
> + errmsg("colum_name option cannot be empty for
> postgres_fdw")));
> + }
>
> If we begin to care about empty names in column_name in the FDW
> command, shouldn't we also care about empry values in schema_name and
> table_name?
>
> Typos: PostgresSQL -> PostgreSQL and colum_name -> column_name.
>
> Once you associate table_name and schema_name, you can save in
> translation by rewording the errmsg like that (no need to mention
> postgres_fdw, note the quotes around the option name):
> errmsg("cannot use empty value for option \"%s\"",
> "column_name");
> --
> Michael
>
Thanks Michael for your review comments and response!
I have included them in v3. v3 does not allow empty schema_name &
table_name options along with column_name.
Thanks,
Nishant.
v3-0002-Test-Cases-Changes.patch
Description: Binary data
v3-0001-Disallow-empty-Foreign-Table-column-option-name-i.patch
Description: Binary data
