Re: temp table on commit delete rows performance issue

Hi Floris,

> On Jul 18, 2024, at 21:36, Floris Van Nee <florisvannee(at)Optiver(dot)com> wrote:
>
>
>> I also encountered the similar performance issue with temporary tables
>> andprovided a patch to optimize the truncate performance during commit
>> in [1].
>
> Interesting, that is definitely another good way to improve the performance,
> especially with a large number of temp tables. I think the two optimizations
> can actually work well together.
> Your optimization on only truncating the tables that are actually used.
> Combined with a patch like attached which makes sure that no WAL is generated at all
> for the ON COMMIT DELETE ROWS operation.

It seems that in your patch, WAL logging is skipped for all tables, not just
temporary tables.

Upon further consideration, do we really need to acquire AccessExclusiveLocks
for temporary tables? Since temporary tables can only be accessed within the
current session, perhaps we can make the following optimizations:
```
diff --git a/src/backend/catalog/heap.c b/src/backend/catalog/heap.c
index 00074c8a94..845c9603e2 100644
--- a/src/backend/catalog/heap.c
+++ b/src/backend/catalog/heap.c
@@ -2977,9 +2977,17 @@ RelationTruncateIndexes(Relation heapRelation)
Oid indexId = lfirst_oid(indlist);
Relation currentIndex;
IndexInfo *indexInfo;
+ LOCKMODE lockmode;

- /* Open the index relation; use exclusive lock, just to be sure */
- currentIndex = index_open(indexId, AccessExclusiveLock);
+ /*
+ * Open the index relation; use exclusive lock, just to be sure.
+ * AccessExclusiveLock is not necessary for temporary tables.
+ */
+ if (heapRelation->rd_rel->relpersistence != RELPERSISTENCE_TEMP)
+ lockmode = AccessExclusiveLock;
+ else
+ lockmode = ExclusiveLock;
+ currentIndex = index_open(indexId, lockmode);

/*
* Fetch info needed for index_build. Since we know there are no
@@ -3026,7 +3034,9 @@ heap_truncate(List *relids)
Oid rid = lfirst_oid(cell);
Relation rel;

- rel = table_open(rid, AccessExclusiveLock);
+ /* AccessExclusiveLock is not necessary for temporary tables. */
+ rel = table_open(rid, ExclusiveLock);
+ Assert(rel->rd_rel->relpersistence == RELPERSISTENCE_TEMP);
relations = lappend(relations, rel);
}

@@ -3059,6 +3069,7 @@ void
heap_truncate_one_rel(Relation rel)
{
Oid toastrelid;
+ LOCKMODE lockmode;

/*
* Truncate the relation. Partitioned tables have no storage, so there is
@@ -3073,11 +3084,17 @@ heap_truncate_one_rel(Relation rel)
/* If the relation has indexes, truncate the indexes too */
RelationTruncateIndexes(rel);

+ /* AccessExclusiveLock is not necessary for temporary tables. */
+ if (rel->rd_rel->relpersistence != RELPERSISTENCE_TEMP)
+ lockmode = AccessExclusiveLock;
+ else
+ lockmode = ExclusiveLock;
+
/* If there is a toast table, truncate that too */
toastrelid = rel->rd_rel->reltoastrelid;
if (OidIsValid(toastrelid))
{
- Relation toastrel = table_open(toastrelid, AccessExclusiveLock);
+ Relation toastrel = table_open(toastrelid, lockmode);

table_relation_nontransactional_truncate(toastrel);
RelationTruncateIndexes(toastrel);
```

Best Regards,
Fei Changhong