| From: | Haibo Yan <tristan(dot)yim(at)gmail(dot)com> |
|---|---|
| To: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> |
| Cc: | Zsolt Parragi <zsolt(dot)parragi(at)percona(dot)com>, pgsql-hackers(at)lists(dot)postgresql(dot)org, Japin Li <japinli(at)hotmail(dot)com>, Kirk Wolak <wolakk(at)gmail(dot)com> |
| Subject: | Re: Global temporary tables |
| Date: | 2026-09-16 03:54:58 |
| Message-ID: | CABXr29FA3sMWtYde4RxD-UJR5-SKsFSB12Vz=GdbCsNaPYX8TQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
On Tue, Sep 15, 2026 at 9:59 AM Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
>
> On Wed, 9 Sept 2026 at 03:18, Haibo Yan <tristan(dot)yim(at)gmail(dot)com> 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
| Attachment | Content-Type | Size |
|---|---|---|
| v1-Reject-unique-and-exclusion-indexes-on-in-use-GTTs.patch | application/octet-stream | 22.4 KB |
| v1-Refresh-GTT-freeze-horizons-after-ON-COMMIT-DELETE.patch | application/octet-stream | 13.8 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | shveta malik | 2026-09-16 04:12:52 | Re: Distinguish publication exclusions in object addresses |
| Previous Message | shihao zhong | 2026-09-16 03:52:09 | Re: [PATCH] Refactor remaining zero-fill relation extensions to use smgrzeroextend() |