Re: Large expressions in indexes can't be stored (non-TOASTable)

On Mon, Sep 23, 2024 at 10:50:21AM -0500, Nathan Bossart wrote:
> I carefully inspected all the code paths this patch touches, and I think
> I've got all the details right, but I would be grateful if someone else
> could take a look.

No objections from here with putting the snapshots pops and pushes
outside the inner routines of reindex/drop concurrently, meaning that
ReindexRelationConcurrently(), DefineIndex() and index_drop() are fine
to do these operations.

Looking at the patch, we could just add an assertion based on
ActiveSnapshotSet() in index_set_state_flags().

Actually, thinking more... Could it be better to have some more
sanity checks in the stack outside the toast code for catalogs with
toast tables? For example, I could imagine adding a check in
CatalogTupleUpdate() so as all catalog accessed that have a toast
relation require an active snapshot. That would make checks more
aggressive, because we would not need any toast data in a catalog to
make sure that there is a snapshot set. This strikes me as something
we could do better to improve the detection of failures like the one
reported by Alexander when updating catalog tuples as this can be
triggered each time we do a CatalogTupleUpdate() when dirtying a
catalog tuple. The idea is then to have something before the
HaveRegisteredOrActiveSnapshot() in the toast internals, for catalogs,
and we would not require toast data to detect problems.
--
Michael