On Thu, May 28, 2020 at 05:55:59PM -0700, Andres Freund wrote: > And there only for very old servers (< 8.2), according to Hiroshi > Inoue. Found that out post 12 freeze. I was planning to drop them for > 13, but I unfortunately didn't get around to do so :(
[... digging ...] Ah, I think I see your point from the code. That's related to the use of RETURNING for ctids. > I guess we could decide to make a freeze exception to remove them now, > although I'm not sure the reasons for doing so are strong enough. Not sure that's a good thing to do after beta1 for 13, but there is an argument for that in 14. FWIW, my company is a huge user of the ODBC driver (perhaps the biggest one?), and we have nothing even close to 8.2. > I concur that it seems unnecessary to make these translatable, even with > the reduced scope from > https://www.postgresql.org/message-id/20200526025959.GE6155%40paquier.xyz Okay, I have switched the patch to do that. Any comments or objections? -- Michael
diff --git a/src/backend/utils/adt/tid.c b/src/backend/utils/adt/tid.c index 4ce8375eab..ac232a238f 100644 --- a/src/backend/utils/adt/tid.c +++ b/src/backend/utils/adt/tid.c @@ -31,6 +31,7 @@ #include "parser/parsetree.h" #include "utils/acl.h" #include "utils/builtins.h" +#include "utils/lsyscache.h" #include "utils/rel.h" #include "utils/snapmgr.h" #include "utils/varlena.h" @@ -349,6 +350,12 @@ currtid_for_view(Relation viewrel, ItemPointer tid) return (Datum) 0; } +/* + * currtid_byreloid + * Return the latest visible tid of a relation's tuple, associated + * to the tid given in input. This is a wrapper for currtid(), and + * uses in input the OID of the relation to look at. + */ Datum currtid_byreloid(PG_FUNCTION_ARGS) { @@ -378,6 +385,11 @@ currtid_byreloid(PG_FUNCTION_ARGS) if (rel->rd_rel->relkind == RELKIND_VIEW) return currtid_for_view(rel, tid); + if (!RELKIND_HAS_STORAGE(rel->rd_rel->relkind)) + elog(ERROR, "cannot look at latest visible tid for relation \"%s.%s\"", + get_namespace_name(RelationGetNamespace(rel)), + RelationGetRelationName(rel)); + ItemPointerCopy(tid, result); snapshot = RegisterSnapshot(GetLatestSnapshot()); @@ -391,6 +403,12 @@ currtid_byreloid(PG_FUNCTION_ARGS) PG_RETURN_ITEMPOINTER(result); } +/* + * currtid_byrelname + * Return the latest visible tid of a relation's tuple, associated + * to the tid given in input. This is a wrapper for currtid2(), and + * uses in input the relation name to look at. + */ Datum currtid_byrelname(PG_FUNCTION_ARGS) { @@ -415,6 +433,11 @@ currtid_byrelname(PG_FUNCTION_ARGS) if (rel->rd_rel->relkind == RELKIND_VIEW) return currtid_for_view(rel, tid); + if (!RELKIND_HAS_STORAGE(rel->rd_rel->relkind)) + elog(ERROR, "cannot look at latest visible tid for relation \"%s.%s\"", + get_namespace_name(RelationGetNamespace(rel)), + RelationGetRelationName(rel)); + result = (ItemPointer) palloc(sizeof(ItemPointerData)); ItemPointerCopy(tid, result); diff --git a/src/test/regress/expected/tid.out b/src/test/regress/expected/tid.out new file mode 100644 index 0000000000..e7e0d74780 --- /dev/null +++ b/src/test/regress/expected/tid.out @@ -0,0 +1,106 @@ +-- tests for functions related to TID handling +CREATE TABLE tid_tab (a int); +-- min() and max() for TIDs +INSERT INTO tid_tab VALUES (1), (2); +SELECT min(ctid) FROM tid_tab; + min +------- + (0,1) +(1 row) + +SELECT max(ctid) FROM tid_tab; + max +------- + (0,2) +(1 row) + +TRUNCATE tid_tab; +-- Tests for currtid() and currtid2() with various relation kinds +-- Materialized view +CREATE MATERIALIZED VIEW tid_matview AS SELECT a FROM tid_tab; +SELECT currtid('tid_matview'::regclass::oid, '(0,1)'::tid); -- fails +ERROR: tid (0, 1) is not valid for relation "tid_matview" +SELECT currtid2('tid_matview'::text, '(0,1)'::tid); -- fails +ERROR: tid (0, 1) is not valid for relation "tid_matview" +INSERT INTO tid_tab VALUES (1); +REFRESH MATERIALIZED VIEW tid_matview; +SELECT currtid('tid_matview'::regclass::oid, '(0,1)'::tid); -- ok + currtid +--------- + (0,1) +(1 row) + +SELECT currtid2('tid_matview'::text, '(0,1)'::tid); -- ok + currtid2 +---------- + (0,1) +(1 row) + +DROP MATERIALIZED VIEW tid_matview; +TRUNCATE tid_tab; +-- Sequence +CREATE SEQUENCE tid_seq; +SELECT currtid('tid_seq'::regclass::oid, '(0,1)'::tid); -- ok + currtid +--------- + (0,1) +(1 row) + +SELECT currtid2('tid_seq'::text, '(0,1)'::tid); -- ok + currtid2 +---------- + (0,1) +(1 row) + +DROP SEQUENCE tid_seq; +-- Index, fails with incorrect relation type +CREATE INDEX tid_ind ON tid_tab(a); +SELECT currtid('tid_ind'::regclass::oid, '(0,1)'::tid); -- fails +ERROR: "tid_ind" is an index +SELECT currtid2('tid_ind'::text, '(0,1)'::tid); -- fails +ERROR: "tid_ind" is an index +DROP INDEX tid_ind; +-- Partitioned table, no storage +CREATE TABLE tid_part (a int) PARTITION BY RANGE (a); +SELECT currtid('tid_part'::regclass::oid, '(0,1)'::tid); -- fails +ERROR: cannot look at latest visible tid for relation "public.tid_part" +SELECT currtid2('tid_part'::text, '(0,1)'::tid); -- fails +ERROR: cannot look at latest visible tid for relation "public.tid_part" +DROP TABLE tid_part; +-- Views +-- ctid not defined in the view +CREATE VIEW tid_view_no_ctid AS SELECT a FROM tid_tab; +SELECT currtid('tid_view_no_ctid'::regclass::oid, '(0,1)'::tid); -- fails +ERROR: currtid cannot handle views with no CTID +SELECT currtid2('tid_view_no_ctid'::text, '(0,1)'::tid); -- fails +ERROR: currtid cannot handle views with no CTID +DROP VIEW tid_view_no_ctid; +-- ctid fetched directly from the source table. +CREATE VIEW tid_view_with_ctid AS SELECT ctid, a FROM tid_tab; +SELECT currtid('tid_view_with_ctid'::regclass::oid, '(0,1)'::tid); -- fails +ERROR: tid (0, 1) is not valid for relation "tid_tab" +SELECT currtid2('tid_view_with_ctid'::text, '(0,1)'::tid); -- fails +ERROR: tid (0, 1) is not valid for relation "tid_tab" +INSERT INTO tid_tab VALUES (1); +SELECT currtid('tid_view_with_ctid'::regclass::oid, '(0,1)'::tid); -- ok + currtid +--------- + (0,1) +(1 row) + +SELECT currtid2('tid_view_with_ctid'::text, '(0,1)'::tid); -- ok + currtid2 +---------- + (0,1) +(1 row) + +DROP VIEW tid_view_with_ctid; +TRUNCATE tid_tab; +-- ctid attribute with incorrect data type +CREATE VIEW tid_view_fake_ctid AS SELECT 1 AS ctid, 2 AS a; +SELECT currtid('tid_view_fake_ctid'::regclass::oid, '(0,1)'::tid); -- fails +ERROR: ctid isn't of type TID +SELECT currtid2('tid_view_fake_ctid'::text, '(0,1)'::tid); -- fails +ERROR: ctid isn't of type TID +DROP VIEW tid_view_fake_ctid; +DROP TABLE tid_tab CASCADE; diff --git a/src/test/regress/parallel_schedule b/src/test/regress/parallel_schedule index 95f1925072..026ea880cd 100644 --- a/src/test/regress/parallel_schedule +++ b/src/test/regress/parallel_schedule @@ -78,7 +78,7 @@ test: brin gin gist spgist privileges init_privs security_label collate matview # ---------- # Another group of parallel tests # ---------- -test: create_table_like alter_generic alter_operator misc async dbsize misc_functions sysviews tsrf tidscan collate.icu.utf8 incremental_sort +test: create_table_like alter_generic alter_operator misc async dbsize misc_functions sysviews tsrf tid tidscan collate.icu.utf8 incremental_sort # rules cannot run concurrently with any test that creates # a view or rule in the public schema diff --git a/src/test/regress/serial_schedule b/src/test/regress/serial_schedule index 8ba4136220..979d926119 100644 --- a/src/test/regress/serial_schedule +++ b/src/test/regress/serial_schedule @@ -135,6 +135,7 @@ test: dbsize test: misc_functions test: sysviews test: tsrf +test: tid test: tidscan test: collate.icu.utf8 test: rules diff --git a/src/test/regress/sql/tid.sql b/src/test/regress/sql/tid.sql new file mode 100644 index 0000000000..c0d02df34f --- /dev/null +++ b/src/test/regress/sql/tid.sql @@ -0,0 +1,63 @@ +-- tests for functions related to TID handling + +CREATE TABLE tid_tab (a int); + +-- min() and max() for TIDs +INSERT INTO tid_tab VALUES (1), (2); +SELECT min(ctid) FROM tid_tab; +SELECT max(ctid) FROM tid_tab; +TRUNCATE tid_tab; + +-- Tests for currtid() and currtid2() with various relation kinds + +-- Materialized view +CREATE MATERIALIZED VIEW tid_matview AS SELECT a FROM tid_tab; +SELECT currtid('tid_matview'::regclass::oid, '(0,1)'::tid); -- fails +SELECT currtid2('tid_matview'::text, '(0,1)'::tid); -- fails +INSERT INTO tid_tab VALUES (1); +REFRESH MATERIALIZED VIEW tid_matview; +SELECT currtid('tid_matview'::regclass::oid, '(0,1)'::tid); -- ok +SELECT currtid2('tid_matview'::text, '(0,1)'::tid); -- ok +DROP MATERIALIZED VIEW tid_matview; +TRUNCATE tid_tab; + +-- Sequence +CREATE SEQUENCE tid_seq; +SELECT currtid('tid_seq'::regclass::oid, '(0,1)'::tid); -- ok +SELECT currtid2('tid_seq'::text, '(0,1)'::tid); -- ok +DROP SEQUENCE tid_seq; + +-- Index, fails with incorrect relation type +CREATE INDEX tid_ind ON tid_tab(a); +SELECT currtid('tid_ind'::regclass::oid, '(0,1)'::tid); -- fails +SELECT currtid2('tid_ind'::text, '(0,1)'::tid); -- fails +DROP INDEX tid_ind; + +-- Partitioned table, no storage +CREATE TABLE tid_part (a int) PARTITION BY RANGE (a); +SELECT currtid('tid_part'::regclass::oid, '(0,1)'::tid); -- fails +SELECT currtid2('tid_part'::text, '(0,1)'::tid); -- fails +DROP TABLE tid_part; + +-- Views +-- ctid not defined in the view +CREATE VIEW tid_view_no_ctid AS SELECT a FROM tid_tab; +SELECT currtid('tid_view_no_ctid'::regclass::oid, '(0,1)'::tid); -- fails +SELECT currtid2('tid_view_no_ctid'::text, '(0,1)'::tid); -- fails +DROP VIEW tid_view_no_ctid; +-- ctid fetched directly from the source table. +CREATE VIEW tid_view_with_ctid AS SELECT ctid, a FROM tid_tab; +SELECT currtid('tid_view_with_ctid'::regclass::oid, '(0,1)'::tid); -- fails +SELECT currtid2('tid_view_with_ctid'::text, '(0,1)'::tid); -- fails +INSERT INTO tid_tab VALUES (1); +SELECT currtid('tid_view_with_ctid'::regclass::oid, '(0,1)'::tid); -- ok +SELECT currtid2('tid_view_with_ctid'::text, '(0,1)'::tid); -- ok +DROP VIEW tid_view_with_ctid; +TRUNCATE tid_tab; +-- ctid attribute with incorrect data type +CREATE VIEW tid_view_fake_ctid AS SELECT 1 AS ctid, 2 AS a; +SELECT currtid('tid_view_fake_ctid'::regclass::oid, '(0,1)'::tid); -- fails +SELECT currtid2('tid_view_fake_ctid'::text, '(0,1)'::tid); -- fails +DROP VIEW tid_view_fake_ctid; + +DROP TABLE tid_tab CASCADE;
signature.asc
Description: PGP signature