Skip to content

Commit

Permalink
Fix memory context bug executing TRUNCATE
Browse files Browse the repository at this point in the history
Inside the `process_truncate` function is created a new relations list
removing the distributed hypertables and this new list is assigned to
the current statement relation list. This new list is allocated into
the `PortalContext` that is destroyed at the end of the statement
execution.

The problem arise on the subsequent `TRUNCATE` call because the
compiled plpgsql code is cached into another memory context and the
elements of the relations inside this cache is pointing to an invalid
memory area because the `PortalContext` is destroyed.

Fixed it by allocating the new relations list to the `TopMemoryContext`
but just when the new list is different than the original.

Fixes #3580, #3622, #3181
  • Loading branch information
fabriziomello committed Oct 5, 2021
1 parent b3380e8 commit 0ccc1a9
Show file tree
Hide file tree
Showing 5 changed files with 116 additions and 10 deletions.
27 changes: 23 additions & 4 deletions src/process_utility.c
Original file line number Diff line number Diff line change
Expand Up @@ -908,11 +908,13 @@ handle_truncate_hypertable(ProcessUtilityArgs *args, TruncateStmt *stmt, Hyperta
static DDLResult
process_truncate(ProcessUtilityArgs *args)
{
MemoryContext oldctx = MemoryContextSwitchTo(TopMemoryContext);
TruncateStmt *stmt = (TruncateStmt *) args->parsetree;
Cache *hcache = ts_hypertable_cache_pin();
ListCell *cell;
List *hypertables = NIL;
List *relations = NIL;
bool list_changed = false;

/* 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.
Expand All @@ -931,7 +933,12 @@ process_truncate(ProcessUtilityArgs *args)
relid = RangeVarGetRelid(rv, AccessExclusiveLock, true);

if (!OidIsValid(relid))
{
/* We should add invalid relations to the list to raise error on the
* standard_ProcessUtility when we're trying to TRUNCATE a nonexistent relation */
relations = lappend(relations, rv);
continue;
}

switch (get_rel_relkind(relid))
{
Expand All @@ -955,6 +962,9 @@ process_truncate(ProcessUtilityArgs *args)
/* 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;
}

relations = lappend(relations, rv);
Expand Down Expand Up @@ -999,6 +1009,9 @@ process_truncate(ProcessUtilityArgs *args)

if (!hypertable_is_distributed(ht))
relations = lappend(relations, rv);
else
/* mark list as changed because we'll not add the distributed hypertable */
list_changed = true;
}
break;
}
Expand All @@ -1008,10 +1021,14 @@ process_truncate(ProcessUtilityArgs *args)
}
}

/* 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;
else
/* Free the list if it was not changed */
pfree(relations);

if (stmt->relations != NIL)
{
Expand Down Expand Up @@ -1048,6 +1065,8 @@ process_truncate(ProcessUtilityArgs *args)

ts_cache_release(hcache);

MemoryContextSwitchTo(oldctx);

return DDL_DONE;
}

Expand Down
58 changes: 55 additions & 3 deletions test/expected/truncate.out
Original file line number Diff line number Diff line change
Expand Up @@ -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
----------------------------------------+------------
Expand All @@ -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)

39 changes: 38 additions & 1 deletion test/sql/truncate.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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;
1 change: 0 additions & 1 deletion tsl/test/expected/chunk_api.out
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion tsl/test/sql/chunk_api.sql
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down

0 comments on commit 0ccc1a9

Please sign in to comment.