From 84b4a0cf6619b88bff1b20e8bf612b1c09f150fd Mon Sep 17 00:00:00 2001 From: Michael Paquier Date: Fri, 9 Nov 2018 10:03:31 +0900 Subject: [PATCH] Fix dependency handling of partitions and inheritance for ON COMMIT MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes a set of issues with ON COMMIT actions when used on partitioned tables and tables with inheritance children: - Applying ON COMMIT DROP on a partitioned table with partitions or on a table with inheritance children caused a failure at commit time, with complains about the children being already dropped as all relations are dropped one at the same time. - Applying ON COMMIT DELETE on a partition relying on a partitioned table which uses ON COMMIT DROP would cause the partition truncation to fail as the parent is removed first. The solution to the first problem is to handle the removal of all the dependencies in one go instead of dropping relations one-by-one, based on a suggestion from Álvaro Herrera. So instead all the relation OIDs to remove are gathered and then processed in one round of multiple deletions. The solution to the second problem is to reorder the actions, with truncation happening first and relation drop done after. Even if it means that a partition could be first truncated, then immediately dropped if its partitioned table is dropped, this has the merit to keep the code simple as there is no need to do existence checks on the relations to drop. Contrary to a manual TRUNCATE on a partitioned table, ON COMMIT DELETE does not cascade to its partitions. The ON COMMIT action defined on each partition gets the priority. Author: Michael Paquier Reviewed-by: Amit Langote, Álvaro Herrera, Robert Haas Discussion: https://postgr.es/m/68f17907-ec98-1192-f99f-8011400517f5@lab.ntt.co.jp Backpatch-through: 10 --- doc/src/sgml/ref/create_table.sgml | 7 ++- src/backend/commands/tablecmds.c | 77 +++++++++++++++++++-------- src/test/regress/expected/temp.out | 85 ++++++++++++++++++++++++++++++ src/test/regress/sql/temp.sql | 59 +++++++++++++++++++++ 4 files changed, 203 insertions(+), 25 deletions(-) diff --git a/doc/src/sgml/ref/create_table.sgml b/doc/src/sgml/ref/create_table.sgml index 86b758ff8e..a2c7adecf1 100644 --- a/doc/src/sgml/ref/create_table.sgml +++ b/doc/src/sgml/ref/create_table.sgml @@ -1157,7 +1157,8 @@ WITH ( MODULUS numeric_literal, REM All rows in the temporary table will be deleted at the end of each transaction block. Essentially, an automatic is done - at each commit. + at each commit. When used on a partitioned table, this + is not cascaded to its partitions. @@ -1167,7 +1168,9 @@ WITH ( MODULUS numeric_literal, REM The temporary table will be dropped at the end of the current - transaction block. + transaction block. When used on a partitioned table, this action + drops its partitions and when used on tables with inheritance + children, it drops the dependent children. diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index cc8d3af86a..d40d3ca70c 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -13262,6 +13262,7 @@ PreCommit_on_commit_actions(void) { ListCell *l; List *oids_to_truncate = NIL; + List *oids_to_drop = NIL; foreach(l, on_commits) { @@ -13288,36 +13289,66 @@ PreCommit_on_commit_actions(void) oids_to_truncate = lappend_oid(oids_to_truncate, oc->relid); break; case ONCOMMIT_DROP: - { - ObjectAddress object; - - object.classId = RelationRelationId; - object.objectId = oc->relid; - object.objectSubId = 0; - - /* - * Since this is an automatic drop, rather than one - * directly initiated by the user, we pass the - * PERFORM_DELETION_INTERNAL flag. - */ - performDeletion(&object, - DROP_CASCADE, PERFORM_DELETION_INTERNAL); - - /* - * Note that table deletion will call - * remove_on_commit_action, so the entry should get marked - * as deleted. - */ - Assert(oc->deleting_subid != InvalidSubTransactionId); - break; - } + oids_to_drop = lappend_oid(oids_to_drop, oc->relid); + break; } } + + /* + * Truncate relations before dropping so that all dependencies between + * relations are removed after they are worked on. Doing it like this + * might be a waste as it is possible that a relation being truncated will + * be dropped anyway due to its parent being dropped, but this makes the + * code more robust because of not having to re-check that the relation + * exists at truncation time. + */ if (oids_to_truncate != NIL) { heap_truncate(oids_to_truncate); CommandCounterIncrement(); /* XXX needed? */ } + if (oids_to_drop != NIL) + { + ObjectAddresses *targetObjects = new_object_addresses(); + ListCell *l; + + foreach(l, oids_to_drop) + { + ObjectAddress object; + + object.classId = RelationRelationId; + object.objectId = lfirst_oid(l); + object.objectSubId = 0; + + Assert(!object_address_present(&object, targetObjects)); + + add_exact_object_address(&object, targetObjects); + } + + /* + * Since this is an automatic drop, rather than one directly initiated + * by the user, we pass the PERFORM_DELETION_INTERNAL flag. + */ + performMultipleDeletions(targetObjects, DROP_CASCADE, + PERFORM_DELETION_INTERNAL | PERFORM_DELETION_QUIETLY); + +#ifdef USE_ASSERT_CHECKING + + /* + * Note that table deletion will call remove_on_commit_action, so the + * entry should get marked as deleted. + */ + foreach(l, on_commits) + { + OnCommitItem *oc = (OnCommitItem *) lfirst(l); + + if (oc->oncommit != ONCOMMIT_DROP) + continue; + + Assert(oc->deleting_subid != InvalidSubTransactionId); + } +#endif + } } /* diff --git a/src/test/regress/expected/temp.out b/src/test/regress/expected/temp.out index a769abe9bb..f018f17ca0 100644 --- a/src/test/regress/expected/temp.out +++ b/src/test/regress/expected/temp.out @@ -216,3 +216,88 @@ select * from temp_parted_oncommit; (0 rows) drop table temp_parted_oncommit; +-- Check dependencies between ON COMMIT actions with a partitioned +-- table and its partitions. Using ON COMMIT DROP on a parent removes +-- the whole set. +begin; +create temp table temp_parted_oncommit_test (a int) + partition by list (a) on commit drop; +create temp table temp_parted_oncommit_test1 + partition of temp_parted_oncommit_test + for values in (1) on commit delete rows; +create temp table temp_parted_oncommit_test2 + partition of temp_parted_oncommit_test + for values in (2) on commit drop; +insert into temp_parted_oncommit_test values (1), (2); +commit; +-- no relations remain in this case. +select relname from pg_class where relname like 'temp_parted_oncommit_test%'; + relname +--------- +(0 rows) + +-- Using ON COMMIT DELETE on a partitioned table does not remove +-- all rows if partitions preserve their data. +begin; +create temp table temp_parted_oncommit_test (a int) + partition by list (a) on commit delete rows; +create temp table temp_parted_oncommit_test1 + partition of temp_parted_oncommit_test + for values in (1) on commit preserve rows; +create temp table temp_parted_oncommit_test2 + partition of temp_parted_oncommit_test + for values in (2) on commit drop; +insert into temp_parted_oncommit_test values (1), (2); +commit; +-- Data from the remaining partition is still here as its rows are +-- preserved. +select * from temp_parted_oncommit_test; + a +--- + 1 +(1 row) + +-- two relations remain in this case. +select relname from pg_class where relname like 'temp_parted_oncommit_test%'; + relname +---------------------------- + temp_parted_oncommit_test + temp_parted_oncommit_test1 +(2 rows) + +drop table temp_parted_oncommit_test; +-- Check dependencies between ON COMMIT actions with inheritance trees. +-- Using ON COMMIT DROP on a parent removes the whole set. +begin; +create temp table temp_inh_oncommit_test (a int) on commit drop; +create temp table temp_inh_oncommit_test1 () + inherits(temp_inh_oncommit_test) on commit delete rows; +insert into temp_inh_oncommit_test1 values (1); +commit; +-- no relations remain in this case +select relname from pg_class where relname like 'temp_inh_oncommit_test%'; + relname +--------- +(0 rows) + +-- Data on the parent is removed, and the child goes away. +begin; +create temp table temp_inh_oncommit_test (a int) on commit delete rows; +create temp table temp_inh_oncommit_test1 () + inherits(temp_inh_oncommit_test) on commit drop; +insert into temp_inh_oncommit_test1 values (1); +insert into temp_inh_oncommit_test values (1); +commit; +select * from temp_inh_oncommit_test; + a +--- +(0 rows) + +-- one relation remains +select relname from pg_class where relname like 'temp_inh_oncommit_test%'; + relname +------------------------ + temp_inh_oncommit_test +(1 row) + +drop table temp_inh_oncommit_test; diff --git a/src/test/regress/sql/temp.sql b/src/test/regress/sql/temp.sql index 1074c7cfac..1beccc6ceb 100644 --- a/src/test/regress/sql/temp.sql +++ b/src/test/regress/sql/temp.sql @@ -165,3 +165,62 @@ commit; -- partitions are emptied by the previous commit select * from temp_parted_oncommit; drop table temp_parted_oncommit; + +-- Check dependencies between ON COMMIT actions with a partitioned +-- table and its partitions. Using ON COMMIT DROP on a parent removes +-- the whole set. +begin; +create temp table temp_parted_oncommit_test (a int) + partition by list (a) on commit drop; +create temp table temp_parted_oncommit_test1 + partition of temp_parted_oncommit_test + for values in (1) on commit delete rows; +create temp table temp_parted_oncommit_test2 + partition of temp_parted_oncommit_test + for values in (2) on commit drop; +insert into temp_parted_oncommit_test values (1), (2); +commit; +-- no relations remain in this case. +select relname from pg_class where relname like 'temp_parted_oncommit_test%'; +-- Using ON COMMIT DELETE on a partitioned table does not remove +-- all rows if partitions preserve their data. +begin; +create temp table temp_parted_oncommit_test (a int) + partition by list (a) on commit delete rows; +create temp table temp_parted_oncommit_test1 + partition of temp_parted_oncommit_test + for values in (1) on commit preserve rows; +create temp table temp_parted_oncommit_test2 + partition of temp_parted_oncommit_test + for values in (2) on commit drop; +insert into temp_parted_oncommit_test values (1), (2); +commit; +-- Data from the remaining partition is still here as its rows are +-- preserved. +select * from temp_parted_oncommit_test; +-- two relations remain in this case. +select relname from pg_class where relname like 'temp_parted_oncommit_test%'; +drop table temp_parted_oncommit_test; + +-- Check dependencies between ON COMMIT actions with inheritance trees. +-- Using ON COMMIT DROP on a parent removes the whole set. +begin; +create temp table temp_inh_oncommit_test (a int) on commit drop; +create temp table temp_inh_oncommit_test1 () + inherits(temp_inh_oncommit_test) on commit delete rows; +insert into temp_inh_oncommit_test1 values (1); +commit; +-- no relations remain in this case +select relname from pg_class where relname like 'temp_inh_oncommit_test%'; +-- Data on the parent is removed, and the child goes away. +begin; +create temp table temp_inh_oncommit_test (a int) on commit delete rows; +create temp table temp_inh_oncommit_test1 () + inherits(temp_inh_oncommit_test) on commit drop; +insert into temp_inh_oncommit_test1 values (1); +insert into temp_inh_oncommit_test values (1); +commit; +select * from temp_inh_oncommit_test; +-- one relation remains +select relname from pg_class where relname like 'temp_inh_oncommit_test%'; +drop table temp_inh_oncommit_test;