Re: Global temporary tables

On Thu, Sep 17, 2026 at 8:57 PM Haibo Yan <tristan(dot)yim(at)gmail(dot)com> wrote:
>
> On Thu, Sep 17, 2026 at 10:28 AM Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> wrote:
> >
> > On Wed, 16 Sept 2026 at 04:55, Haibo Yan <tristan(dot)yim(at)gmail(dot)com> wrote:
> > >
> > > 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.
> >
> > Thanks for testing.
> >
> > > 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 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.
> >
> > An alternative is for session A to mark the index as not ready
> > (indisready == false) as well as not valid locally, so that it doesn't
> > attempt to enforce uniqueness at all, until REINDEX is run. That has a
> > couple of advantages:
> >
> > 1). It doesn't act as a blocker for session B to create unique
> > indexes. Otherwise session B might have to wait for all other sessions
> > to exit, which might be a pain on a busy system.
> >
> > 2). It works better for any kind of index -- if session A continues to
> > execute INSERTs after the index is defined, it doesn't waste time
> > inserting entries into the index, which is pointless while the index
> > is invalid, because it can't be used in queries, and a later REINDEX
> > would throw the partial index away and rebuild from scratch anyway.
> >
> > Since point 2 applies to all kinds of indexes, not just unique
> > indexes, it seems worth doing, regardless of whether we reject unique
> > indexes on a table being used by another backend, but I'd prefer not
> > to reject it, if we don't have to.
> >
> > Attached is v12, doing this.
> >
> > GtrInfo now has an indisready field as well as indisvalid, and I've
> > replaced pg_gtr_index_is_valid() with pg_gtr_index_info(), which
> > returns both fields.
> >
> > > 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.
> >
> > Yes, that makes sense. I merged that into 0006, except that I didn't
> > like the way it was computing the freeze horizons in heap.c. That felt
> > like a layering violation to me, because I think it's really up to the
> > table access method to decide on the freeze horizons.
> >
> > So I modified TableAmRoutine.relation_nontransactional_truncate to
> > return freezeXid and minmulti, in the same way that
> > TableAmRoutine.relation_set_new_filelocator does. I think that makes
> > sense, because they're both used to truncate a relation, so it's nice
> > to make them consistent.
> >
> > While testing that, I noticed another issue -- vacuum() was calling
> > UpdateTempFrozenXids() and then vac_update_datfrozenxid(). However,
> > because UpdateTempFrozenXids() was only scheduling a commit-time
> > update to tempfrozenxid and tempminmxid, rather than updating them
> > immediately, the new values were not available to
> > vac_update_datfrozenxid(). So datfrozenxid would always end up a
> > little behind values in the relations just vacuumed.
> >
> > To fix that, I added an "immediate" parameter to
> > UpdateTempFrozenXids() to allow it to optionally make the updates
> > immediately. That's safe from vacuum(), because the changes it makes
> > cannot be rolled back. Every other caller has to pass immediate ==
> > false, even a non-transactional truncate, because although the effects
> > of the truncate cannot be rolled back, there might have been other
> > changes in the same transaction that can be rolled back if the
> > transaction aborts.
> >
> > > 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.
> >
> > Thinking about that some more, I think the best option is to reject
> > CONCURRENTLY for global temporary relations for now. For local
> > temporary relations several paths through the code do a lock upgrade,
> > which is OK, because no other session can access a local temporary
> > relation. For a global temporary relation, it's not OK (it could
> > deadlock).
> >
> > So v12 now explicitly rejects CONCURRENTLY for GTTs, and I've updated the docs.
> >
> > > 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.
> >
> > Ah, yes. Fixed in v12.
> >
> > I also made a couple of other changes to 0001:
> >
> > 1). I changed RelationBuildLocalRelation() to take a char reloncommit
> > parameter, instead of an enum OnCommitAction parameter (doing the
> > translation in heap_create() instead, which is its only caller). This
> > saves having to #include primnodes.h from relcache.h.
> >
> > 2). I made a plain \d display the ON COMMIT action, rather than
> > requiring \d+. It's only one line of text, and it seems like
> > sufficiently important information to always include it for temporary
> > tables.
> >
> > Regards,
> > Dean
>
> Hi Dean,
>
> Thanks for the v12 update. I tested the new `indisready` design and the other
> v12 changes in some more detail.
>
> Overall, I think the new direction works well. In particular, making a
> newly-seen
> local index both `indisready = false` and `indisvalid = false` closes the
> partial-enforcement problem from v11 cleanly. I tested ordinary, partial,
> expression, and `NULLS NOT DISTINCT` unique indexes, as well as write paths
> including INSERT, UPDATE, COPY, MERGE, prepared statements and ON CONFLICT. The
> local index remains completely inert until REINDEX, and a successful REINDEX
> transitions it back to ready/valid and restores normal enforcement.
>
> The ON COMMIT DELETE / freeze-horizon changes also look good in my testing,
> including the new immediate/deferred `UpdateTempFrozenXids()` behavior. The
> explicit rejection of CONCURRENTLY and the REPACK fix also behaved as expected.
>
> I did find two new ON CONFLICT edge cases introduced by making `indisready`
> session-local.
>
> The first is in `infer_arbiter_indexes()`.
>
> A constraint-backed index is still assumed to always be ready:
>
> Assert(idxForm->indisready);
>
> but for a GTT, `idxForm` now contains the session-effective value, so another
> session can legitimately see the constraint's index as locally not ready.
>
> I can reproduce this with:
>
> - session A populating a GTT;
> - session B creating a unique index and attaching it as a UNIQUE constraint;
> - session A issuing `INSERT ... ON CONFLICT ON CONSTRAINT ...`.
>
> On a cassert build this hits the assertion and terminates the backend. Without
> the assertion it naturally falls through to the normal:
>
> there is no unique or exclusion constraint matching the ON
> CONFLICT specification
>
> error.
>
> I attached:
>
> `v1-Fix-ON-CONFLICT-assertion-for-not-ready-GTT-index.patch`
>
> which only relaxes that assertion for global temporary relations. I also added
> an isolation test that reproduces the assertion without the fix and verifies
> the clean error with the fix.
>
> The second case involves partitioned GTTs.
>
> The planner sees the partitioned parent index, which has no local
> storage/GtrInfo and therefore appears ready from the shared catalog. At
> execution time, however, the corresponding leaf index can be locally
> `indisready = false`. `ExecCheckIndexConstraints()` correctly skips that leaf,
> but then reaches:
>
> elog(ERROR, "unexpected failure to find arbiter index");
>
>
> I changed that path so it records specifically when a requested arbiter index
> was skipped because `ii_ReadyForInserts` was false. Only that confirmed case
> gets the GTT-specific user error; any other unexpected failure still reaches
> the original internal `elog`.
>
> The resulting error is:
>
> ERROR: cannot use ON CONFLICT with global temporary table ...
> DETAIL: The arbiter index is not ready for inserts in this session.
> HINT: Use REINDEX to rebuild the index in this session.
>
> After `REINDEX TABLE` the same ON CONFLICT operation works normally.
>
> That fix is attached as:
>
> `v1-Report-not-ready-partitioned-GTT-arbiter-index.patch`
>
> with a partitioned-GTT isolation test covering both the error and the
> successful post-REINDEX behavior.
>
> Both patches are independent and apply directly on top of v12. I tested each
> separately with a cassert/debug build, 240/240 regression tests, 135/135
> isolation tests, and `debug_discard_caches = 1`. I also tested them together
> with the full regression and isolation suites.
>
> There is one other policy inconsistency I noticed but didn't include in
> either patch.
>
> `ALTER TABLE ... ADD CONSTRAINT ... UNIQUE USING INDEX` currently bypasses
> the cross-session guard that the direct `ADD CONSTRAINT ... UNIQUE (...)`
> path uses. With the new v12 not-ready model, I think there are two internally
> consistent choices:
>
> 1. add the same guard to `ATExecAddIndexConstraint()`, or
> 2. remove the existing restriction from `ATExecAddIndex()` and allow constraint
> creation to follow the same local-not-ready-until-REINDEX model.
>
> I left that unchanged because it seems more like a policy decision than a
> mechanical fix.
>
> Regards,
> Haibo

One small follow-up on the constraint path.

I noticed that these two forms currently behave differently when another
session is already using the GTT:

ALTER TABLE t ADD CONSTRAINT uq UNIQUE (a);

is rejected by the existing `ATExecAddIndex()` check, while:

CREATE UNIQUE INDEX t_a_idx ON t(a);
ALTER TABLE t ADD CONSTRAINT uq UNIQUE USING INDEX t_a_idx;

goes through `ATExecAddIndexConstraint()` and is allowed.

With the new v12 `indisready = false` model, I think either policy could be
made consistent:

- add the same cross-session check to `ATExecAddIndexConstraint()`, or
- remove the existing restriction from `ATExecAddIndex()` and let both
forms create the constraint, with the local index remaining not-ready
until REINDEX.

I didn't include a patch for this because it seems more like a policy choice
than a mechanical bug fix, but I think the two paths should probably behave
the same.

Regards,
Haibo