From 95e389b3c2ea35ad60419f285fd5c1d0511142e7 Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Mon, 1 Jun 2020 10:32:53 +0900 Subject: [PATCH] Fix crashes with currtid() and currtid2() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A relation that has no storage initializes rd_tableam to NULL, which caused those two functions to crash because of a pointer dereference. Note that in 11 and older versions, this has always failed with a confusing error "could not open file". These two functions are used by the Postgres ODBC driver, which requires them only when connecting to a backend strictly older than 8.1. When connected to 8.2 or a newer version, the driver uses a RETURNING clause instead whose support has been added in 8.2, so it should be possible to just remove both functions in the future. This is left as an issue to address later. While on it, add more regression tests for those functions as we never really had coverage for them, and for aggregates of TIDs. Reported-by: Jaime Casanova, via sqlsmith Author: Michael Paquier Reviewed-by: Álvaro Herrera Discussion: https://postgr.es/m/CAJGNTeO93u-5APMga6WH41eTZ3Uee9f3s8dCpA-GSSqNs1b=Ug@mail.gmail.com Backpatch-through: 12 --- src/backend/utils/adt/tid.c | 11 +++ src/test/regress/expected/tid.out | 106 +++++++++++++++++++++++++++++ src/test/regress/parallel_schedule | 2 +- src/test/regress/serial_schedule | 1 + src/test/regress/sql/tid.sql | 63 +++++++++++++++++ 5 files changed, 182 insertions(+), 1 deletion(-) create mode 100644 src/test/regress/expected/tid.out create mode 100644 src/test/regress/sql/tid.sql diff --git a/src/backend/utils/adt/tid.c b/src/backend/utils/adt/tid.c index 2bba09feaad..d301c4e2a7b 100644 --- a/src/backend/utils/adt/tid.c +++ b/src/backend/utils/adt/tid.c @@ -31,6 +31,7 @@ #include "utils/acl.h" #include "utils/builtins.h" #include "utils/hashutils.h" +#include "utils/lsyscache.h" #include "utils/rel.h" #include "utils/snapmgr.h" #include "utils/varlena.h" @@ -378,6 +379,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()); @@ -415,6 +421,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 00000000000..e7e0d747807 --- /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 8fb55f045e6..7a46d677b42 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 +test: create_table_like alter_generic alter_operator misc async dbsize misc_functions sysviews tsrf tid tidscan # 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 a39ca1012a3..34fd990243b 100644 --- a/src/test/regress/serial_schedule +++ b/src/test/regress/serial_schedule @@ -132,6 +132,7 @@ test: dbsize test: misc_functions test: sysviews test: tsrf +test: tid test: tidscan test: rules test: psql diff --git a/src/test/regress/sql/tid.sql b/src/test/regress/sql/tid.sql new file mode 100644 index 00000000000..c0d02df34f8 --- /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;