> The way that seems acceptable to me is to add (optional) nulls mask into > the end of existing style SpGistLeafTuple header and use indextuple > routines to attach attributes after it. In this case, we can reduce the > amount of code at the cost of adding one extra MAXALIGN size to the overall > tuple size on 32-bit arch as now tuple header size of 12 bit already fits 3 > MAXALIGNS (on 64 bit the header now is shorter than 2 maxaligns (12 bytes > of 16) and nulls mask will be free of cost). If you mean this I try to make > changes soon. What do you think of it?
Yeah, that was pretty much the same conclusion I came to. For INDEX_MAX_KEYS values up to 32, the nulls bitmap will fit into what's now padding space on 64-bit machines. For backwards compatibility, we'd have to be careful that the code knows there's no nulls bitmap in an index without included columns, so I'm not sure how messy that will be. But it's worth trying that way to see how it comes out.
I made a refactoring of the patch code according to the discussion:
1. Changed a leaf tuple format to: header - (optional) bitmask - key value - (optional) INCLUDE values
2. Re-use existing code of heap_fill_tuple() to fill data part of a leaf tuple
3. Splitted index_deform_tuple() into two portions: (a) bigger 'inner' one - index_deform_anyheader_tuple() - to make processing of index-like tuples (now IndexTuple and SpGistLeafTuple) working independent from type of tuple header. (b) a small 'outer' index_deform_tuple() and spgDeformLeafTuple() to make all header-specific processing and then call the inner (a)
4. Inserted a tuple descriptor into the SpGistCache chunk of memory. So cleaning the cached chunk will also invalidate the tuple descriptor and not make it dangling or leaked. This also allows not to build it every time unless the cache is invalidated.
5. Corrected amroutine->amcaninclude according to new upstream fix.
6. Returned big chunks that were shifted in spgist_private.h to their initial places where possible and made other cosmetic changes to improve the patch.
PFA v.11 of the patch.
Do you think the proposed changes are in the right direction?