[9.3] Automatically updatable views vs writable foreign tables
Hi,
I've just started 9.3 beta testing and I noticed that a "simple" view
defined on top of a writable foreign table is not automatically
updatable.
Given that these are both new-to-9.3 features, I think it would be a
shame if they don't work together. It's basically a 1-line patch to
make such views automatically updatable, plus a small extra code block
in relation_is_updatable() to reflect the change in the
information_schema views.
The attached patch does that and adds a couple of extra regression tests.
The tests, however, reveal a separate issue with writable foreign
tables --- the information_schema views haven't been updated to
reflect the fact that foreign tables may now be updatable.
Specifically, for foreign tables
information_schema.tables.is_insertable_into and
information_schema.columns.is_updatable always say 'NO' even if the
foreign table is writable. Fixing that would require new C functions
along the same lines as pg_view_is_insertable/updatable(), or those
functions could just be renamed and repurposed to do the check for all
relation kinds, except those known to be always/never updatable.
Regards,
Dean
Attachments:
writable-fdw-view.patchapplication/octet-stream; name=writable-fdw-view.patchDownload
diff --git a/contrib/file_fdw/input/file_fdw.source b/contrib/file_fdw/input/file_fdw.source
new file mode 100644
index f7fd28d..063e23c
*** a/contrib/file_fdw/input/file_fdw.source
--- b/contrib/file_fdw/input/file_fdw.source
*************** DELETE FROM agg_csv WHERE a = 100;
*** 121,126 ****
--- 121,148 ----
-- but this should be ignored
SELECT * FROM agg_csv FOR UPDATE;
+ -- simple views on top of the foreign table aren't updatable
+ CREATE VIEW agg_csv_v AS SELECT * FROM agg_csv;
+
+ SELECT table_name, is_insertable_into
+ FROM information_schema.tables
+ WHERE table_name LIKE 'agg_csv%'
+ ORDER BY table_name;
+
+ SELECT table_name, is_updatable, is_insertable_into
+ FROM information_schema.views
+ WHERE table_name LIKE 'agg_csv%'
+ ORDER BY table_name;
+
+ SELECT table_name, column_name, is_updatable
+ FROM information_schema.columns
+ WHERE table_name LIKE 'agg_csv%'
+ ORDER BY table_name, ordinal_position;
+
+ INSERT INTO agg_csv_v VALUES(1,2.0);
+ UPDATE agg_csv_v SET a = 1;
+ DELETE FROM agg_csv_v WHERE a = 100;
+
-- privilege tests
SET ROLE file_fdw_superuser;
SELECT * FROM agg_text ORDER BY a;
diff --git a/contrib/file_fdw/output/file_fdw.source b/contrib/file_fdw/output/file_fdw.source
new file mode 100644
index 4f90bae..2e015cc
*** a/contrib/file_fdw/output/file_fdw.source
--- b/contrib/file_fdw/output/file_fdw.source
*************** SELECT * FROM agg_csv FOR UPDATE;
*** 199,204 ****
--- 199,243 ----
42 | 324.78
(3 rows)
+ -- simple views on top of the foreign table aren't updatable
+ CREATE VIEW agg_csv_v AS SELECT * FROM agg_csv;
+ SELECT table_name, is_insertable_into
+ FROM information_schema.tables
+ WHERE table_name LIKE 'agg_csv%'
+ ORDER BY table_name;
+ table_name | is_insertable_into
+ ------------+--------------------
+ agg_csv | NO
+ agg_csv_v | NO
+ (2 rows)
+
+ SELECT table_name, is_updatable, is_insertable_into
+ FROM information_schema.views
+ WHERE table_name LIKE 'agg_csv%'
+ ORDER BY table_name;
+ table_name | is_updatable | is_insertable_into
+ ------------+--------------+--------------------
+ agg_csv_v | NO | NO
+ (1 row)
+
+ SELECT table_name, column_name, is_updatable
+ FROM information_schema.columns
+ WHERE table_name LIKE 'agg_csv%'
+ ORDER BY table_name, ordinal_position;
+ table_name | column_name | is_updatable
+ ------------+-------------+--------------
+ agg_csv | a | NO
+ agg_csv | b | NO
+ agg_csv_v | a | NO
+ agg_csv_v | b | NO
+ (4 rows)
+
+ INSERT INTO agg_csv_v VALUES(1,2.0);
+ ERROR: cannot insert into foreign table "agg_csv"
+ UPDATE agg_csv_v SET a = 1;
+ ERROR: cannot update foreign table "agg_csv"
+ DELETE FROM agg_csv_v WHERE a = 100;
+ ERROR: cannot delete from foreign table "agg_csv"
-- privilege tests
SET ROLE file_fdw_superuser;
SELECT * FROM agg_text ORDER BY a;
*************** SET ROLE file_fdw_superuser;
*** 243,255 ****
-- cleanup
RESET ROLE;
DROP EXTENSION file_fdw CASCADE;
! NOTICE: drop cascades to 8 other objects
DETAIL: drop cascades to server file_server
drop cascades to user mapping for file_fdw_user
drop cascades to user mapping for file_fdw_superuser
drop cascades to user mapping for no_priv_user
drop cascades to foreign table agg_text
drop cascades to foreign table agg_csv
drop cascades to foreign table agg_bad
drop cascades to foreign table text_csv
DROP ROLE file_fdw_superuser, file_fdw_user, no_priv_user;
--- 282,295 ----
-- cleanup
RESET ROLE;
DROP EXTENSION file_fdw CASCADE;
! NOTICE: drop cascades to 9 other objects
DETAIL: drop cascades to server file_server
drop cascades to user mapping for file_fdw_user
drop cascades to user mapping for file_fdw_superuser
drop cascades to user mapping for no_priv_user
drop cascades to foreign table agg_text
drop cascades to foreign table agg_csv
+ drop cascades to view agg_csv_v
drop cascades to foreign table agg_bad
drop cascades to foreign table text_csv
DROP ROLE file_fdw_superuser, file_fdw_user, no_priv_user;
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
new file mode 100644
index cb007cd..4c2c801
*** a/contrib/postgres_fdw/expected/postgres_fdw.out
--- b/contrib/postgres_fdw/expected/postgres_fdw.out
*************** select c2, count(*) from "S 1"."T 1" whe
*** 2339,2341 ****
--- 2339,2410 ----
407 | 100
(13 rows)
+ -- Test updatable view on top of writable foreign table
+ CREATE VIEW ft2_v AS SELECT * FROM ft2;
+ SELECT table_name, is_insertable_into
+ FROM information_schema.tables
+ WHERE table_name LIKE 'ft2%'
+ ORDER BY table_name;
+ table_name | is_insertable_into
+ ------------+--------------------
+ ft2 | NO
+ ft2_v | YES
+ (2 rows)
+
+ SELECT table_name, is_updatable, is_insertable_into
+ FROM information_schema.views
+ WHERE table_name LIKE 'ft2%'
+ ORDER BY table_name;
+ table_name | is_updatable | is_insertable_into
+ ------------+--------------+--------------------
+ ft2_v | YES | YES
+ (1 row)
+
+ SELECT table_name, column_name, is_updatable
+ FROM information_schema.columns
+ WHERE table_name LIKE 'ft2%'
+ ORDER BY table_name, ordinal_position;
+ table_name | column_name | is_updatable
+ ------------+-------------+--------------
+ ft2 | c1 | NO
+ ft2 | c2 | NO
+ ft2 | c3 | NO
+ ft2 | c4 | NO
+ ft2 | c5 | NO
+ ft2 | c6 | NO
+ ft2 | c7 | NO
+ ft2 | c8 | NO
+ ft2_v | c1 | YES
+ ft2_v | c2 | YES
+ ft2_v | c3 | YES
+ ft2_v | c4 | YES
+ ft2_v | c5 | YES
+ ft2_v | c6 | YES
+ ft2_v | c7 | YES
+ ft2_v | c8 | YES
+ (16 rows)
+
+ INSERT INTO ft2_v (c1,c2,c3) VALUES (9999, 250, 'xxx') RETURNING *;
+ c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
+ ------+-----+-----------------+----+----+----+------------+----
+ 9999 | 250 | xxx_trig_update | | | | ft2 |
+ (1 row)
+
+ UPDATE ft2_v SET c2 = c2 + 500 WHERE c1 = 9999 RETURNING *;
+ c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
+ ------+-----+-----------------------------+----+----+----+------------+----
+ 9999 | 750 | xxx_trig_update_trig_update | | | | ft2 |
+ (1 row)
+
+ SELECT * FROM ft2_v WHERE c1 = 9999;
+ c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
+ ------+-----+-----------------------------+----+----+----+------------+----
+ 9999 | 750 | xxx_trig_update_trig_update | | | | ft2 |
+ (1 row)
+
+ DELETE FROM ft2_v WHERE c1 = 9999 RETURNING *;
+ c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
+ ------+-----+-----------------------------+----+----+----+------------+----
+ 9999 | 750 | xxx_trig_update_trig_update | | | | ft2 |
+ (1 row)
+
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
new file mode 100644
index 670d769..416afc3
*** a/contrib/postgres_fdw/sql/postgres_fdw.sql
--- b/contrib/postgres_fdw/sql/postgres_fdw.sql
*************** select c2, count(*) from "S 1"."T 1" whe
*** 369,371 ****
--- 369,394 ----
commit;
select c2, count(*) from ft2 where c2 < 500 group by 1 order by 1;
select c2, count(*) from "S 1"."T 1" where c2 < 500 group by 1 order by 1;
+
+ -- Test updatable view on top of writable foreign table
+ CREATE VIEW ft2_v AS SELECT * FROM ft2;
+
+ SELECT table_name, is_insertable_into
+ FROM information_schema.tables
+ WHERE table_name LIKE 'ft2%'
+ ORDER BY table_name;
+
+ SELECT table_name, is_updatable, is_insertable_into
+ FROM information_schema.views
+ WHERE table_name LIKE 'ft2%'
+ ORDER BY table_name;
+
+ SELECT table_name, column_name, is_updatable
+ FROM information_schema.columns
+ WHERE table_name LIKE 'ft2%'
+ ORDER BY table_name, ordinal_position;
+
+ INSERT INTO ft2_v (c1,c2,c3) VALUES (9999, 250, 'xxx') RETURNING *;
+ UPDATE ft2_v SET c2 = c2 + 500 WHERE c1 = 9999 RETURNING *;
+ SELECT * FROM ft2_v WHERE c1 = 9999;
+ DELETE FROM ft2_v WHERE c1 = 9999 RETURNING *;
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
new file mode 100644
index 83f26e3..296d6a1
*** a/src/backend/rewrite/rewriteHandler.c
--- b/src/backend/rewrite/rewriteHandler.c
*************** view_is_auto_updatable(Relation view)
*** 2014,2019 ****
--- 2014,2020 ----
base_rte = rt_fetch(rtr->rtindex, viewquery->rtable);
if (base_rte->rtekind != RTE_RELATION ||
(base_rte->relkind != RELKIND_RELATION &&
+ base_rte->relkind != RELKIND_FOREIGN_TABLE &&
base_rte->relkind != RELKIND_VIEW))
return gettext_noop("Views that do not select from a single table or view are not automatically updatable.");
*************** relation_is_updatable(Oid reloid, int re
*** 2120,2125 ****
--- 2121,2143 ----
}
}
+ /* If this is a foreign table, check if it is updatable */
+ if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
+ {
+ int events = 0;
+ FdwRoutine *fdwroutine = GetFdwRoutineForRelation(rel, false);
+
+ if (fdwroutine->ExecForeignInsert != NULL)
+ events |= (1 << CMD_INSERT);
+ if (fdwroutine->ExecForeignUpdate != NULL)
+ events |= (1 << CMD_UPDATE);
+ if (fdwroutine->ExecForeignDelete != NULL)
+ events |= (1 << CMD_DELETE);
+
+ relation_close(rel, AccessShareLock);
+ return (events & req_events) == req_events;
+ }
+
/* Check if this is an automatically updatable view */
if (rel->rd_rel->relkind == RELKIND_VIEW &&
view_is_auto_updatable(rel) == NULL)
Dean Rasheed <dean.a.rasheed@gmail.com> writes:
I've just started 9.3 beta testing and I noticed that a "simple" view
defined on top of a writable foreign table is not automatically
updatable.
Given that these are both new-to-9.3 features, I think it would be a
shame if they don't work together. It's basically a 1-line patch to
make such views automatically updatable, plus a small extra code block
in relation_is_updatable() to reflect the change in the
information_schema views.
Meh. This is assuming that an FDW that defines, say, ExecForeignDelete
is thereby promising that *all* tables it supports are deletable. That
is not required by the current FDW API spec.
If we want to do something about this, I'd be a bit inclined to say that
we should add a new FDW callback function to let the FDW say whether
a particular rel is updatable or not.
I think it would be a good idea to get that done for 9.3, since all this
support is new in 9.3, and it's not too late to adjust the API now.
If we wait, there will be compatibility headaches.
Specifically, for foreign tables
information_schema.tables.is_insertable_into and
information_schema.columns.is_updatable always say 'NO' even if the
foreign table is writable. Fixing that would require new C functions
along the same lines as pg_view_is_insertable/updatable(), or those
functions could just be renamed and repurposed to do the check for all
relation kinds, except those known to be always/never updatable.
I'd vote to rename/extend them to be pg_relation_is_updatable I think.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 05/16/2013 05:16 PM, Tom Lane wrote:
Dean Rasheed <dean.a.rasheed@gmail.com> writes:
I've just started 9.3 beta testing and I noticed that a "simple" view
defined on top of a writable foreign table is not automatically
updatable.
Given that these are both new-to-9.3 features, I think it would be a
shame if they don't work together. It's basically a 1-line patch to
make such views automatically updatable, plus a small extra code block
in relation_is_updatable() to reflect the change in the
information_schema views.Meh. This is assuming that an FDW that defines, say, ExecForeignDelete
is thereby promising that *all* tables it supports are deletable. That
is not required by the current FDW API spec.If we want to do something about this, I'd be a bit inclined to say that
we should add a new FDW callback function to let the FDW say whether
a particular rel is updatable or not.I think it would be a good idea to get that done for 9.3, since all this
support is new in 9.3, and it's not too late to adjust the API now.
If we wait, there will be compatibility headaches.
+1
cheers
andrew
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 16 May 2013 22:16, Tom Lane <tgl@sss.pgh.pa.us> wrote:
This is assuming that an FDW that defines, say, ExecForeignDelete
is thereby promising that *all* tables it supports are deletable. That
is not required by the current FDW API spec.
Ah OK, I didn't appreciate that distinction.
If we want to do something about this, I'd be a bit inclined to say that
we should add a new FDW callback function to let the FDW say whether
a particular rel is updatable or not.I think it would be a good idea to get that done for 9.3, since all this
support is new in 9.3, and it's not too late to adjust the API now.
If we wait, there will be compatibility headaches.
+1. That seems like something that should be part of the API, even if
we didn't have an immediate use for it.
Regards,
Dean
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 16 May 2013 22:16, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Specifically, for foreign tables
information_schema.tables.is_insertable_into and
information_schema.columns.is_updatable always say 'NO' even if the
foreign table is writable. Fixing that would require new C functions
along the same lines as pg_view_is_insertable/updatable(), or those
functions could just be renamed and repurposed to do the check for all
relation kinds, except those known to be always/never updatable.I'd vote to rename/extend them to be pg_relation_is_updatable I think.
I remember now just how ugly this code is. The SQL standard has
separate concepts of trigger-insertable, trigger-updatable,
trigger-deletable, insertable and updatable but not deletable for some
reason. So we define updatable as supporting both UPDATE and DELETE
without triggers. I think what we have implemented is technically
correct with regards to the spec in this area, but it is not
particularly useful as far as telling whether a relation will actually
support a particular query in practice (for example a simple view on
top of a trigger-updatable view is neither updatable nor
trigger-updatable).
One place where I think we have diverged from the spec, however, is in
information_schema.columns.updatable. This should be returning 'YES'
if the individual column is updatable, and I see no reason for that
the require the relation to support DELETE, which is what we currently
do (and always have done).
To implement the information_schema properly per-spec, I think we need
3 functions: pg_relation_is_insertable(), pg_relation_is_updatable()
and pg_column_is_updatable(), with the latter just checking UPDATE
events. It's probably a good idea to add these functions now, since I
hope in the future to support more of the SQL spec regarding
automatically updatable views, which will involve views for which only
a subset of their columns are updatable.
The attached patch does that, and tightens up relation_is_updatable()
to support all relation kinds, but it still assumes that if a FDW
defines, say, ExecForeignUpdate, then all its foreign tables are
updatable. That could be improved upon by defining new FDW API
callbacks that select from the remote information_schema, but I'm now
starting to doubt whether its really worth the trouble, given its
bizzare definition.
Regards,
Dean
Attachments:
writable-fdw-view2.patchapplication/octet-stream; name=writable-fdw-view2.patchDownload
diff --git a/contrib/file_fdw/input/file_fdw.source b/contrib/file_fdw/input/file_fdw.source
new file mode 100644
index f7fd28d..063e23c
*** a/contrib/file_fdw/input/file_fdw.source
--- b/contrib/file_fdw/input/file_fdw.source
*************** DELETE FROM agg_csv WHERE a = 100;
*** 121,126 ****
--- 121,148 ----
-- but this should be ignored
SELECT * FROM agg_csv FOR UPDATE;
+ -- simple views on top of the foreign table aren't updatable
+ CREATE VIEW agg_csv_v AS SELECT * FROM agg_csv;
+
+ SELECT table_name, is_insertable_into
+ FROM information_schema.tables
+ WHERE table_name LIKE 'agg_csv%'
+ ORDER BY table_name;
+
+ SELECT table_name, is_updatable, is_insertable_into
+ FROM information_schema.views
+ WHERE table_name LIKE 'agg_csv%'
+ ORDER BY table_name;
+
+ SELECT table_name, column_name, is_updatable
+ FROM information_schema.columns
+ WHERE table_name LIKE 'agg_csv%'
+ ORDER BY table_name, ordinal_position;
+
+ INSERT INTO agg_csv_v VALUES(1,2.0);
+ UPDATE agg_csv_v SET a = 1;
+ DELETE FROM agg_csv_v WHERE a = 100;
+
-- privilege tests
SET ROLE file_fdw_superuser;
SELECT * FROM agg_text ORDER BY a;
diff --git a/contrib/file_fdw/output/file_fdw.source b/contrib/file_fdw/output/file_fdw.source
new file mode 100644
index 4f90bae..2e015cc
*** a/contrib/file_fdw/output/file_fdw.source
--- b/contrib/file_fdw/output/file_fdw.source
*************** SELECT * FROM agg_csv FOR UPDATE;
*** 199,204 ****
--- 199,243 ----
42 | 324.78
(3 rows)
+ -- simple views on top of the foreign table aren't updatable
+ CREATE VIEW agg_csv_v AS SELECT * FROM agg_csv;
+ SELECT table_name, is_insertable_into
+ FROM information_schema.tables
+ WHERE table_name LIKE 'agg_csv%'
+ ORDER BY table_name;
+ table_name | is_insertable_into
+ ------------+--------------------
+ agg_csv | NO
+ agg_csv_v | NO
+ (2 rows)
+
+ SELECT table_name, is_updatable, is_insertable_into
+ FROM information_schema.views
+ WHERE table_name LIKE 'agg_csv%'
+ ORDER BY table_name;
+ table_name | is_updatable | is_insertable_into
+ ------------+--------------+--------------------
+ agg_csv_v | NO | NO
+ (1 row)
+
+ SELECT table_name, column_name, is_updatable
+ FROM information_schema.columns
+ WHERE table_name LIKE 'agg_csv%'
+ ORDER BY table_name, ordinal_position;
+ table_name | column_name | is_updatable
+ ------------+-------------+--------------
+ agg_csv | a | NO
+ agg_csv | b | NO
+ agg_csv_v | a | NO
+ agg_csv_v | b | NO
+ (4 rows)
+
+ INSERT INTO agg_csv_v VALUES(1,2.0);
+ ERROR: cannot insert into foreign table "agg_csv"
+ UPDATE agg_csv_v SET a = 1;
+ ERROR: cannot update foreign table "agg_csv"
+ DELETE FROM agg_csv_v WHERE a = 100;
+ ERROR: cannot delete from foreign table "agg_csv"
-- privilege tests
SET ROLE file_fdw_superuser;
SELECT * FROM agg_text ORDER BY a;
*************** SET ROLE file_fdw_superuser;
*** 243,255 ****
-- cleanup
RESET ROLE;
DROP EXTENSION file_fdw CASCADE;
! NOTICE: drop cascades to 8 other objects
DETAIL: drop cascades to server file_server
drop cascades to user mapping for file_fdw_user
drop cascades to user mapping for file_fdw_superuser
drop cascades to user mapping for no_priv_user
drop cascades to foreign table agg_text
drop cascades to foreign table agg_csv
drop cascades to foreign table agg_bad
drop cascades to foreign table text_csv
DROP ROLE file_fdw_superuser, file_fdw_user, no_priv_user;
--- 282,295 ----
-- cleanup
RESET ROLE;
DROP EXTENSION file_fdw CASCADE;
! NOTICE: drop cascades to 9 other objects
DETAIL: drop cascades to server file_server
drop cascades to user mapping for file_fdw_user
drop cascades to user mapping for file_fdw_superuser
drop cascades to user mapping for no_priv_user
drop cascades to foreign table agg_text
drop cascades to foreign table agg_csv
+ drop cascades to view agg_csv_v
drop cascades to foreign table agg_bad
drop cascades to foreign table text_csv
DROP ROLE file_fdw_superuser, file_fdw_user, no_priv_user;
diff --git a/contrib/postgres_fdw/expected/postgres_fdw.out b/contrib/postgres_fdw/expected/postgres_fdw.out
new file mode 100644
index cb007cd..8f4fe90
*** a/contrib/postgres_fdw/expected/postgres_fdw.out
--- b/contrib/postgres_fdw/expected/postgres_fdw.out
*************** select c2, count(*) from "S 1"."T 1" whe
*** 2339,2341 ****
--- 2339,2410 ----
407 | 100
(13 rows)
+ -- Test updatable view on top of writable foreign table
+ CREATE VIEW ft2_v AS SELECT * FROM ft2;
+ SELECT table_name, is_insertable_into
+ FROM information_schema.tables
+ WHERE table_name LIKE 'ft2%'
+ ORDER BY table_name;
+ table_name | is_insertable_into
+ ------------+--------------------
+ ft2 | YES
+ ft2_v | YES
+ (2 rows)
+
+ SELECT table_name, is_updatable, is_insertable_into
+ FROM information_schema.views
+ WHERE table_name LIKE 'ft2%'
+ ORDER BY table_name;
+ table_name | is_updatable | is_insertable_into
+ ------------+--------------+--------------------
+ ft2_v | YES | YES
+ (1 row)
+
+ SELECT table_name, column_name, is_updatable
+ FROM information_schema.columns
+ WHERE table_name LIKE 'ft2%'
+ ORDER BY table_name, ordinal_position;
+ table_name | column_name | is_updatable
+ ------------+-------------+--------------
+ ft2 | c1 | YES
+ ft2 | c2 | YES
+ ft2 | c3 | YES
+ ft2 | c4 | YES
+ ft2 | c5 | YES
+ ft2 | c6 | YES
+ ft2 | c7 | YES
+ ft2 | c8 | YES
+ ft2_v | c1 | YES
+ ft2_v | c2 | YES
+ ft2_v | c3 | YES
+ ft2_v | c4 | YES
+ ft2_v | c5 | YES
+ ft2_v | c6 | YES
+ ft2_v | c7 | YES
+ ft2_v | c8 | YES
+ (16 rows)
+
+ INSERT INTO ft2_v (c1,c2,c3) VALUES (9999, 250, 'xxx') RETURNING *;
+ c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
+ ------+-----+-----------------+----+----+----+------------+----
+ 9999 | 250 | xxx_trig_update | | | | ft2 |
+ (1 row)
+
+ UPDATE ft2_v SET c2 = c2 + 500 WHERE c1 = 9999 RETURNING *;
+ c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
+ ------+-----+-----------------------------+----+----+----+------------+----
+ 9999 | 750 | xxx_trig_update_trig_update | | | | ft2 |
+ (1 row)
+
+ SELECT * FROM ft2_v WHERE c1 = 9999;
+ c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
+ ------+-----+-----------------------------+----+----+----+------------+----
+ 9999 | 750 | xxx_trig_update_trig_update | | | | ft2 |
+ (1 row)
+
+ DELETE FROM ft2_v WHERE c1 = 9999 RETURNING *;
+ c1 | c2 | c3 | c4 | c5 | c6 | c7 | c8
+ ------+-----+-----------------------------+----+----+----+------------+----
+ 9999 | 750 | xxx_trig_update_trig_update | | | | ft2 |
+ (1 row)
+
diff --git a/contrib/postgres_fdw/sql/postgres_fdw.sql b/contrib/postgres_fdw/sql/postgres_fdw.sql
new file mode 100644
index 670d769..416afc3
*** a/contrib/postgres_fdw/sql/postgres_fdw.sql
--- b/contrib/postgres_fdw/sql/postgres_fdw.sql
*************** select c2, count(*) from "S 1"."T 1" whe
*** 369,371 ****
--- 369,394 ----
commit;
select c2, count(*) from ft2 where c2 < 500 group by 1 order by 1;
select c2, count(*) from "S 1"."T 1" where c2 < 500 group by 1 order by 1;
+
+ -- Test updatable view on top of writable foreign table
+ CREATE VIEW ft2_v AS SELECT * FROM ft2;
+
+ SELECT table_name, is_insertable_into
+ FROM information_schema.tables
+ WHERE table_name LIKE 'ft2%'
+ ORDER BY table_name;
+
+ SELECT table_name, is_updatable, is_insertable_into
+ FROM information_schema.views
+ WHERE table_name LIKE 'ft2%'
+ ORDER BY table_name;
+
+ SELECT table_name, column_name, is_updatable
+ FROM information_schema.columns
+ WHERE table_name LIKE 'ft2%'
+ ORDER BY table_name, ordinal_position;
+
+ INSERT INTO ft2_v (c1,c2,c3) VALUES (9999, 250, 'xxx') RETURNING *;
+ UPDATE ft2_v SET c2 = c2 + 500 WHERE c1 = 9999 RETURNING *;
+ SELECT * FROM ft2_v WHERE c1 = 9999;
+ DELETE FROM ft2_v WHERE c1 = 9999 RETURNING *;
diff --git a/src/backend/catalog/information_schema.sql b/src/backend/catalog/information_schema.sql
new file mode 100644
index 2307586..1837b1a
*** a/src/backend/catalog/information_schema.sql
--- b/src/backend/catalog/information_schema.sql
*************** CREATE VIEW columns AS
*** 731,737 ****
CAST(null AS character_data) AS generation_expression,
CAST(CASE WHEN c.relkind = 'r' OR
! (c.relkind = 'v' AND pg_view_is_updatable(c.oid))
THEN 'YES' ELSE 'NO' END AS yes_or_no) AS is_updatable
FROM (pg_attribute a LEFT JOIN pg_attrdef ad ON attrelid = adrelid AND attnum = adnum)
--- 731,737 ----
CAST(null AS character_data) AS generation_expression,
CAST(CASE WHEN c.relkind = 'r' OR
! (c.relkind IN ('v', 'f') AND pg_column_is_updatable(c.oid, a.attnum))
THEN 'YES' ELSE 'NO' END AS yes_or_no) AS is_updatable
FROM (pg_attribute a LEFT JOIN pg_attrdef ad ON attrelid = adrelid AND attnum = adnum)
*************** CREATE VIEW tables AS
*** 1895,1901 ****
CAST(t.typname AS sql_identifier) AS user_defined_type_name,
CAST(CASE WHEN c.relkind = 'r' OR
! (c.relkind = 'v' AND pg_view_is_insertable(c.oid))
THEN 'YES' ELSE 'NO' END AS yes_or_no) AS is_insertable_into,
CAST(CASE WHEN t.typname IS NOT NULL THEN 'YES' ELSE 'NO' END AS yes_or_no) AS is_typed,
--- 1895,1901 ----
CAST(t.typname AS sql_identifier) AS user_defined_type_name,
CAST(CASE WHEN c.relkind = 'r' OR
! (c.relkind IN ('v', 'f') AND pg_relation_is_insertable(c.oid))
THEN 'YES' ELSE 'NO' END AS yes_or_no) AS is_insertable_into,
CAST(CASE WHEN t.typname IS NOT NULL THEN 'YES' ELSE 'NO' END AS yes_or_no) AS is_typed,
*************** CREATE VIEW views AS
*** 2494,2504 ****
CAST('NONE' AS character_data) AS check_option,
CAST(
! CASE WHEN pg_view_is_updatable(c.oid) THEN 'YES' ELSE 'NO' END
AS yes_or_no) AS is_updatable,
CAST(
! CASE WHEN pg_view_is_insertable(c.oid) THEN 'YES' ELSE 'NO' END
AS yes_or_no) AS is_insertable_into,
CAST(
--- 2494,2504 ----
CAST('NONE' AS character_data) AS check_option,
CAST(
! CASE WHEN pg_relation_is_updatable(c.oid) THEN 'YES' ELSE 'NO' END
AS yes_or_no) AS is_updatable,
CAST(
! CASE WHEN pg_relation_is_insertable(c.oid) THEN 'YES' ELSE 'NO' END
AS yes_or_no) AS is_insertable_into,
CAST(
diff --git a/src/backend/rewrite/rewriteHandler.c b/src/backend/rewrite/rewriteHandler.c
new file mode 100644
index 83f26e3..a75be0e
*** a/src/backend/rewrite/rewriteHandler.c
--- b/src/backend/rewrite/rewriteHandler.c
*************** view_is_auto_updatable(Relation view)
*** 2014,2019 ****
--- 2014,2020 ----
base_rte = rt_fetch(rtr->rtindex, viewquery->rtable);
if (base_rte->rtekind != RTE_RELATION ||
(base_rte->relkind != RELKIND_RELATION &&
+ base_rte->relkind != RELKIND_FOREIGN_TABLE &&
base_rte->relkind != RELKIND_VIEW))
return gettext_noop("Views that do not select from a single table or view are not automatically updatable.");
*************** relation_is_updatable(Oid reloid, int re
*** 2096,2101 ****
--- 2097,2109 ----
if (rel == NULL)
return false;
+ /* If the relation is a table, it is always updatable */
+ if (rel->rd_rel->relkind == RELKIND_RELATION)
+ {
+ relation_close(rel, AccessShareLock);
+ return true;
+ }
+
/* Look for unconditional DO INSTEAD rules, and note supported events */
rulelocks = rel->rd_rules;
if (rulelocks != NULL)
*************** relation_is_updatable(Oid reloid, int re
*** 2120,2125 ****
--- 2128,2150 ----
}
}
+ /* If this is a foreign table, check if it is updatable */
+ if (rel->rd_rel->relkind == RELKIND_FOREIGN_TABLE)
+ {
+ int events = 0;
+ FdwRoutine *fdwroutine = GetFdwRoutineForRelation(rel, false);
+
+ if (fdwroutine->ExecForeignInsert != NULL)
+ events |= (1 << CMD_INSERT);
+ if (fdwroutine->ExecForeignUpdate != NULL)
+ events |= (1 << CMD_UPDATE);
+ if (fdwroutine->ExecForeignDelete != NULL)
+ events |= (1 << CMD_DELETE);
+
+ relation_close(rel, AccessShareLock);
+ return (events & req_events) == req_events;
+ }
+
/* Check if this is an automatically updatable view */
if (rel->rd_rel->relkind == RELKIND_VIEW &&
view_is_auto_updatable(rel) == NULL)
diff --git a/src/backend/utils/adt/misc.c b/src/backend/utils/adt/misc.c
new file mode 100644
index 4e38d7c..fece924
*** a/src/backend/utils/adt/misc.c
--- b/src/backend/utils/adt/misc.c
*************** pg_collation_for(PG_FUNCTION_ARGS)
*** 527,557 ****
}
! /*
! * information_schema support functions
! *
! * Test whether a view (identified by pg_class OID) is insertable-into or
! * updatable. The latter requires delete capability too. This is an
! * artifact of the way the SQL standard defines the information_schema views:
! * if we defined separate functions for update and delete, we'd double the
! * work required to compute the view columns.
*
* These rely on relation_is_updatable(), which is in rewriteHandler.c.
*/
Datum
! pg_view_is_insertable(PG_FUNCTION_ARGS)
{
! Oid viewoid = PG_GETARG_OID(0);
int req_events = (1 << CMD_INSERT);
! PG_RETURN_BOOL(relation_is_updatable(viewoid, req_events));
}
Datum
! pg_view_is_updatable(PG_FUNCTION_ARGS)
{
! Oid viewoid = PG_GETARG_OID(0);
int req_events = (1 << CMD_UPDATE) | (1 << CMD_DELETE);
! PG_RETURN_BOOL(relation_is_updatable(viewoid, req_events));
}
--- 527,591 ----
}
! /* ----------------------------------------------------------------------
! * Information_schema support functions.
*
* These rely on relation_is_updatable(), which is in rewriteHandler.c.
+ * ----------------------------------------------------------------------
+ */
+
+ /*
+ * Test whether a relation (identified by pg_class OID) is insertable-into.
+ *
+ * This is used in information_schema.tables and information_schema.views, and
+ * it supports all kinds of relations although we only actually use it for
+ * views and foreign tables since the other relkinds are trivial.
*/
Datum
! pg_relation_is_insertable(PG_FUNCTION_ARGS)
{
! Oid oid = PG_GETARG_OID(0);
int req_events = (1 << CMD_INSERT);
! PG_RETURN_BOOL(relation_is_updatable(oid, req_events));
}
+ /*
+ * Test whether a relation (identified by pg_class OID) is updatable.
+ *
+ * This is only used for views in information_schema.views, although it will
+ * also work for all other relation kinds.
+ *
+ * NOTE: It requires delete capability too, which is an artifact of the way
+ * the SQL standard defines the information_schema views: if we defined
+ * separate functions for update and delete, we'd double the work required to
+ * compute the view columns.
+ */
Datum
! pg_relation_is_updatable(PG_FUNCTION_ARGS)
{
! Oid oid = PG_GETARG_OID(0);
int req_events = (1 << CMD_UPDATE) | (1 << CMD_DELETE);
! PG_RETURN_BOOL(relation_is_updatable(oid, req_events));
! }
!
! /*
! * Test whether a column (identified by pg_class OID and attnum) is updatable.
! *
! * This is used in information_schema.columns, and it supports all kinds of
! * relations although we only actually use it for views and foreign tables
! * since the other relkinds are trivial.
! */
! Datum
! pg_column_is_updatable(PG_FUNCTION_ARGS)
! {
! /*
! * For now, we ignore the column argument, but in the future we may have
! * relations with a mix of updatable and non-updatable columns.
! */
! Oid oid = PG_GETARG_OID(0);
! int req_events = (1 << CMD_UPDATE);
!
! PG_RETURN_BOOL(relation_is_updatable(oid, req_events));
}
diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h
new file mode 100644
index 685b9c7..5b07d58
*** a/src/include/catalog/pg_proc.h
--- b/src/include/catalog/pg_proc.h
*************** DESCR("type of the argument");
*** 1976,1985 ****
DATA(insert OID = 3162 ( pg_collation_for PGNSP PGUID 12 1 0 0 0 f f f f f f s 1 0 25 "2276" _null_ _null_ _null_ _null_ pg_collation_for _null_ _null_ _null_ ));
DESCR("collation of the argument; implementation of the COLLATION FOR expression");
! DATA(insert OID = 3842 ( pg_view_is_insertable PGNSP PGUID 12 10 0 0 0 f f f f t f s 1 0 16 "26" _null_ _null_ _null_ _null_ pg_view_is_insertable _null_ _null_ _null_ ));
! DESCR("is a view insertable-into");
! DATA(insert OID = 3843 ( pg_view_is_updatable PGNSP PGUID 12 10 0 0 0 f f f f t f s 1 0 16 "26" _null_ _null_ _null_ _null_ pg_view_is_updatable _null_ _null_ _null_ ));
! DESCR("is a view updatable");
/* Deferrable unique constraint trigger */
DATA(insert OID = 1250 ( unique_key_recheck PGNSP PGUID 12 1 0 0 0 f f f f t f v 0 0 2279 "" _null_ _null_ _null_ _null_ unique_key_recheck _null_ _null_ _null_ ));
--- 1976,1987 ----
DATA(insert OID = 3162 ( pg_collation_for PGNSP PGUID 12 1 0 0 0 f f f f f f s 1 0 25 "2276" _null_ _null_ _null_ _null_ pg_collation_for _null_ _null_ _null_ ));
DESCR("collation of the argument; implementation of the COLLATION FOR expression");
! DATA(insert OID = 3842 ( pg_relation_is_insertable PGNSP PGUID 12 10 0 0 0 f f f f t f s 1 0 16 "26" _null_ _null_ _null_ _null_ pg_relation_is_insertable _null_ _null_ _null_ ));
! DESCR("is a relation insertable-into");
! DATA(insert OID = 3843 ( pg_relation_is_updatable PGNSP PGUID 12 10 0 0 0 f f f f t f s 1 0 16 "26" _null_ _null_ _null_ _null_ pg_relation_is_updatable _null_ _null_ _null_ ));
! DESCR("is a relation updatable");
! DATA(insert OID = 3847 ( pg_column_is_updatable PGNSP PGUID 12 10 0 0 0 f f f f t f s 2 0 16 "26 23" _null_ _null_ _null_ _null_ pg_column_is_updatable _null_ _null_ _null_ ));
! DESCR("is a column updatable");
/* Deferrable unique constraint trigger */
DATA(insert OID = 1250 ( unique_key_recheck PGNSP PGUID 12 1 0 0 0 f f f f t f v 0 0 2279 "" _null_ _null_ _null_ _null_ unique_key_recheck _null_ _null_ _null_ ));
diff --git a/src/include/utils/builtins.h b/src/include/utils/builtins.h
new file mode 100644
index 15b60ab..d2e639e
*** a/src/include/utils/builtins.h
--- b/src/include/utils/builtins.h
*************** extern Datum pg_sleep(PG_FUNCTION_ARGS);
*** 485,492 ****
extern Datum pg_get_keywords(PG_FUNCTION_ARGS);
extern Datum pg_typeof(PG_FUNCTION_ARGS);
extern Datum pg_collation_for(PG_FUNCTION_ARGS);
! extern Datum pg_view_is_insertable(PG_FUNCTION_ARGS);
! extern Datum pg_view_is_updatable(PG_FUNCTION_ARGS);
/* oid.c */
extern Datum oidin(PG_FUNCTION_ARGS);
--- 485,493 ----
extern Datum pg_get_keywords(PG_FUNCTION_ARGS);
extern Datum pg_typeof(PG_FUNCTION_ARGS);
extern Datum pg_collation_for(PG_FUNCTION_ARGS);
! extern Datum pg_relation_is_insertable(PG_FUNCTION_ARGS);
! extern Datum pg_relation_is_updatable(PG_FUNCTION_ARGS);
! extern Datum pg_column_is_updatable(PG_FUNCTION_ARGS);
/* oid.c */
extern Datum oidin(PG_FUNCTION_ARGS);
diff --git a/src/test/regress/expected/updatable_views.out b/src/test/regress/expected/updatable_views.out
new file mode 100644
index ecb61e0..3adba33
*** a/src/test/regress/expected/updatable_views.out
--- b/src/test/regress/expected/updatable_views.out
*************** SELECT table_name, column_name, is_updat
*** 468,477 ****
ORDER BY table_name, ordinal_position;
table_name | column_name | is_updatable
------------+-------------+--------------
! rw_view1 | a | NO
! rw_view1 | b | NO
! rw_view2 | a | NO
! rw_view2 | b | NO
(4 rows)
CREATE RULE rw_view1_del_rule AS ON DELETE TO rw_view1
--- 468,477 ----
ORDER BY table_name, ordinal_position;
table_name | column_name | is_updatable
------------+-------------+--------------
! rw_view1 | a | YES
! rw_view1 | b | YES
! rw_view2 | a | YES
! rw_view2 | b | YES
(4 rows)
CREATE RULE rw_view1_del_rule AS ON DELETE TO rw_view1
looking at this patch some more ...
Dean Rasheed <dean.a.rasheed@gmail.com> writes:
One place where I think we have diverged from the spec, however, is in
information_schema.columns.updatable. This should be returning 'YES'
if the individual column is updatable, and I see no reason for that
the require the relation to support DELETE, which is what we currently
do (and always have done).
I'm not convinced about this change. The spec's notion of updatability
requires both UPDATE and DELETE to be allowed; that's why they don't
have a separate is_deletable attribute. And they don't have any such
thing as a column whose updatability doesn't require updatability of the
underlying table. So I think the previous behavior was correct and
should be maintained: although Postgres does permit decoupling
deletability from updatability, only tables/columns for which both
operations are possible should be marked is_updatable in the
information_schema. Otherwise, an application relying on the assumption
that "is_updatable" means it can DELETE will be broken.
I can see however that varying opinions on this are possible. Although
I'd removed the separate pg_column_is_updatable() function from your
patch with the intent of using pg_relation_is_updatable() directly,
I'm now thinking about putting back the former, so that this decision
is taken in C code where we can change it without an initdb.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 12 June 2013 18:35, Tom Lane <tgl@sss.pgh.pa.us> wrote:
looking at this patch some more ...
Dean Rasheed <dean.a.rasheed@gmail.com> writes:
One place where I think we have diverged from the spec, however, is in
information_schema.columns.updatable. This should be returning 'YES'
if the individual column is updatable, and I see no reason for that
the require the relation to support DELETE, which is what we currently
do (and always have done).I'm not convinced about this change. The spec's notion of updatability
requires both UPDATE and DELETE to be allowed; that's why they don't
have a separate is_deletable attribute. And they don't have any such
thing as a column whose updatability doesn't require updatability of the
underlying table. So I think the previous behavior was correct and
should be maintained: although Postgres does permit decoupling
deletability from updatability, only tables/columns for which both
operations are possible should be marked is_updatable in the
information_schema. Otherwise, an application relying on the assumption
that "is_updatable" means it can DELETE will be broken.I can see however that varying opinions on this are possible. Although
I'd removed the separate pg_column_is_updatable() function from your
patch with the intent of using pg_relation_is_updatable() directly,
I'm now thinking about putting back the former, so that this decision
is taken in C code where we can change it without an initdb.
The more I read the spec, the less sense it seems to make, and each
time I read it, I seem to reach a different conclusion.
On my latest reading, I've almost convinced myself that "updatable" is
meant to imply support for all 3 operations (INSERT, UPDATE and
DELETE), at least in the absence of transient tables. The descriptions
of all 3 seem to require the table to be updatable. INSERT requires
the table to be insertable-into, updatable and all its columns to be
updatable, but the requirement for insertable-into is only to rule out
transient tables. So if you don't have transient tables, which aren't
insertable-into, then all 3 operations are possible if and only if the
table is updatable.
That interpretation could be used to simplify the API, but no doubt
when I re-read the spec tomorrow, I'll reach a different conclusion.
Regards,
Dean
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
Dean Rasheed <dean.a.rasheed@gmail.com> writes:
The more I read the spec, the less sense it seems to make, and each
time I read it, I seem to reach a different conclusion.
On my latest reading, I've almost convinced myself that "updatable" is
meant to imply support for all 3 operations (INSERT, UPDATE and
DELETE), at least in the absence of transient tables. The descriptions
of all 3 seem to require the table to be updatable.
Still, they do admit the possibility of insertable_into being different
from is_updatable. So I'm pretty happy with what we've got, at least
on the relation level. Columns seem a bit more debatable; though I
continue to think that an is_updatable column in a not-is_updatable
table isn't contemplated by the spec.
regards, tom lane
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
On 13 June 2013 01:11, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Dean Rasheed <dean.a.rasheed@gmail.com> writes:
The more I read the spec, the less sense it seems to make, and each
time I read it, I seem to reach a different conclusion.On my latest reading, I've almost convinced myself that "updatable" is
meant to imply support for all 3 operations (INSERT, UPDATE and
DELETE), at least in the absence of transient tables. The descriptions
of all 3 seem to require the table to be updatable.Still, they do admit the possibility of insertable_into being different
from is_updatable. So I'm pretty happy with what we've got, at least
on the relation level. Columns seem a bit more debatable; though I
continue to think that an is_updatable column in a not-is_updatable
table isn't contemplated by the spec.
Of course if we didn't have rules, this wouldn't be as issue, because
then a view that handled one update operation would handle them all.
The spec doesn't need to worry about that, so it can define the
updatability of a view as a singular concept based on the view's
definition; and insertable_into in terms of the properties of the base
table. In that context, the possibility of an is_updatable column in a
not-is_updatable table doesn't need to be considered.
I don't think that any more reading of the spec is going to help,
because it's simply not as issue that they had to worry about. If the
spec did consider rules, it would probably define rule_insertable,
etc., in the same way as triggers. So our problem is in trying to
shoe-horn rule-updatability into the spec's idea of updatability, and
it doesn't really fit. The more technically correct answer might be to
say that rule-updatable doesn't count as updatable any more than
trigger-updatable does, but that wouldn't be very useful in practice
because there are no columns in the information schema to check for
rule-updatability. So really, I think we're trying to come up with the
most practically useful definition, and in that context I think we've
probably done the right thing at the relation-level, but I still think
that a column could be marked as is_updatable, even if the table
didn't support DELETEs.
That said, I think that this is of such limited interest to anyone
that I'm inclined to simply keep the status quo.
Regards,
Dean
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers