Re: Maintaining cluster order on insert

Heikki Linnakangas <heikki(at)enterprisedb(dot)com> writes:
> The implementation has changed a bit since August. I thought I had
> submitted an updated version in the winter but couldn't find it. Anyway,
> I updated and dusted off the source tree, tidied up the comments a
> little bit, and fixed some inconsistencies in pg_proc entries that made
> opr_sanity to fail.

I started looking at this patch. My first reaction is that I liked last
August's API (an independent "suggestblock" call) a lot better. I think
trying to hold an index page lock across the heap insert is an extremely
bad idea; it will hurt concurrency and possibly cause deadlocks
(especially for unique indexes).

The other question is why is execMain involved in this? That makes the
design nonfunctional for tuples inserted in any other way than through
the main executor --- COPY for instance. Also, if this is successful,
I could see using it on system catalogs eventually. I'm inclined to
think that the right design is for heap_insert to call amsuggestblock
for itself, or maybe even push that down to RelationGetBufferForTuple.
(Note: having heap_insert contain logic that duplicates
RelationGetBufferForTuple's is another bit of bad design here, but
that's at least correctable locally.) Now the difficulty in that is
that the heapam.c routines don't have hold of any data structure
containing index knowledge ... but they do have hold of the Relation
structure for the heap. I suggest making RelationGetIndexList() cache
the OID of the clustered index (if any) in relcache entries, and add
RelationGetClusterIndex much like RelationGetOidIndex, and then
heap_insert can use that.

regards, tom lane