diff --git a/CHANGELOG.md b/CHANGELOG.md index 94fa09267..48d1de319 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,8 +7,12 @@ accidentally triggering the load of a previous DB version.** ## Unreleased **Bugfixes** +* #3580 Fix memory context bug executing TRUNCATE * #3654 Fix index attnum mapping in reorder_chunk +**Thanks** +* @hardikm10, @DavidPavlicek and @pafiti for reporting bugs on TRUNCATE + ## 2.4.2 (2021-09-21) This release contains bug fixes since the 2.4.1 release. diff --git a/src/process_utility.c b/src/process_utility.c index d82921f09..ecd27e9c4 100644 --- a/src/process_utility.c +++ b/src/process_utility.c @@ -913,6 +913,8 @@ process_truncate(ProcessUtilityArgs *args) ListCell *cell; List *hypertables = NIL; List *relations = NIL; + bool list_changed = false; + MemoryContext parsetreectx = GetMemoryChunkContext(args->parsetree); /* For all hypertables, we drop the now empty chunks. We also propagate the * TRUNCATE call to the compressed version of the hypertable, if it exists. @@ -922,8 +924,9 @@ process_truncate(ProcessUtilityArgs *args) { RangeVar *rv = lfirst(cell); Oid relid; + bool list_append = false; - if (NULL == rv) + if (!rv) continue; /* Grab AccessExclusiveLock, same as regular TRUNCATE processing grabs @@ -931,87 +934,113 @@ process_truncate(ProcessUtilityArgs *args) relid = RangeVarGetRelid(rv, AccessExclusiveLock, true); if (!OidIsValid(relid)) - continue; - - switch (get_rel_relkind(relid)) { - case RELKIND_VIEW: + /* We should add invalid relations to the list to raise error on the + * standard_ProcessUtility when we're trying to TRUNCATE a nonexistent relation */ + list_append = true; + } + else + { + switch (get_rel_relkind(relid)) { - ContinuousAgg *cagg = ts_continuous_agg_find_by_relid(relid); - - if (NULL != cagg) + case RELKIND_VIEW: { - Hypertable *ht; + ContinuousAgg *cagg = ts_continuous_agg_find_by_relid(relid); - if (!relation_should_recurse(rv)) - ereport(ERROR, - (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("cannot truncate only a continuous aggregate"))); + if (cagg) + { + Hypertable *ht; + MemoryContext oldctx; - ht = ts_hypertable_get_by_id(cagg->data.mat_hypertable_id); - Assert(ht != NULL); - rv = makeRangeVar(NameStr(ht->fd.schema_name), NameStr(ht->fd.table_name), -1); + if (!relation_should_recurse(rv)) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("cannot truncate only a continuous aggregate"))); - /* Invalidate the entire continuous aggregate since it no - * longer has any data */ - ts_cm_functions->continuous_agg_invalidate(ht, PG_INT64_MIN, PG_INT64_MAX); + ht = ts_hypertable_get_by_id(cagg->data.mat_hypertable_id); + Assert(ht != NULL); + + /* Create list item into the same context of the list */ + oldctx = MemoryContextSwitchTo(parsetreectx); + rv = makeRangeVar(NameStr(ht->fd.schema_name), + NameStr(ht->fd.table_name), + -1); + MemoryContextSwitchTo(oldctx); + + /* Invalidate the entire continuous aggregate since it no + * longer has any data */ + ts_cm_functions->continuous_agg_invalidate(ht, PG_INT64_MIN, PG_INT64_MAX); + + /* mark list as changed because we'll add the materialization hypertable */ + list_changed = true; + } + + list_append = true; + break; } - - relations = lappend(relations, rv); - break; - } - case RELKIND_RELATION: - { - Hypertable *ht = - ts_hypertable_cache_get_entry(hcache, relid, CACHE_FLAG_MISSING_OK); - - if (ht == NULL) - relations = lappend(relations, rv); - else + case RELKIND_RELATION: { - ContinuousAggHypertableStatus agg_status; + Hypertable *ht = + ts_hypertable_cache_get_entry(hcache, relid, CACHE_FLAG_MISSING_OK); - agg_status = ts_continuous_agg_hypertable_status(ht->fd.id); + if (!ht) + list_append = true; + else + { + ContinuousAggHypertableStatus agg_status; - ts_hypertable_permissions_check_by_id(ht->fd.id); + agg_status = ts_continuous_agg_hypertable_status(ht->fd.id); - if ((agg_status & HypertableIsMaterialization) != 0) - ereport(ERROR, - (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), - errmsg("cannot TRUNCATE a hypertable underlying a continuous " - "aggregate"), - errhint("TRUNCATE the continuous aggregate instead."))); + ts_hypertable_permissions_check_by_id(ht->fd.id); - if (agg_status == HypertableIsRawTable) - /* The truncation invalidates all associated continuous aggregates */ - ts_cm_functions->continuous_agg_invalidate(ht, - TS_TIME_NOBEGIN, - TS_TIME_NOEND); + if ((agg_status & HypertableIsMaterialization) != 0) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot TRUNCATE a hypertable underlying a continuous " + "aggregate"), + errhint("TRUNCATE the continuous aggregate instead."))); - if (!relation_should_recurse(rv)) - ereport(ERROR, - (errcode(ERRCODE_WRONG_OBJECT_TYPE), - errmsg("cannot truncate only a hypertable"), - errhint("Do not specify the ONLY keyword, or use truncate" - " only on the chunks directly."))); + if (agg_status == HypertableIsRawTable) + /* The truncation invalidates all associated continuous aggregates */ + ts_cm_functions->continuous_agg_invalidate(ht, + TS_TIME_NOBEGIN, + TS_TIME_NOEND); - hypertables = lappend(hypertables, ht); + if (!relation_should_recurse(rv)) + ereport(ERROR, + (errcode(ERRCODE_WRONG_OBJECT_TYPE), + errmsg("cannot truncate only a hypertable"), + errhint("Do not specify the ONLY keyword, or use truncate" + " only on the chunks directly."))); - if (!hypertable_is_distributed(ht)) - relations = lappend(relations, rv); + hypertables = lappend(hypertables, ht); + + if (!hypertable_is_distributed(ht)) + list_append = true; + else + /* mark list as changed because we'll not add the distributed hypertable + */ + list_changed = true; + } + break; } - break; } - default: - relations = lappend(relations, rv); - break; + } + + /* Append the relation to the list in the same parse tree memory context */ + if (list_append) + { + MemoryContext oldctx = MemoryContextSwitchTo(parsetreectx); + relations = lappend(relations, rv); + MemoryContextSwitchTo(oldctx); } } - /* Update relations list to include only tables that hold data. On an - * access node, distributed hypertables hold no data and chunks are - * foreign tables, so those tables are excluded. */ - stmt->relations = relations; + /* Update relations list just when changed to include only tables + * that hold data. On an access node, distributed hypertables hold + * no data and chunks are foreign tables, so those tables are excluded. */ + if (list_changed) + stmt->relations = relations; if (stmt->relations != NIL) { diff --git a/test/expected/truncate.out b/test/expected/truncate.out index 25c1f4c26..977a3caa0 100644 --- a/test/expected/truncate.out +++ b/test/expected/truncate.out @@ -152,6 +152,58 @@ SELECT * FROM truncate_normal; 1 (1 row) +-- fix for bug #3580 +\set ON_ERROR_STOP 0 +TRUNCATE nonexistentrelation; +ERROR: relation "nonexistentrelation" does not exist +\set ON_ERROR_STOP 1 +CREATE TABLE truncate_nested (color int); +INSERT INTO truncate_nested VALUES (2); +SELECT * FROM truncate_normal, truncate_nested; + color | color +-------+------- + 1 | 2 +(1 row) + +CREATE FUNCTION fn_truncate_nested() +RETURNS trigger LANGUAGE plpgsql +AS $$ +BEGIN + TRUNCATE truncate_nested; + RETURN NEW; +END; +$$; +CREATE TRIGGER tg_truncate_nested + BEFORE TRUNCATE ON truncate_normal + FOR EACH STATEMENT EXECUTE FUNCTION fn_truncate_nested(); +TRUNCATE truncate_normal; +SELECT * FROM truncate_normal, truncate_nested; + color | color +-------+------- +(0 rows) + +INSERT INTO truncate_normal VALUES (3); +INSERT INTO truncate_nested VALUES (4); +SELECT * FROM truncate_normal, truncate_nested; + color | color +-------+------- + 3 | 4 +(1 row) + +TRUNCATE truncate_normal; +SELECT * FROM truncate_normal, truncate_nested; + color | color +-------+------- +(0 rows) + +INSERT INTO truncate_normal VALUES (5); +INSERT INTO truncate_nested VALUES (6); +SELECT * FROM truncate_normal, truncate_nested; + color | color +-------+------- + 5 | 6 +(1 row) + SELECT * FROM test.show_subtables('"two_Partitions"'); Child | Tablespace ----------------------------------------+------------ @@ -174,8 +226,8 @@ SELECT * FROM "two_Partitions"; ------------+-----------+----------+----------+----------+------------- (0 rows) -SELECT * FROM truncate_normal; - color -------- +SELECT * FROM truncate_normal, truncate_nested; + color | color +-------+------- (0 rows) diff --git a/test/sql/truncate.sql b/test/sql/truncate.sql index 1aeb7c515..796e0f491 100644 --- a/test/sql/truncate.sql +++ b/test/sql/truncate.sql @@ -66,10 +66,47 @@ CREATE TABLE truncate_normal (color int); INSERT INTO truncate_normal VALUES (1); SELECT * FROM truncate_normal; +-- fix for bug #3580 +\set ON_ERROR_STOP 0 +TRUNCATE nonexistentrelation; +\set ON_ERROR_STOP 1 +CREATE TABLE truncate_nested (color int); +INSERT INTO truncate_nested VALUES (2); + +SELECT * FROM truncate_normal, truncate_nested; + +CREATE FUNCTION fn_truncate_nested() +RETURNS trigger LANGUAGE plpgsql +AS $$ +BEGIN + TRUNCATE truncate_nested; + RETURN NEW; +END; +$$; + +CREATE TRIGGER tg_truncate_nested + BEFORE TRUNCATE ON truncate_normal + FOR EACH STATEMENT EXECUTE FUNCTION fn_truncate_nested(); + +TRUNCATE truncate_normal; +SELECT * FROM truncate_normal, truncate_nested; + +INSERT INTO truncate_normal VALUES (3); +INSERT INTO truncate_nested VALUES (4); + +SELECT * FROM truncate_normal, truncate_nested; + +TRUNCATE truncate_normal; +SELECT * FROM truncate_normal, truncate_nested; + +INSERT INTO truncate_normal VALUES (5); +INSERT INTO truncate_nested VALUES (6); +SELECT * FROM truncate_normal, truncate_nested; + SELECT * FROM test.show_subtables('"two_Partitions"'); TRUNCATE "two_Partitions", truncate_normal CASCADE; -- should be empty SELECT * FROM test.show_subtables('"two_Partitions"'); SELECT * FROM "two_Partitions"; -SELECT * FROM truncate_normal; +SELECT * FROM truncate_normal, truncate_nested; diff --git a/tsl/test/expected/chunk_api.out b/tsl/test/expected/chunk_api.out index 38937a4cb..4760597ba 100644 --- a/tsl/test/expected/chunk_api.out +++ b/tsl/test/expected/chunk_api.out @@ -552,7 +552,6 @@ _timescaledb_internal._dist_hyper_2_5_chunk| 4|f | 0| -- Clean up RESET ROLE; TRUNCATE disttable; -TRUNCATE costtable; SELECT * FROM delete_data_node('data_node_1', force => true); WARNING: insufficient number of data nodes for distributed hypertable "disttable" NOTICE: the number of partitions in dimension "device" was decreased to 1 diff --git a/tsl/test/sql/chunk_api.sql b/tsl/test/sql/chunk_api.sql index 51334420a..103536f7a 100644 --- a/tsl/test/sql/chunk_api.sql +++ b/tsl/test/sql/chunk_api.sql @@ -254,7 +254,6 @@ $$); -- Clean up RESET ROLE; TRUNCATE disttable; -TRUNCATE costtable; SELECT * FROM delete_data_node('data_node_1', force => true); SELECT * FROM delete_data_node('data_node_2', force => true); DROP DATABASE :DN_DBNAME_1;