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)