Re: [PROPOSAL] : Disallow use of empty column name in (column_name '') in ALTER or CREATE of foreign table.

From: Nishant Sharma <nishant(dot)sharma(at)enterprisedb(dot)com>
To: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-hackers(at)postgresql(dot)org
Subject: Re: [PROPOSAL] : Disallow use of empty column name in (column_name '') in ALTER or CREATE of foreign table.
Date: 2024-08-22 10:30:13
Message-ID: CADrsxda_OgW8FzFCjCgbF3sUy=Ke+bn1gqAROcvVdu26oTBxbw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Thanks Tom and Ashutosh for your responses!

I also agree that, v1 patch set was applying SQL syntax restrictions to all
FDWs,
which is not reasonable.

PFA v2 patch set. This is based on the suggestion given by Ashutosh to have
the
check in postgres_fdw validator. As it fits to apply the SQL syntax
restriction only
to FDWs which follow SQL syntax restrictions.
With this change, it gives hint/idea to any user using PostgresSQL with
their own
FDWs to add this check in their equivalent fdw validator.

Regarding, 'empty' vs 'misspelled' remote column name, from my point of
view,
I see some differences between them:-
1. SYNTAX wise - SQL syntax has restrictions for not allowing creating
column
names as empty. And it does not bother about, whether user used a misspelled
word or not for the column name.
2. IMPLEMENTATION wise - we don't need any extra info to decide that the
column
name received from command is empty, but we do need column name infos from
remote server to decide whether user misspelled the remote column name,
which
not only applies to the column_name options, but maybe to the column names
used
while creating the table syntax for foreign tables if the fdw column_name
option is
not added. Ex:- "CREATE FOREIGN TABLE test_fdw(name VARCHAR(15),
id VARCHAR(5)) ...." - to 'name' and 'id' here.

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.

I ran "make check world" and do not see any failure related to patches.
But, I do see
an existing failure "t/001_pgbench_with_server.pl".

Regards,
Nishant Sharma
EnterpriseDB, Pune.

On Mon, Aug 19, 2024 at 4:27 PM Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
wrote:

> On Fri, Aug 16, 2024 at 8:26 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> >
> > Nishant Sharma <nishant(dot)sharma(at)enterprisedb(dot)com> writes:
> > > Actual column names used while creation of foreign table are not
> allowed to
> > > be an
> > > empty string, but when we use column_name as an empty string in OPTIONS
> > > during
> > > CREATE or ALTER of foreign tables, it is allowed.
> >
> > Is this really a bug? The valid remote names are determined by
> > whatever underlies the FDW, and I doubt we should assume that
> > SQL syntax restrictions apply to every FDW. Perhaps it would
> > be reasonable to apply such checks locally in SQL-based FDWs,
> > but I object to assuming such things at the level of
> > ATExecAlterColumnGenericOptions.
>
> I agree.
>
> >
> > More generally, I don't see any meaningful difference between
> > this mistake and the more common one of misspelling the remote
> > column name, which is something we're not going to be able
> > to check for (at least not in anything like this way). If
> > you wanted to move the ease-of-use goalposts materially,
> > you should be looking for a way to do that.
>
> I think this check should be delegated to an FDW validator.
>
> --
> Best Wishes,
> Ashutosh Bapat
>

Attachment Content-Type Size
v2-0001-Disallow-empty-Foreign-Table-column-option-name-i.patch application/octet-stream 1.1 KB
v2-0002-Test-Cases-Changes.patch application/octet-stream 2.8 KB

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2024-08-22 10:32:41 Re: Segfault in jit tuple deforming on arm64 due to LLVM issue
Previous Message shveta malik 2024-08-22 10:14:50 Re: Conflict Detection and Resolution