В письме от четверг, 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)
