> Fix CI failure of doc build in v1 patch.
Thanks for the patch! I am +1 for this, but I have a few comments:
1/ In the IDENTITY case, the remote side may not be
able to handle the DEFAULT value. See the example below:
-- on the foreign server
postgres=# CREATE TABLE t2 (id int, c1 text);
CREATE TABLE
-- on the local server
postgres=#
postgres=# CREATE TABLE t1 (id int GENERATED ALWAYS AS IDENTITY, c1 text);
CREATE TABLE
postgres=# CREATE FOREIGN TABLE t2 (LIKE t1 INCLUDING INDEXES) server r1;
CREATE FOREIGN TABLE
postgres=# INSERT INTO t2 (c1) VALUES ('A');
INSERT 0 1
postgres=# SELECT * FROM t2;
id | c1
----+----
| A
(1 row)
This is also the reason foreign tables don't document IDENTITY as valid [1].
It may even be a bug for it to be allowed with the CREATE FOREIGN TABLE
syntax:
postgres=# CREATE FOREIGN TABLE t3 (id int GENERATED ALWAYS AS
IDENTITY, c1 text) server r1;
CREATE FOREIGN TABLE
postgres=# \d+ t3
Foreign table "public.t3"
Column | Type | Collation | Nullable | Default
| FDW options | Storage | Stats target | Description
--------+---------+-----------+----------+------------------------------+-------------+----------+--------------+-------------
id | integer | | not null | generated always as
identity | | plain | |
c1 | text | | |
| | extended | |
Not-null constraints:
"t3_id_not_null" NOT NULL "id"
Server: r1
2/ As IDENTITY, STORAGE is also allowed on foreign tables, which
does not make much sense either, as the fdw may not be Postgres,
or if it is Postgres, it may have a different STORAGE setting. This is
also not documented. I am inclined to say to not include it either.
I think we should not allow IDENTITY and STORAGE in this patch
as they are not documented [1] as is, and perhaps a separate discussion
to correct the behavior for the CREATE FOREIGN TABLE case.
3/ a minor nit on the comments.
instead of
+ Foreign tables have no real storage in PostgreSQL.
Can it just be
Foreign tables have no storage in PostgreSQL.
the "real" is not needed.
[1] https://www.postgresql.org/docs/current/sql-createforeigntable.html
--
Regards,
Sami
