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

Hi,


--------------------------------------------------------------------------------------------------------------
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.

EXAMPLES:-
1) CREATE FOREIGN TABLE test_fdw("" VARCHAR(15), id VARCHAR(5)) SERVER localhost_fdw OPTIONS (schema_name 'public', table_name 'test');
ERROR:  zero-length delimited identifier at or near """"
LINE 1: CREATE FOREIGN TABLE test_fdw("" VARCHAR(15), id VARCHAR(5))...

2) CREATE FOREIGN TABLE test_fdw(name VARCHAR(15) OPTIONS (column_name ''), id VARCHAR(5)) SERVER localhost_fdw OPTIONS (schema_name 'public', table_name 'test');
CREATE FOREIGN TABLE

postgres@43832=#\d test_fdw
                          Foreign table "public.test_fdw"
 Column |         Type          | Collation | Nullable | Default |   FDW options    
--------+-----------------------+-----------+----------+---------+------------------
 name   | character varying(15) |           |          |         | (column_name '')
 id     | character varying(5)  |           |          |         |
Server: localhost_fdw
FDW options: (schema_name 'public', table_name 'test')

--------------------------------------------------------------------------------------------------------------

Due to the above, when we try to simply select a remote table, the remote query uses
the empty column name from the FDW column option and the select fails.

EXAMPLES:-
1) select * from test_fdw;
ERROR:  zero-length delimited identifier at or near """"
CONTEXT:  remote SQL command: SELECT "", id FROM public.test

2) explain verbose select * from test_fdw;
                                QUERY PLAN                                
--------------------------------------------------------------------------
 Foreign Scan on public.test_fdw  (cost=100.00..297.66 rows=853 width=72)
   Output: name, id
   Remote SQL: SELECT "", id FROM public.test
(3 rows)

--------------------------------------------------------------------------------------------------------------

We can fix this issue either during fetching of FDW column option names while
building remote query or we do not allow at CREATE or ALTER of foreign tables itself.
We think it would be better to disallow adding the column_name option as empty in
CREATE or ALTER itself as we do not allow empty actual column names for a foreign
table. Unless I missed to understand the purpose of allowing column_name as empty.

THE PROPOSED SOLUTION OUTPUT:-
1) CREATE FOREIGN TABLE test_fdw(name VARCHAR(15) OPTIONS (column_name ''), id VARCHAR(5)) SERVER localhost_fdw OPTIONS (schema_name 'public', table_name 'test');
ERROR:  column generic option name cannot be empty

2) CREATE FOREIGN TABLE test_fdw(name VARCHAR(15), id VARCHAR(5)) SERVER localhost_fdw OPTIONS (schema_name 'public', table_name 'test');
CREATE FOREIGN TABLE

ALTER FOREIGN TABLE test_fdw ALTER COLUMN id OPTIONS (column_name '');
ERROR:  column generic option name cannot be empty

--------------------------------------------------------------------------------------------------------------

PFA, the fix and test cases patches attached. 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.

P.S
Thanks to Jeevan Chalke and Suraj Kharage for their inputs for the proposal.
Nishant Sharma <nishant.sharma@enterprisedb.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.

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.

            regards, tom lane



On Fri, Aug 16, 2024 at 8:26 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>
> Nishant Sharma <nishant.sharma@enterprisedb.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