Re: patch: SQL/MED(FDW) DDL

Thanks for your comment. I've updated the patches.

> I checked the patch. It includes changes for DDL, system catalogs,
> information schema, and basic psql support. The patch itself have no
useful
> works, but we need the parts anyway to support the SQL standard.

As you mentioned, this patch provides
CREATE/ALTER/DROP FOREIGNTABLE functionality only.
Please review this patch from the viewpoints:
- confirming level to the SQL standard
- features which should be supported on foreign table
- WITH OIDS
- DEFAULT
- constraints (currently NOT NULL and CHECK are supported)
- etc.

SELECTing on foreign tables functionality will be provided
by the fdw_select patch, as attached.

> * There are unused types in the patch.
Fixed (definitions are moved to the other patch).

> * Needs an error check to SELECT FROM foreign table.
Fixed. I've added relkind check in set_plain_rel_pathlist(),
which aborts query with elog(ERROR) if the relation was a foreign table.
This fix also checks for DELETE, UPDATE and EXPLAIN SELECT.

> * Type checks between TABLE and FOREIGN TABLE are a bit inconsistent.
> For example, "ALTER TABLE ADD COLUMN" can add a column to a
foreign tables
> but "DROP TABLE" cannot remove foreign tables.
> IMHO, however, we can allow such looseness because operations actually
> forbidden will end with ERRORs without problems.

In current implementation, we allow some kind of operations to
ALTER TABLE which can be done on views on foreign tables (i.e.
ALTER TABLE RENAME TO).
But as Itagaki-san says, the inconsistency will not cause serious
problem.

Regards,

(2010/09/17 11:29), Itagaki Takahiro wrote:it.
>
> I checked the patch. It includes changes for DDL, system catalogs,
> information schema, and basic psql support. The patch itself have no useful
> works, but we need the parts anyway to support the SQL standard.
>
> I have a couples of comments:
>
> * There are unused types in the patch. They might be used by additional
> patches based on the patch, but should be removed for now.
> - enum GenericOptionFlags.ForeignTableOpt
> - typedef struct FSConnection FSConnection;
> - typedef struct FdwRoutine FdwRoutine;
> - typedef struct FdwReply FdwReply;
>
> * Needs an error check to SELECT FROM foreign table. It might be replaced
> to actual FDW routines soon, but the current error message is not ideal.
> postgres=# SELECT * FROM ft1;
> ERROR: could not open file "base/11908/16391": No such file or directory
>
> * Type checks between TABLE and FOREIGN TABLE are a bit inconsistent.
> For example, "ALTER TABLE ADD COLUMN" can add a column to a foreign tables
> but "DROP TABLE" cannot remove foreign tables.
> IMHO, however, we can allow such looseness because operations actually
> forbidden will end with ERRORs without problems.
>