Re: Global temporary tables - Mailing list pgsql-hackers

From Japin Li
Subject Re: Global temporary tables
Date
Msg-id SY7PR01MB1092186BA253F11C5AD691046B6F52@SY7PR01MB10921.ausprd01.prod.outlook.com
Whole thread
In response to Global temporary tables  (Dean Rasheed <dean.a.rasheed@gmail.com>)
Responses Re: Global temporary tables
List pgsql-hackers
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
ofdatabase 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
beingused 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.



pgsql-hackers by date:

Previous
From: Amit Kapila
Date:
Subject: Re: Support EXCEPT for TABLES IN SCHEMA publications
Next
From: Jakub Wartak
Date:
Subject: Re: Adding basic NUMA awareness