Re: [PATCH] Covering SPGiST index

I've noticed CI error due to the fact that MSVC doesn't allow arrays of
flexible size arrays and made a fix for the issue.
Also did some minor refinement in tuple creation.
PFA v12 of a patch.

чт, 26 нояб. 2020 г. в 21:48, Pavel Borisov <pashkin(dot)elfe(at)gmail(dot)com>:

> > 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?
>
> Thank you!
> --
> Best regards,
> Pavel Borisov
>
> Postgres Professional: http://postgrespro.com <http://www.postgrespro.com>
>

--
Best regards,
Pavel Borisov

Postgres Professional: http://postgrespro.com <http://www.postgrespro.com>