On Sat, Nov 21, 2020 at 09:39:28PM -0500, Tom Lane wrote: > Michael Paquier <mich...@paquier.xyz> writes: >> So, what you are basically saying is to switch currtid_byreloid() to >> become a function local to tid.c. And then have just >> currtid_byrelname() and currtid_for_view() call that, right? > > Yeah, that sounds about right.
Okay, here you go with the attached. If there are any other comments, please feel free. -- Michael
diff --git a/src/include/access/heapam.h b/src/include/access/heapam.h
index 92b19dba32..54b2eb7378 100644
--- a/src/include/access/heapam.h
+++ b/src/include/access/heapam.h
@@ -129,7 +129,6 @@ extern bool heap_hot_search_buffer(ItemPointer tid, Relation relation,
bool *all_dead, bool first_call);
extern void heap_get_latest_tid(TableScanDesc scan, ItemPointer tid);
-extern void setLastTid(const ItemPointer tid);
extern BulkInsertState GetBulkInsertState(void);
extern void FreeBulkInsertState(BulkInsertState);
diff --git a/src/include/catalog/catversion.h b/src/include/catalog/catversion.h
index c6da0df868..5f33dc9db0 100644
--- a/src/include/catalog/catversion.h
+++ b/src/include/catalog/catversion.h
@@ -53,6 +53,6 @@
*/
/* yyyymmddN */
-#define CATALOG_VERSION_NO 202011191
+#define CATALOG_VERSION_NO 202011221
#endif
diff --git a/src/include/catalog/pg_proc.dat b/src/include/catalog/pg_proc.dat
index 33dacfd340..e7fbda9f81 100644
--- a/src/include/catalog/pg_proc.dat
+++ b/src/include/catalog/pg_proc.dat
@@ -2549,9 +2549,6 @@
{ oid => '1292',
proname => 'tideq', proleakproof => 't', prorettype => 'bool',
proargtypes => 'tid tid', prosrc => 'tideq' },
-{ oid => '1293', descr => 'latest tid of a tuple',
- proname => 'currtid', provolatile => 'v', proparallel => 'u',
- prorettype => 'tid', proargtypes => 'oid tid', prosrc => 'currtid_byreloid' },
{ oid => '1294', descr => 'latest tid of a tuple',
proname => 'currtid2', provolatile => 'v', proparallel => 'u',
prorettype => 'tid', proargtypes => 'text tid',
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 29e07b7228..e0f24283b8 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -645,10 +645,7 @@ ExecInsert(ModifyTableState *mtstate,
}
if (canSetTag)
- {
(estate->es_processed)++;
- setLastTid(&slot->tts_tid);
- }
/*
* If this insert is the result of a partition key update that moved the
diff --git a/src/backend/utils/adt/tid.c b/src/backend/utils/adt/tid.c
index 509a0fdffc..6f8b400e83 100644
--- a/src/backend/utils/adt/tid.c
+++ b/src/backend/utils/adt/tid.c
@@ -47,6 +47,8 @@
#define DELIM ','
#define NTIDARGS 2
+static ItemPointer currtid_for_view(Relation viewrel, ItemPointer tid);
+
/* ----------------------------------------------------------------
* tidin
* ----------------------------------------------------------------
@@ -275,12 +277,44 @@ hashtidextended(PG_FUNCTION_ARGS)
* Maybe these implementations should be moved to another place
*/
-static ItemPointerData Current_last_tid = {{0, 0}, 0};
-
-void
-setLastTid(const ItemPointer tid)
+/*
+ * Utility wrapper for current CTID functions.
+ * Returns the latest version of a tuple pointing at "tid" for
+ * relation "rel".
+ */
+static ItemPointer
+currtid_internal(Relation rel, ItemPointer tid)
{
- Current_last_tid = *tid;
+ ItemPointer result;
+ AclResult aclresult;
+ Snapshot snapshot;
+ TableScanDesc scan;
+
+ result = (ItemPointer) palloc(sizeof(ItemPointerData));
+
+ aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
+ ACL_SELECT);
+ if (aclresult != ACLCHECK_OK)
+ aclcheck_error(aclresult, get_relkind_objtype(rel->rd_rel->relkind),
+ RelationGetRelationName(rel));
+
+ 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());
+ scan = table_beginscan_tid(rel, snapshot);
+ table_tuple_get_latest_tid(scan, result);
+ table_endscan(scan);
+ UnregisterSnapshot(snapshot);
+
+ return result;
}
/*
@@ -288,7 +322,7 @@ setLastTid(const ItemPointer tid)
* CTID should be defined in the view and it must
* correspond to the CTID of a base relation.
*/
-static Datum
+static ItemPointer
currtid_for_view(Relation viewrel, ItemPointer tid)
{
TupleDesc att = RelationGetDescr(viewrel);
@@ -338,12 +372,12 @@ currtid_for_view(Relation viewrel, ItemPointer tid)
rte = rt_fetch(var->varno, query->rtable);
if (rte)
{
- Datum result;
+ ItemPointer result;
+ Relation rel;
- result = DirectFunctionCall2(currtid_byreloid,
- ObjectIdGetDatum(rte->relid),
- PointerGetDatum(tid));
- table_close(viewrel, AccessShareLock);
+ rel = table_open(rte->relid, AccessShareLock);
+ result = currtid_internal(rel, tid);
+ table_close(rel, AccessShareLock);
return result;
}
}
@@ -352,93 +386,28 @@ currtid_for_view(Relation viewrel, ItemPointer tid)
}
}
elog(ERROR, "currtid cannot handle this view");
- return (Datum) 0;
-}
-
-Datum
-currtid_byreloid(PG_FUNCTION_ARGS)
-{
- Oid reloid = PG_GETARG_OID(0);
- ItemPointer tid = PG_GETARG_ITEMPOINTER(1);
- ItemPointer result;
- Relation rel;
- AclResult aclresult;
- Snapshot snapshot;
- TableScanDesc scan;
-
- result = (ItemPointer) palloc(sizeof(ItemPointerData));
- if (!reloid)
- {
- *result = Current_last_tid;
- PG_RETURN_ITEMPOINTER(result);
- }
-
- rel = table_open(reloid, AccessShareLock);
-
- aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
- ACL_SELECT);
- if (aclresult != ACLCHECK_OK)
- aclcheck_error(aclresult, get_relkind_objtype(rel->rd_rel->relkind),
- RelationGetRelationName(rel));
-
- 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());
- scan = table_beginscan_tid(rel, snapshot);
- table_tuple_get_latest_tid(scan, result);
- table_endscan(scan);
- UnregisterSnapshot(snapshot);
-
- table_close(rel, AccessShareLock);
-
- PG_RETURN_ITEMPOINTER(result);
+ return NULL;
}
+/*
+ * currtid_byrelname
+ * Get the latest tuple version of the tuple pointing at a CTID, for a
+ * given relation name.
+ */
Datum
currtid_byrelname(PG_FUNCTION_ARGS)
{
- text *relname = PG_GETARG_TEXT_PP(0);
+ text *relname = PG_GETARG_TEXT_PP(0);
ItemPointer tid = PG_GETARG_ITEMPOINTER(1);
ItemPointer result;
RangeVar *relrv;
Relation rel;
- AclResult aclresult;
- Snapshot snapshot;
- TableScanDesc scan;
relrv = makeRangeVarFromNameList(textToQualifiedNameList(relname));
rel = table_openrv(relrv, AccessShareLock);
- aclresult = pg_class_aclcheck(RelationGetRelid(rel), GetUserId(),
- ACL_SELECT);
- if (aclresult != ACLCHECK_OK)
- aclcheck_error(aclresult, get_relkind_objtype(rel->rd_rel->relkind),
- RelationGetRelationName(rel));
-
- 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);
-
- snapshot = RegisterSnapshot(GetLatestSnapshot());
- scan = table_beginscan_tid(rel, snapshot);
- table_tuple_get_latest_tid(scan, result);
- table_endscan(scan);
- UnregisterSnapshot(snapshot);
+ /* grab the latest tuple version associated to this CTID */
+ result = currtid_internal(rel, tid);
table_close(rel, AccessShareLock);
diff --git a/src/test/regress/expected/tid.out b/src/test/regress/expected/tid.out
index e7e0d74780..8da1a45576 100644
--- a/src/test/regress/expected/tid.out
+++ b/src/test/regress/expected/tid.out
@@ -15,21 +15,13 @@ SELECT max(ctid) FROM tid_tab;
(1 row)
TRUNCATE tid_tab;
--- Tests for currtid() and currtid2() with various relation kinds
+-- Tests for 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
----------
@@ -40,12 +32,6 @@ 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
----------
@@ -55,39 +41,25 @@ 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
-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
----------
@@ -98,8 +70,6 @@ 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;
diff --git a/src/test/regress/sql/tid.sql b/src/test/regress/sql/tid.sql
index c0d02df34f..34546a3cb7 100644
--- a/src/test/regress/sql/tid.sql
+++ b/src/test/regress/sql/tid.sql
@@ -8,55 +8,46 @@ SELECT min(ctid) FROM tid_tab;
SELECT max(ctid) FROM tid_tab;
TRUNCATE tid_tab;
--- Tests for currtid() and currtid2() with various relation kinds
+-- Tests for 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;
signature.asc
Description: PGP signature
