Hi,
While working on one of the issue, I have noticed below unexpected behavior
with "PREPARE TRANSACTION".
We are getting this unexpected behavior with PREPARE TRANSACTION when it is
mixed with Temporary Objects. Please consider the below setup and SQL block.
set max_prepared_transactions to 1 (or any non zero value), this is to
enable the “prepare transaction”.
Now please try to run the below set of statements.
[BLOCK-1:]
postgres=# create temp table fullname (first text, last text);
CREATE TABLE
postgres=# BEGIN;
BEGIN
postgres=*# create function longname(fullname) returns text language sql
postgres-*# as $$select $1.first || ' ' || $1.last$$;
CREATE FUNCTION
postgres=*# prepare transaction 'mytran';
ERROR: cannot PREPARE a transaction that has operated on temporary objects
Above error is expected.
The problem is if we again try to create the same function in the “PREPARE
TRANSACTION” as below.
[BLOCK-2:]
postgres=# BEGIN;
BEGIN
postgres=*# create function longname(fullname) returns text language sql
as $$select $1.first || ' ' || $1.last$$;
CREATE FUNCTION
postgres=*# PREPARE transaction 'mytran';
PREPARE TRANSACTION
Now, this time we succeed and not getting the above error (”ERROR: cannot
PREPARE a transaction that has operated on temporary objects), like the way
we were getting with BLOCK-1
This is happening because we set MyXactFlags in relation_open function
call, and here relation_open is getting called from load_typcache_tupdesc,
but in the second run of “create function…” in the above #2 block will not
call load_typcache_tupdesc because of the below condition(typentry->tupDesc
== NULL) in lookup_type_cache().
/*
* If it's a composite type (row type), get tupdesc if requested
*/
if ((flags & TYPECACHE_TUPDESC) &&
typentry->tupDesc == NULL &&
typentry->typtype == TYPTYPE_COMPOSITE)
{
load_typcache_tupdesc(typentry);
}
We set typentry->tupDesc to non NULL(and populates it with proper tuple
descriptor in the cache) value during our first call to “create function…”
in BLOCK-1.
We have logic in file xact.c::PrepareTransaction() to simply error out if
we have accessed any temporary object in the current transaction but
because of the above-described issue of not setting
XACT_FLAGS_ACCESSEDTEMPNAMESPACE in MyXactFlags second run of “create
function..” Works and PREPARE TRANSACTION succeeds(but it should fail).
Please find attached the proposed patch to FIX this issue.
Thoughts?
Thanks,
Himanshu Upadhyaya
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From 6e2a6bdfb77870213d051f31b2b6683ccea45ab2 Mon Sep 17 00:00:00 2001
From: Himanshu Upadhyaya <himanshu.upadhy...@enterprisedb.com>
Date: Mon, 5 Apr 2021 21:04:56 +0530
Subject: [PATCH v1] PREPARE TRANSACTION must fail when mixed with temporary
object.
---
src/backend/utils/cache/typcache.c | 30 ++++++++++++++++--------------
src/test/regress/expected/temp.out | 14 ++++++++++++++
src/test/regress/sql/temp.sql | 14 ++++++++++++++
3 files changed, 44 insertions(+), 14 deletions(-)
diff --git a/src/backend/utils/cache/typcache.c b/src/backend/utils/cache/typcache.c
index 4915ef5934..36fac6150d 100644
--- a/src/backend/utils/cache/typcache.c
+++ b/src/backend/utils/cache/typcache.c
@@ -813,7 +813,6 @@ lookup_type_cache(Oid type_id, int flags)
* If it's a composite type (row type), get tupdesc if requested
*/
if ((flags & TYPECACHE_TUPDESC) &&
- typentry->tupDesc == NULL &&
typentry->typtype == TYPTYPE_COMPOSITE)
{
load_typcache_tupdesc(typentry);
@@ -881,21 +880,25 @@ load_typcache_tupdesc(TypeCacheEntry *typentry)
/*
* Link to the tupdesc and increment its refcount (we assert it's a
- * refcounted descriptor). We don't use IncrTupleDescRefCount() for this,
- * because the reference mustn't be entered in the current resource owner;
+ * refcounted descriptor), do this only if it was not populated previously.
+ * We don't use IncrTupleDescRefCount() for this, because the reference
+ * mustn't be entered in the current resource owner;
* it can outlive the current query.
*/
- typentry->tupDesc = RelationGetDescr(rel);
+ if (typentry->tupDesc == NULL)
+ {
+ typentry->tupDesc = RelationGetDescr(rel);
- Assert(typentry->tupDesc->tdrefcount > 0);
- typentry->tupDesc->tdrefcount++;
+ Assert(typentry->tupDesc->tdrefcount > 0);
+ typentry->tupDesc->tdrefcount++;
- /*
- * In future, we could take some pains to not change tupDesc_identifier if
- * the tupdesc didn't really change; but for now it's not worth it.
- */
- typentry->tupDesc_identifier = ++tupledesc_id_counter;
+ /*
+ * In future, we could take some pains to not change tupDesc_identifier if
+ * the tupdesc didn't really change; but for now it's not worth it.
+ */
+ typentry->tupDesc_identifier = ++tupledesc_id_counter;
+ }
relation_close(rel, AccessShareLock);
}
@@ -1529,9 +1532,8 @@ cache_record_field_properties(TypeCacheEntry *typentry)
int newflags;
int i;
- /* Fetch composite type's tupdesc if we don't have it already */
- if (typentry->tupDesc == NULL)
- load_typcache_tupdesc(typentry);
+ /* Fetch composite type's tupdesc */
+ load_typcache_tupdesc(typentry);
tupdesc = typentry->tupDesc;
/* Must bump the refcount while we do additional catalog lookups */
diff --git a/src/test/regress/expected/temp.out b/src/test/regress/expected/temp.out
index a5b3ed34a3..ebf5909389 100644
--- a/src/test/regress/expected/temp.out
+++ b/src/test/regress/expected/temp.out
@@ -319,6 +319,20 @@ select relname from pg_class where relname ~ '^temp_inh_oncommit_test';
(1 row)
drop table temp_inh_oncommit_test;
+-- Test for accessing Temporary table
+-- in prepare transaction.
+-- These cases must fail and generate errors about Temporary objects.
+CREATE TEMP TABLE temp_tbl (first TEXT, last TEXT);
+BEGIN;
+CREATE FUNCTION longname(temp_tbl) RETURNS TEXT LANGUAGE SQL
+AS $$SELECT $1.first || ' ' || $1.last$$;
+PREPARE TRANSACTION 'temp_tbl_access';
+ERROR: cannot PREPARE a transaction that has operated on temporary objects
+BEGIN;
+CREATE FUNCTION longname(temp_tbl) RETURNS TEXT LANGUAGE SQL
+AS $$SELECT $1.first || ' ' || $1.last$$;
+PREPARE TRANSACTION 'temp_tbl_access';
+ERROR: cannot PREPARE a transaction that has operated on temporary objects
-- Tests with two-phase commit
-- Transactions creating objects in a temporary namespace cannot be used
-- with two-phase commit.
diff --git a/src/test/regress/sql/temp.sql b/src/test/regress/sql/temp.sql
index 424d12b283..7cda474782 100644
--- a/src/test/regress/sql/temp.sql
+++ b/src/test/regress/sql/temp.sql
@@ -240,6 +240,20 @@ select * from temp_inh_oncommit_test;
select relname from pg_class where relname ~ '^temp_inh_oncommit_test';
drop table temp_inh_oncommit_test;
+-- Test for accessing Temporary table
+-- in prepare transaction.
+-- These cases must fail and generate errors about Temporary objects.
+CREATE TEMP TABLE temp_tbl (first TEXT, last TEXT);
+BEGIN;
+CREATE FUNCTION longname(temp_tbl) RETURNS TEXT LANGUAGE SQL
+AS $$SELECT $1.first || ' ' || $1.last$$;
+PREPARE TRANSACTION 'temp_tbl_access';
+
+BEGIN;
+CREATE FUNCTION longname(temp_tbl) RETURNS TEXT LANGUAGE SQL
+AS $$SELECT $1.first || ' ' || $1.last$$;
+PREPARE TRANSACTION 'temp_tbl_access';
+
-- Tests with two-phase commit
-- Transactions creating objects in a temporary namespace cannot be used
-- with two-phase commit.
--
2.25.1
