Re: [PATCH] Do not use StdRdOptions in Access Methods

В письме от четверг, 14 ноября 2019 г. 16:50:18 MSK пользователь Michael 
Paquier написал:

> > I've changed the patch to use build_reloptions function and again propose
> > it to the commitfest.
> 
> Thanks for the new patch.  I have not reviewed the patch in details,
> but I have a small comment.
> 
> > +#define SpGistGetFillFactor(relation) \
> > +	((relation)->rd_options ? \
> > +		((SpGistOptions *) (relation)->rd_options)->fillfactor : \
> > +		SPGIST_DEFAULT_FILLFACTOR)
> > +
> 
> Wouldn't it make sense to add assertions here to make sure that the
> relkind is an index?  You basically did that in commit 3967737.

For me there is no mush sense in it, as it does not prevent us from wrong type 
casting. Indexes can use all kinds of structures for reloptions, and checking 
that this is index, will not do much.

Do you know way how to distinguish one index from another? If we can check in 
assertion this is index, and this index is spgist, then assertion will make 
sense for 100%. I just have no idea how to do it. As far as I can see it is 
not possible now.


Another issue here is, that we should to it to all indexes, not only that have 
been using StdRdOptions, but to all indexes we have. (Damn code 
inconsistency). So I guess this should go as another patch to keep it step by 
step improvements.



-- 
Software Developer: https://www.upwork.com/freelancers/~014a87e140ff02c0da
Body-oriented Therapist: https://vk.com/nataraj_rebalancing  (Russian)