Fix crashes with currtid() and currtid2()

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

diff --git a/src/backend/utils/adt/tid.c b/src/backend/utils/adt/tid.c
index 4ce8375eab70fb2eb6ca521a59942b4c1b2e9da9..cc699ee2f422d495fe789bf7a61774dc99c4c953 100644 (file)
--- 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"
@@ -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 (file)
index 0000000..e7e0d74
--- /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 95f1925072b09c8ece25517a895316937e45c42d..026ea880cde3a4ca6830050a810bf055887f517e 100644 (file)
--- 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 8ba413622027c63929696dd780757557cf235806..979d9261197d35c50f61c3f5f51c7f69c10fc419 100644 (file)
--- 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 (file)
index 0000000..c0d02df
--- /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;