Re: Global temporary tables

Hi, Dean

On Wed, 01 Jul 2026 at 08:32, Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> On Wed, 1 Jul 2026 at 07:24, Japin Li <japinli@hotmail.com> wrote:
>>
>> Thanks for working on this.
>
> Thank you for testing it!
>
>> While testing the v5 patch, I encountered a lock wait.
>>
>> 2026-07-01 14:18:43.603 CST [1486593] LOG:  process 1486593 still waiting for AccessExclusiveLock on relation 16384 of database 5 after 1000.106 ms
>> 2026-07-01 14:18:43.603 CST [1486593] DETAIL:  Process holding the lock: 1486484. Wait queue: 1486593.
>> 2026-07-01 14:18:43.603 CST [1486593] CONTEXT:  waiting for AccessExclusiveLock on relation 16384 of database 5
>> 2026-07-01 14:18:43.603 CST [1486593] STATEMENT:  SELECT * FROM gtt_delete;
>>
>> Is this expected?
>
> Ah yes, you're right. That's not good.
>
> The root cause looks to be the same issue that Pavel reported -- the
> ON COMMIT DELETE ROWS does a TRUNCATE when the transaction is
> committed, which requires an AccessExclusiveLock. I think the patch
> should reduce the lock level required for TRUNCATE on a global
> temporary relation to RowExclusiveLock, like DELETE.
>

Here is my review on v5-0002.  Please take a look.

1.
$ git am ~/Patches/gtt/*.patch
Applying: Save temporary table ON COMMIT actions to pg_class.
Applying: Basic support for global temporary tables.
Applying: Support indexes on global temporary tables.
Applying: Support global temporary sequences.
Applying: Support global temporary catalog tables and add pg_temp_class.
Applying: Add relation statistics columns to pg_temp_class.
Applying: Add relfrozenxid and relminmxid columns to pg_temp_class.
.git/rebase-apply/patch:940: new blank line at EOF.
+
warning: 1 line adds whitespace errors.
Applying: Add pg_temp_statistic global temporary catalog table.
Applying: Add pg_temp_statistic_ext_data global temporary catalog table.
Applying: Add pg_temp_index global temporary catalog table.

2.
+static void
+gtr_delete_all_storage_on_exit(int code, Datum arg)
+{
+	ProcNumber	backend;
+	HASH_SEQ_STATUS status;
+	GtrStorageEntry *entry;
+	SMgrRelation srel;
+
+	/* Loop over all storage created and delete it */
+	backend = ProcNumberForTempRelations();
+	hash_seq_init(&status, gtr_local_storage);
+	while ((entry = hash_seq_search(&status)) != NULL)
+	{
+		srel = smgropen(entry->rlocator, backend);
+		smgrdounlinkall(&srel, 1, false);
+		smgrclose(srel);
+	}
+}

We can restrict srel to the loop scope.

