| From: | Japin Li <japinli(at)hotmail(dot)com> |
|---|---|
| To: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> |
| Cc: | Kirk Wolak <wolakk(at)gmail(dot)com>, Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>, Andrew Dunstan <andrew(at)dunslane(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Subject: | Re: Global temporary tables |
| Date: | 2026-07-02 09:11:35 |
| Message-ID: | SY7PR01MB1092186BA253F11C5AD691046B6F52@SY7PR01MB10921.ausprd01.prod.outlook.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi, Dean
On Wed, 01 Jul 2026 at 08:32, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
> On Wed, 1 Jul 2026 at 07:24, Japin Li <japinli(at)hotmail(dot)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.
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Jakub Wartak | 2026-07-02 09:24:21 | Re: Adding basic NUMA awareness |
| Previous Message | Amit Kapila | 2026-07-02 09:03:47 | Re: Support EXCEPT for TABLES IN SCHEMA publications |