On Tue, Sep 15, 2026 at 9:59 AM Dean Rasheed <[email protected]> wrote: > > On Wed, 9 Sept 2026 at 03:18, Haibo Yan <[email protected]> wrote: > > > > I followed up on your comment in v10 that pg_temp_class and > > pg_temp_index might be better removed, with the session-local state kept > > in memory instead. > > > > I traced the lifetime and transaction semantics of the two catalogs in the > > v10 code, and then tried implementing that direction. The attached two > > patches are incremental review patches on top of your v10 series; they are > > not intended to replace or renumber the GTT series. > > Attached is v11 of this patch series. > > I finished off the refactoring that I started, getting rid of the > pg_temp_class and pg_temp_index catalog tables, and just keeping that > data in memory, attached to the usage entries. The end result is > similar to what you did, with mostly cosmetic differences. > > One noticeable difference is that I didn't bother with separate > transactional histories for the class and index fields, but instead > just lumped them together in a single structure, which saves some > amount of code duplication. I don't think that makes any practical > difference from the user's point of view. > > I also opted for 2 separate functions to retrieve information about > global temporary relations -- one that returns information for a > single relation, given its OID, and one that returns all the global > temporary relations being used in the session. > > I think all the other changes just come down to different coding > styles, and various other bits of minor tidying up. > > > psql’s same-session \d / \di behavior is preserved using > > pg_gtt_index_isvalid(). The existing direct pg_index reads in tools > > such as pg_dump, pg_upgrade, and pg_amcheck remain as they were in v10; > > those tools were not using pg_temp_index in v10 either. > > This part of my patch ended up almost identical. > > > I left pg_temp_statistic and pg_temp_statistic_ext_data unchanged. > > They carry substantially more data and fit the existing statistics tuple > > interfaces much better, so I don’t think the same argument automatically > > applies to them. > > Yes, that was my thinking. > > One thing that I did change was the where > ProcessInvalidatedGlobalTempRelations() is called. It is now called at > transaction start and command start, rather than when opening global > temporary relations. That reduces the frequency with which it is > called to what I think is the bare minimum, and it also removes the > possibility of it being called recursively. > > > First, while testing I found a pre-existing v10 issue where creating a > > global temporary sequence and then rolling the creation back can leave a > > local usage entry behind. > > Ah, good point. I fixed that by adding a new bool flag to > TrackGlobalTempRelation() so that it can distinguish between tracking > a newly created relation and initialising an already-existing one -- > for a sequence, creation needs to be transaction, just like any other > kind of relation, but initialisation needs to be non-transactional, so > that ROLLBACK doesn't reset the sequence. > > > I also did not try to address the previously discussed GTT inheritance > > case, the broader frozen-XID/autovacuum policy questions, or the missing > > Meson wiring for the two existing GTT TAP tests. > > I haven't changed these, other than to add the missing Meson wiring. > > One other thing that I found was that it was possible to make the > DISCARD GLOBAL TEMP code simpler, without pg_temp_class or > pg_temp_index to worry about, so I ended up significantly rewriting > that code. > > Aside from that, I added some additional tests, and adjusted the docs. > > I think that this is a definite improvement, for the reasons > previously stated. In particular, I'm glad to see the back of v10's > flushing code. In addition, the patch series is now 9 patches instead > of 11, and the overall line count has been significantly reduced, > which is a nice bonus. > > Regards, > Dean
Hi Dean,
Thanks for the v11 update. I spent some more time testing the new series.
Overall, the v11 refactoring looks good to me, particularly the simplified
`GtrInfo` state/history, sequence handling, invalidation processing, and the
new `DISCARD GLOBAL TEMP` implementation.
I did find a few issues around indexes and the frozen-XID handling.
The first is `CREATE UNIQUE INDEX` when another session is already
using the GTT.
I reproduced a case where session A had existing rows, session B created a
UNIQUE index, and session A could then silently insert a duplicate of one of its
pre-existing rows. Duplicates among rows inserted afterwards were
still detected,
so the effective result was only partial uniqueness enforcement.
I looked at the related paths as well, and the same underlying issue applies to
partial/expression unique indexes, `NULLS NOT DISTINCT`, partitioned-index
creation/attachment, and exclusion-enforcing indexes. In particular, I could
reproduce the exclusion case through a temporal primary key.
I think the cleanest rule is to reject creation of a unique or
exclusion-enforcing index when another backend is already using the relevant
GTT. An ordinary locally-invalid index only loses an access path, while an
incomplete unique/exclusion index loses an enforcement guarantee.
I've attached a patch implementing that check in `DefineIndex()`. It leaves
ordinary index creation and session-local `REINDEX` unchanged.
The second issue is `ON COMMIT DELETE ROWS`. I found that after the implicit
truncate, the GTT can remain empty while retaining its old local
`relfrozenxid`/`relminmxid`, which in turn keeps the backend's `tempfrozenxid`
unnecessarily old. In one test the frozen horizon remained unchanged across
hundreds of later transactions and advanced immediately after an explicit
`TRUNCATE`.
Since the physical truncate is nontransactional and the relation is known to be
empty once it succeeds, I think it is safe to refresh the GTT-local freeze
horizons at that point. The second attached patch does that using the same
horizon values used when initializing fresh heap storage, and relies on the
existing end-of-transaction GTT code to recompute the published PGPROC minimum.
I also noticed two smaller issues that I haven't included in these patches:
1.`CREATE INDEX CONCURRENTLY` / `REINDEX CONCURRENTLY` on a GTT
are currently
accepted but downgraded to non-concurrent operations. The existing
comments/docs
justify this by saying that no other session can access a
temporary relation,
which isn't true for a GTT. I think the behavior should at least
be documented
explicitly; whether GTTs should instead reject `CONCURRENTLY` or use lighter
locking seems like a separate design question.
2. `repack.c:check_index_requirements()` has one direct read of
`pg_index.indisvalid` rather than the session-effective value. The same
file already uses `GetEffective_indisvalid()` in another path, so this
looks like a small missed overlay site.
The two attached patches are independent and both apply directly on top of v11:
v1-Reject-unique-and-exclusion-indexes-on-in-use-GTTs.patch
v1-Refresh-GTT-freeze-horizons-after-ON-COMMIT-DELETE.patch
I tested each independently on top of v11 with cassert/debug builds and the
relevant regression/isolation tests, and also ran the full Meson suite with each
patch.
Regards,
Haibo
v1-Reject-unique-and-exclusion-indexes-on-in-use-GTTs.patch
Description: Binary data
v1-Refresh-GTT-freeze-horizons-after-ON-COMMIT-DELETE.patch
Description: Binary data
