Re: Global temporary tables

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