3.
+gtr_remove_all_usage_on_exit(int code, Datum arg)
+{
+	HASH_SEQ_STATUS status;
+	GtrUsageEntry *local_entry;
+	GtrSharedUsageKey key;
+	GtrSharedUsageEntry *shared_entry;
+
+	/* Loop over all the global temporary relations we were using */
+	hash_seq_init(&status, gtr_local_usage);
+	while ((local_entry = hash_seq_search(&status)) != NULL)
+	{
+		/* Find the shared usage entry */
+		key.dbid = MyDatabaseId;
+		key.relid = local_entry->relid;
+		shared_entry = dshash_find(gtr_shared_usage, &key, true);

Same as above: move the declarations of key and shared_entry inside the loop.

4.
+static void
+gtr_init_usage_tables(void)
+{
+	HASHCTL		ctl;
+	MemoryContext oldcontext;
+
+	/* Local usage table */
+	if (gtr_local_usage == NULL)
+	{
+		ctl.keysize = sizeof(Oid);
+		ctl.entrysize = sizeof(GtrUsageEntry);
+
+		gtr_local_usage = hash_create("Global temporary relations in use locally",
+									  128, &ctl, HASH_ELEM | HASH_BLOBS);
+	}
+
+	/* Shared usage table */
+	if (gtr_shared_usage == NULL)
+	{
+		/* Use a lock to ensure only one process creates the table */
+		LWLockAcquire(GlobalTempRelControlLock, LW_EXCLUSIVE);
+
+		/* Be sure any local memory allocated by DSA routines is persistent */
+		oldcontext = MemoryContextSwitchTo(TopMemoryContext);

Likewise, narrow ctl to the first if and oldContext to the second.

5.
+	 * the usage hash table entries for it.  Otherwise, reset the hash entry's
+	 * subids to zero.

Instead of zero, how about using an invalid sub-transaction ID?

6.
+static void
+gtr_process_invalidated_gtrs(void)
+{
+	MemoryContext oldcontext;
+	Oid			relid;
+
+	/*
+	 * Scan the gtrs_invalidated list and add any dropped relations to the
+	 * gtrs_dropped list.  Since the transaction might fail later on, we need
+	 * the gtrs_dropped list to persist until we can successfully process it.
+	 */
+	oldcontext = MemoryContextSwitchTo(TopMemoryContext);
+
+	/*
+	 * As we scan the gtrs_invalidated list, more invalidation messages might
+	 * arrive, so keep going until it is empty.
+	 */
+	while (gtrs_invalidated != NIL)
+	{
+		/* Pop the last relid from the list */
+		relid = llast_oid(gtrs_invalidated);

Restrict relid to the while loop scope.

7.
+void
+TrackGlobalTempRelationStorage(Oid relid, RelFileLocator rlocator,
+							   ProcNumber backend, bool create)
+{
+	GtrStorageEntry *entry;
+	bool		found;
+	SMgrRelation srel;
+
+	if (create)
+	{
+		/* Initialize the storage table, if necessary */
+		gtr_init_storage_table();
+
+		/* Insert an entry to track the storage */
+		entry = hash_search(gtr_local_storage, &rlocator, HASH_ENTER, &found);

Likewise, restrict found and srel to the if statement.

8.
+		/* Register the ON COMMIT action for relation, if it's DELETE ROWS */
+		Assert(relation->rd_rel->reloncommit == RELONCOMMIT_PRESERVE_ROWS ||
+			   relation->rd_rel->reloncommit == RELONCOMMIT_DELETE_ROWS);
+
+		if (relation->rd_rel->reloncommit == RELONCOMMIT_DELETE_ROWS)
+			register_on_commit_action(relation->rd_id, ONCOMMIT_DELETE_ROWS);

How about moving the comment to just before the if statement?

9.
+void
+InvalidateGlobalTempRelation(Oid relid)
+{
+	MemoryContext oldcontext;
+	HASH_SEQ_STATUS status;
+	GtrUsageEntry *entry;
+
+	/* Quick exit if we haven't used any global temporary relations */
+	if (gtr_local_usage == NULL)
+		return;

Restrict status and entry to the else statement.

10.
+void
+GlobalTempRelationDropped(Oid relid)

Not a fan of this name, but no better idea yet.

11.
+void
+PreCommit_GlobalTempRelation(void)
+{
+	HASH_SEQ_STATUS status;
+	GtrStorageEntry *storage_entry;
+	Relation	rel;
+

Restrict rel to the while loop.

12.
+List *
+AllGlobalTempRelationsInUse(Oid dbid)

How about GetGlobalTempReplationsInUse()?

13.
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+						 errmsg("cannot create relations in temporary schemas of other sessions")));

For consistency with the above, I suggest changing the error message to:

    cannot create global temporary relation in temporary schema of other sessions

14.
+         * If they're global temp relations, reassign rel2's storage to rel1.

We use "global temporary relations" elsewhere – should we keep the same
terminology here?

15.
+                     errmsg(RELATION_IS_GLOBAL_TEMP(relation)
+                            ? "cannot create a local temporary relation as partition of global temporary relation \"%s\""
+                            : "cannot create a temporary relation as partition of permanent relation \"%s\"",

Should we use the following error message:

    cannot create a local temporary relation as partition of permanent relation \"%s\"

16.
+                            : relpersistence == RELPERSISTENCE_GLOBAL_TEMP
+                            ? "cannot create a global temporary relation as partition of local temporary relation \"%s\""
                             : "cannot create a permanent relation as partition of temporary relation \"%s\"",

Same as above:

    cannot create a permanent relation as partition of local temporary relation \"%s\"

17.
+            if (RELATION_IS_GLOBAL_TEMP(OldHeap) &&
+                IsOtherUsingGlobalTempRelation(RelationGetRelid(OldHeap)))
+                ereport(ERROR,
+                        errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                        errmsg("cannot rewrite global temporary table \"%s\" because it is being used in another session",
+                               RelationGetRelationName(OldHeap)));


18.
+                            errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
+                            errmsg("cannot add or alter constraints of global temporary table \"%s\" because it is being used in another session",
+                                   get_rel_name(tab->relid)));

19.
 			if (pkrel->rd_rel->relpersistence != RELPERSISTENCE_TEMP)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
-						 errmsg("constraints on temporary tables may reference only temporary tables")));
+						 errmsg("constraints on local temporary tables may reference only local temporary tables")));
 			if (!pkrel->rd_islocaltemp || !rel->rd_islocaltemp)
 				ereport(ERROR,
 						(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
-						 errmsg("constraints on temporary tables must involve temporary tables of this session")));
+						 errmsg("constraints on local temporary tables must involve local temporary tables of this session")));
+			break;
+		case RELPERSISTENCE_GLOBAL_TEMP:
+			if (!RELATION_IS_GLOBAL_TEMP(pkrel))
+				ereport(ERROR,
+						(errcode(ERRCODE_INVALID_TABLE_DEFINITION),
+						 errmsg("constraints on global temporary tables may reference only global temporary tables")));

s/global temporary table/global temporary relation/g
s/local temporary table/local temporary relation/g

20.
 		attachrel->rd_rel->relpersistence == RELPERSISTENCE_TEMP)
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				 errmsg("cannot attach a temporary relation as partition of permanent relation \"%s\"",
+				 errmsg(RELATION_IS_GLOBAL_TEMP(rel)
+						? "cannot attach a local temporary relation as partition of global temporary relation \"%s\""
+						: "cannot attach a temporary relation as partition of permanent relation \"%s\"",
 						RelationGetRelationName(rel))));

