From e3cf4f269153a488e2b18ca67fafbe18a7901c73 Mon Sep 17 00:00:00 2001 From: Adam Berlin Date: Wed, 3 Apr 2019 18:55:12 +0000 Subject: [PATCH] Do not modify a cached plan statement during ExecuteTruncate. User defined functions cache their plan, therefore if we modify the plan during execution, we risk having invalid data during the next execution of the cached plan. ExecuteTruncate modifies the plan's relations when declaring partitions that need truncation. Instead, we copy the list of relations that need truncation and add the partition relations to the copy. --- src/backend/commands/tablecmds.c | 12 +++- ...h_user_defined_function_that_truncates.out | 60 +++++++++++++++++++ src/test/regress/greenplum_schedule | 2 +- ...h_user_defined_function_that_truncates.sql | 31 ++++++++++ 4 files changed, 101 insertions(+), 4 deletions(-) create mode 100644 src/test/regress/expected/partition_with_user_defined_function_that_truncates.out create mode 100644 src/test/regress/sql/partition_with_user_defined_function_that_truncates.sql diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index c2b0d67ff3..60c8d17a08 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -1574,11 +1574,17 @@ ExecuteTruncate(TruncateStmt *stmt) SubTransactionId mySubid; ListCell *cell; List *partList = NIL; + List *relationsToTruncate = NIL; + + /* + * Copy list to ensure we do not modify a cached plan + */ + relationsToTruncate = list_copy(stmt->relations); /* * Check if table has partitions and add them too */ - foreach(cell, stmt->relations) + foreach(cell, relationsToTruncate) { RangeVar *rv = lfirst(cell); Relation rel; @@ -1596,12 +1602,12 @@ ExecuteTruncate(TruncateStmt *stmt) heap_close(rel, NoLock); } - stmt->relations = list_concat(partList, stmt->relations); + relationsToTruncate = list_concat(partList, relationsToTruncate); /* * Open, exclusive-lock, and check all the explicitly-specified relations */ - foreach(cell, stmt->relations) + foreach(cell, relationsToTruncate) { RangeVar *rv = lfirst(cell); Relation rel; diff --git a/src/test/regress/expected/partition_with_user_defined_function_that_truncates.out b/src/test/regress/expected/partition_with_user_defined_function_that_truncates.out new file mode 100644 index 0000000000..d25f2706b4 --- /dev/null +++ b/src/test/regress/expected/partition_with_user_defined_function_that_truncates.out @@ -0,0 +1,60 @@ +-- Given there is a partitioned table + create table some_partitioned_table_to_truncate + ( + a integer + ) + partition by range (a) ( + partition b start (0) + ); +NOTICE: Table doesn't have 'DISTRIBUTED BY' clause -- Using column named 'a' as the Greenplum Database data distribution key for this table. +HINT: The 'DISTRIBUTED BY' clause determines the distribution of data. Make sure column(s) chosen are the optimal data distribution key to minimize skew. +NOTICE: CREATE TABLE will create partition "some_partitioned_table_to_truncate_1_prt_b" for table "some_partitioned_table_to_truncate" +-- And a function that truncates the partitioned table + CREATE OR REPLACE FUNCTION truncate_a_partition_table() RETURNS VOID AS + $$ + BEGIN + TRUNCATE some_partitioned_table_to_truncate; + END; + $$ LANGUAGE plpgsql; +-- When I truncate the table twice + insert into some_partitioned_table_to_truncate + select i from generate_series(1, 10) i; + select count(*) from some_partitioned_table_to_truncate; + count +------- + 10 +(1 row) + + select truncate_a_partition_table(); + truncate_a_partition_table +---------------------------- + +(1 row) + + select count(*) from some_partitioned_table_to_truncate; + count +------- + 0 +(1 row) + +-- Then I get the same result both times (no rows) + insert into some_partitioned_table_to_truncate + select i from generate_series(1, 10) i; + select count(*) from some_partitioned_table_to_truncate; + count +------- + 10 +(1 row) + + select truncate_a_partition_table(); + truncate_a_partition_table +---------------------------- + +(1 row) + + select count(*) from some_partitioned_table_to_truncate; + count +------- + 0 +(1 row) + diff --git a/src/test/regress/greenplum_schedule b/src/test/regress/greenplum_schedule index e23a0d40d9..5bfea59c1a 100755 --- a/src/test/regress/greenplum_schedule +++ b/src/test/regress/greenplum_schedule @@ -86,7 +86,7 @@ test: partial_table # 'partition' runs for a long time, so try to keep it together with other # long-running tests. -test: partition partition1 partition_indexing parruleord partition_storage partition_ddl partition_with_user_defined_function partition_unlogged partition_subquery +test: partition partition1 partition_indexing parruleord partition_storage partition_ddl partition_with_user_defined_function partition_unlogged partition_subquery partition_with_user_defined_function_that_truncates test: index_constraint_naming index_constraint_naming_partition index_constraint_naming_upgrade # 'partition_locking' gets confused if other backends run concurrently and diff --git a/src/test/regress/sql/partition_with_user_defined_function_that_truncates.sql b/src/test/regress/sql/partition_with_user_defined_function_that_truncates.sql new file mode 100644 index 0000000000..c373df07e8 --- /dev/null +++ b/src/test/regress/sql/partition_with_user_defined_function_that_truncates.sql @@ -0,0 +1,31 @@ +-- Given there is a partitioned table + create table some_partitioned_table_to_truncate + ( + a integer + ) + partition by range (a) ( + partition b start (0) + ); + +-- And a function that truncates the partitioned table + CREATE OR REPLACE FUNCTION truncate_a_partition_table() RETURNS VOID AS + $$ + BEGIN + TRUNCATE some_partitioned_table_to_truncate; + END; + $$ LANGUAGE plpgsql; + +-- When I truncate the table twice + insert into some_partitioned_table_to_truncate + select i from generate_series(1, 10) i; + select count(*) from some_partitioned_table_to_truncate; + select truncate_a_partition_table(); + select count(*) from some_partitioned_table_to_truncate; + +-- Then I get the same result both times (no rows) + insert into some_partitioned_table_to_truncate + select i from generate_series(1, 10) i; + select count(*) from some_partitioned_table_to_truncate; + select truncate_a_partition_table(); + select count(*) from some_partitioned_table_to_truncate; + -- GitLab