Tom Lane wrote:
> The source of both of those problems is that in some places, we
> did CatalogOpenIndexes and then used the CatalogIndexState for
> multiple tuple inserts/updates before doing CatalogCloseIndexes.
> The patch dealt with these either by not touching them, just
> leaving the simple_heap_insert/update calls in place (thus failing
> to create any abstraction), or by blithely ignoring the optimization
> and doing s/simple_heap_insert/CatalogTupleInsert/ anyway. For example,
> in inv_api.c we are now doing a CatalogOpenIndexes/CatalogCloseIndexes
> cycle for each chunk of the large object ... and just to add insult to
> injury, the now-useless open/close calls outside the loop are still there.
Ouch. You're right, I missed that.
> I think what we ought to do about this is invent additional API
> functions, say
>
> Oid CatalogTupleInsertWithInfo(Relation heapRel, HeapTuple tup,
> CatalogIndexState indstate);
> void CatalogTupleUpdateWithInfo(Relation heapRel, ItemPointer otid,
> HeapTuple tup, CatalogIndexState indstate);
>
> and use these in place of simple_heap_foo plus CatalogIndexInsert
> in the places where this optimization had been applied.
This looks reasonable enough to me.
> An alternative but much more complicated fix would be to get rid of
> the necessity for callers to worry about this at all, by caching
> a CatalogIndexState in the catalog's relcache entry. That might be
> worth doing eventually (because it would allow sharing index info
> collection across unrelated operations) but I don't want to do it today.
Hmm, interesting idea. No disagreement on postponing.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services