Alvaro Herrera <alvhe...@2ndquadrant.com> writes:
> Glad to see you submitting patches again.
Thanks!
> I suggest to add in your regression tests a case where the prepared
> transaction commits, and ensuring that the temp table is gone from
> catalogs.
Please find such a revision attached.
Regards,
--
dim
commit 0b2b2d4b2ec4cd6cd2d26ef229a3764ab9ad2e78
Author: Dimitri Fontaine <d...@tapoueh.org>
Date: Thu Dec 27 12:22:56 2018 +0100
Add support for on-commit-drop temp tables to prepared transactions.
As the bookkeeping is all done within PreCommit_on_commit_actions() we can
prepare a transaction that have been accessing temporary tables when all of
them as marked ON COMMIT DROP.
diff --git a/doc/src/sgml/ref/prepare_transaction.sgml b/doc/src/sgml/ref/prepare_transaction.sgml
index d958f7a06f..c1ef0707cf 100644
--- a/doc/src/sgml/ref/prepare_transaction.sgml
+++ b/doc/src/sgml/ref/prepare_transaction.sgml
@@ -97,8 +97,9 @@ PREPARE TRANSACTION <replaceable class="parameter">transaction_id</replaceable>
</para>
<para>
- It is not currently allowed to <command>PREPARE</command> a transaction that
- has executed any operations involving temporary tables,
+ It is not currently allowed to <command>PREPARE</command> a transaction
+ that has executed any operations involving temporary tables (except when
+ all involved temporary tables are <literal>ON COMMIT DROP</literal>),
created any cursors <literal>WITH HOLD</literal>, or executed
<command>LISTEN</command>, <command>UNLISTEN</command>, or
<command>NOTIFY</command>.
diff --git a/src/backend/access/transam/xact.c b/src/backend/access/transam/xact.c
index d967400384..1d7f3017ad 100644
--- a/src/backend/access/transam/xact.c
+++ b/src/backend/access/transam/xact.c
@@ -2272,11 +2272,19 @@ PrepareTransaction(void)
* XXX In principle this could be relaxed to allow some useful special
* cases, such as a temp table created and dropped all within the
* transaction. That seems to require much more bookkeeping though.
+ *
+ * A special case of this situation is using ON COMMIT DROP, where the
+ * call to PreCommit_on_commit_actions() is then responsible for
+ * performing the DROP table within the transaction and before we get
+ * here.
*/
- if ((MyXactFlags & XACT_FLAGS_ACCESSEDTEMPREL))
+ if ((MyXactFlags & XACT_FLAGS_ACCESSEDTEMPREL)
+ && !every_on_commit_is_on_commit_drop())
+ {
ereport(ERROR,
(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
- errmsg("cannot PREPARE a transaction that has operated on temporary tables")));
+ errmsg("cannot PREPARE a transaction that has operated on temporary tables that are not ON COMMIT DROP")));
+ }
/*
* Likewise, don't allow PREPARE after pg_export_snapshot. This could be
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index c8c50e8c98..2060f3e68e 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -13139,6 +13139,26 @@ remove_on_commit_action(Oid relid)
}
}
+/*
+ * Return true when every temp relation known in the current session is marked
+ * ON COMMIT DROP. This allows to then PREPARE TRANSACTION, for instance,
+ * because we know that at prepare time the temp table is dropped already.
+ */
+bool
+every_on_commit_is_on_commit_drop()
+{
+ ListCell *l;
+
+ foreach(l, on_commits)
+ {
+ OnCommitItem *oc = (OnCommitItem *) lfirst(l);
+
+ if (oc->oncommit != ONCOMMIT_DROP)
+ return false;
+ }
+ return true;
+}
+
/*
* Perform ON COMMIT actions.
*
diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index 2afcd5be44..2f5731a9f6 100644
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -82,6 +82,7 @@ extern void createForeignKeyTriggers(Relation rel, Oid refRelOid,
extern void register_on_commit_action(Oid relid, OnCommitAction action);
extern void remove_on_commit_action(Oid relid);
+extern bool every_on_commit_is_on_commit_drop(void);
extern void PreCommit_on_commit_actions(void);
extern void AtEOXact_on_commit_actions(bool isCommit);
diff --git a/src/test/regress/expected/prepared_xacts.out b/src/test/regress/expected/prepared_xacts.out
index eb77c18788..d2e81461d0 100644
--- a/src/test/regress/expected/prepared_xacts.out
+++ b/src/test/regress/expected/prepared_xacts.out
@@ -252,3 +252,101 @@ DROP TABLE pxtest2;
DROP TABLE pxtest3; -- will still be there if prepared xacts are disabled
ERROR: table "pxtest3" does not exist
DROP TABLE pxtest4;
+-- we should be able to prepare a transaction with on commit drop temporary
+-- tables
+begin;
+create temp table pxtesttemp(id serial, f1 text) on commit drop;
+insert into pxtesttemp(f1) values('regress'), ('temp') returning *;
+ id | f1
+----+---------
+ 1 | regress
+ 2 | temp
+(2 rows)
+
+prepare transaction 'regress temp';
+commit prepared 'regress temp';
+-- There should be no prepared transactions
+SELECT gid FROM pg_prepared_xacts;
+ gid
+-----
+(0 rows)
+
+-- The temporary table should be gone now
+SELECT relname FROM pg_class WHERE relname = 'pxtesttemp';
+ relname
+---------
+(0 rows)
+
+-- other kinds of temporary tables are not supported though.
+begin;
+create temp table pxtesttemp(id serial, f1 text) on commit delete rows;
+insert into pxtesttemp(f1) values('regress'), ('temp') returning *;
+ id | f1
+----+---------
+ 1 | regress
+ 2 | temp
+(2 rows)
+
+prepare transaction 'regress temp';
+ERROR: cannot PREPARE a transaction that has operated on temporary tables that are not ON COMMIT DROP
+-- There should be no prepared transactions
+SELECT gid FROM pg_prepared_xacts;
+ gid
+-----
+(0 rows)
+
+begin;
+create temp table pxtesttemp1(id serial, f1 text) on commit drop;
+create temp table pxtesttemp2(id serial, f1 text) on commit delete rows;
+insert into pxtesttemp1(f1) values('regress'), ('temp') returning *;
+ id | f1
+----+---------
+ 1 | regress
+ 2 | temp
+(2 rows)
+
+prepare transaction 'regress temp';
+ERROR: cannot PREPARE a transaction that has operated on temporary tables that are not ON COMMIT DROP
+-- There should be no prepared transactions
+SELECT gid FROM pg_prepared_xacts;
+ gid
+-----
+(0 rows)
+
+-- Test with two temporary tables on commit drop now
+begin;
+create temp table pxtesttemp1(id serial, f1 text) on commit drop;
+create temp table pxtesttemp2(id serial, f1 text) on commit drop;
+insert into pxtesttemp1(f1) values('regress'), ('temp');
+insert into pxtesttemp2(f1) values('regress'), ('temp');
+prepare transaction 'regress temp';
+rollback prepared 'regress temp';
+-- There should be no prepared transactions
+SELECT gid FROM pg_prepared_xacts;
+ gid
+-----
+(0 rows)
+
+-- Now test with a temp table that exists in the session, and a temp table
+-- that exists only within the prepared transaction.
+create temp table pxtesttemp1(id serial, f1 text);
+insert into pxtesttemp1(f1) values('regress'), ('temp') returning *;
+ id | f1
+----+---------
+ 1 | regress
+ 2 | temp
+(2 rows)
+
+begin;
+create temp table pxtesttemp2(id serial, f1 text) on commit drop;
+insert into pxtesttemp2(f1) values('regress'), ('temp');
+prepare transaction 'regress temp';
+rollback prepared 'regress temp';
+-- cleanup
+drop table pxtesttemp1;
+-- There should be no prepared transactions
+SELECT gid FROM pg_prepared_xacts;
+ gid
+-----
+(0 rows)
+
diff --git a/src/test/regress/sql/prepared_xacts.sql b/src/test/regress/sql/prepared_xacts.sql
index d8249a27dc..dd8655facf 100644
--- a/src/test/regress/sql/prepared_xacts.sql
+++ b/src/test/regress/sql/prepared_xacts.sql
@@ -156,3 +156,64 @@ SELECT gid FROM pg_prepared_xacts;
DROP TABLE pxtest2;
DROP TABLE pxtest3; -- will still be there if prepared xacts are disabled
DROP TABLE pxtest4;
+
+-- we should be able to prepare a transaction with on commit drop temporary
+-- tables
+begin;
+create temp table pxtesttemp(id serial, f1 text) on commit drop;
+insert into pxtesttemp(f1) values('regress'), ('temp') returning *;
+prepare transaction 'regress temp';
+commit prepared 'regress temp';
+
+-- There should be no prepared transactions
+SELECT gid FROM pg_prepared_xacts;
+
+-- The temporary table should be gone now
+SELECT relname FROM pg_class WHERE relname = 'pxtesttemp';
+
+-- other kinds of temporary tables are not supported though.
+begin;
+create temp table pxtesttemp(id serial, f1 text) on commit delete rows;
+insert into pxtesttemp(f1) values('regress'), ('temp') returning *;
+prepare transaction 'regress temp';
+
+-- There should be no prepared transactions
+SELECT gid FROM pg_prepared_xacts;
+
+begin;
+create temp table pxtesttemp1(id serial, f1 text) on commit drop;
+create temp table pxtesttemp2(id serial, f1 text) on commit delete rows;
+insert into pxtesttemp1(f1) values('regress'), ('temp') returning *;
+prepare transaction 'regress temp';
+
+-- There should be no prepared transactions
+SELECT gid FROM pg_prepared_xacts;
+
+-- Test with two temporary tables on commit drop now
+begin;
+create temp table pxtesttemp1(id serial, f1 text) on commit drop;
+create temp table pxtesttemp2(id serial, f1 text) on commit drop;
+insert into pxtesttemp1(f1) values('regress'), ('temp');
+insert into pxtesttemp2(f1) values('regress'), ('temp');
+prepare transaction 'regress temp';
+rollback prepared 'regress temp';
+
+-- There should be no prepared transactions
+SELECT gid FROM pg_prepared_xacts;
+
+-- Now test with a temp table that exists in the session, and a temp table
+-- that exists only within the prepared transaction.
+create temp table pxtesttemp1(id serial, f1 text);
+insert into pxtesttemp1(f1) values('regress'), ('temp') returning *;
+
+begin;
+create temp table pxtesttemp2(id serial, f1 text) on commit drop;
+insert into pxtesttemp2(f1) values('regress'), ('temp');
+prepare transaction 'regress temp';
+rollback prepared 'regress temp';
+
+-- cleanup
+drop table pxtesttemp1;
+
+-- There should be no prepared transactions
+SELECT gid FROM pg_prepared_xacts;