Same as 15:

    cannot attach a local temporary relation as partition of permanent relation \"%s\"

21.
 		attachrel->rd_rel->relpersistence != RELPERSISTENCE_TEMP)
 		ereport(ERROR,
 				(errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				 errmsg("cannot attach a permanent relation as partition of temporary relation \"%s\"",
+				 errmsg(RELATION_IS_GLOBAL_TEMP(attachrel)
+						? "cannot attach a global temporary relation as partition of local temporary relation \"%s\""
+						: "cannot attach a permanent relation as partition of temporary relation \"%s\"",
 						RelationGetRelationName(rel))));

Same as 16:

    cannot attach a permanent relation as partition of local temporary relation \"%s\

22.
@@ -22776,7 +22828,9 @@ createPartitionTable(List **wqueue, RangeVar *newPartName,
 		newPartName->relpersistence == RELPERSISTENCE_TEMP)
 		ereport(ERROR,
 				errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				errmsg("cannot create a temporary relation as partition of permanent relation \"%s\"",
+				errmsg(parent_relform->relpersistence == RELPERSISTENCE_GLOBAL_TEMP
+					   ? "cannot create a local temporary relation as partition of global temporary relation \"%s\""
+					   : "cannot create a temporary relation as partition of permanent relation \"%s\"",
 					   RelationGetRelationName(parent_rel)));
 
 	/* Permanent rels cannot be partitions belonging to a temporary parent. */

Suggestion:

    cannot create a local temporary relation as partition of permanent relation \"%s\"

23.
@@ -22784,7 +22838,9 @@ createPartitionTable(List **wqueue, RangeVar *newPartName,
 		parent_relform->relpersistence == RELPERSISTENCE_TEMP)
 		ereport(ERROR,
 				errcode(ERRCODE_WRONG_OBJECT_TYPE),
-				errmsg("cannot create a permanent relation as partition of temporary relation \"%s\"",
+				errmsg(newPartName->relpersistence == RELPERSISTENCE_GLOBAL_TEMP
+					   ? "cannot create a global temporary relation as partition of local temporary relation \"%s\""
+					   : "cannot create a permanent relation as partition of temporary relation \"%s\"",
 					   RelationGetRelationName(parent_rel)));

Suggest:

    cannot create a permanent relation as partition of local temporary relation \"%s\"


-- 
Regards,
Japin Li
ChengDu WenWu Information Technology Co., Ltd.