Fix memory context bug executing TRUNCATE

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 same memory
context of the original list.

Fixes #3580, fixes #3622, fixes #3182

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.

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);
+				case RELKIND_VIEW:
-
-				if (NULL != cagg)
 				{
-					Hypertable *ht;
+					ContinuousAgg *cagg = ts_continuous_agg_find_by_relid(relid);
 
-					if (!relation_should_recurse(rv))
+					if (cagg)
-						ereport(ERROR,
+					{
-								(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+						Hypertable *ht;
-								 errmsg("cannot truncate only a continuous aggregate")));
+						MemoryContext oldctx;
 
-					ht = ts_hypertable_get_by_id(cagg->data.mat_hypertable_id);
+						if (!relation_should_recurse(rv))
-					Assert(ht != NULL);
+							ereport(ERROR,
-					rv = makeRangeVar(NameStr(ht->fd.schema_name), NameStr(ht->fd.table_name), -1);
+									(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+									 errmsg("cannot truncate only a continuous aggregate")));
 
-					/* Invalidate the entire continuous aggregate since it no
+						ht = ts_hypertable_get_by_id(cagg->data.mat_hypertable_id);
-					 * longer has any data */
+						Assert(ht != NULL);
-					ts_cm_functions->continuous_agg_invalidate(ht, PG_INT64_MIN, PG_INT64_MAX);
+
+						/* 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;
 				}
-
+				case RELKIND_RELATION:
-				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
 				{
-					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)
+						ts_hypertable_permissions_check_by_id(ht->fd.id);
-						ereport(ERROR,
-								(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-								 errmsg("cannot TRUNCATE a hypertable underlying a continuous "
-										"aggregate"),
-								 errhint("TRUNCATE the continuous aggregate instead.")));
 
-					if (agg_status == HypertableIsRawTable)
+						if ((agg_status & HypertableIsMaterialization) != 0)
-						/* The truncation invalidates all associated continuous aggregates */
+							ereport(ERROR,
-						ts_cm_functions->continuous_agg_invalidate(ht,
+									(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
-																   TS_TIME_NOBEGIN,
+									 errmsg("cannot TRUNCATE a hypertable underlying a continuous "
-																   TS_TIME_NOEND);
+											"aggregate"),
+									 errhint("TRUNCATE the continuous aggregate instead.")));
 
-					if (!relation_should_recurse(rv))
+						if (agg_status == HypertableIsRawTable)
-						ereport(ERROR,
+							/* The truncation invalidates all associated continuous aggregates */
-								(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+							ts_cm_functions->continuous_agg_invalidate(ht,
-								 errmsg("cannot truncate only a hypertable"),
+																	   TS_TIME_NOBEGIN,
-								 errhint("Do not specify the ONLY keyword, or use truncate"
+																	   TS_TIME_NOEND);
-										 " only on the chunks directly.")));
 
-					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))
+						hypertables = lappend(hypertables, ht);
-						relations = lappend(relations, rv);
+
+						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
+	/* Update relations list just when changed to include only tables
-	 * access node, distributed hypertables hold no data and chunks are
+	 * that hold data. On an access node, distributed hypertables hold
-	 * foreign tables, so those tables are excluded. */
+	 * no data and chunks are foreign tables, so those tables are excluded. */
-	stmt->relations = relations;
+	if (list_changed)
+		stmt->relations = relations;
 
 	if (stmt->relations != NIL)
 	{

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;
+SELECT * FROM truncate_normal, truncate_nested;
- color 
+ color | color 
--------
+-------+-------
 (0 rows)
 

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;

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

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;