Re: [PATCH][PROPOSAL] Add enum releation option type

В письме от среда, 20 февраля 2019 г. 15:08:32 MSK пользователь Michael
Paquier написал:
> On Sun, Jan 06, 2019 at 06:28:11PM +0300, Nikolay Shaplov wrote:
> > Actually I am not satisfied with it too... That is why I started that
> > bigger reloptions refactoring work. So for now I would offer to keep
> > initialization as it was for other types: in initialize_reloptions using
> > [type]RelOpts[] arrays. And then I will offer patch that changes it for
> > all types and remove difference between biult-in reloptions and
> > reloptions in extensions.
> Should this be switched as waiting on author or just marked as
> returned with feedback then?
Actually I would prefer "Commited" ;-)

And speaking seriously... Anyway, this test module in src/test/modules should
be a separate patch, as it would test that all types of options are properly
reaching index extension, not only enum.

From my point of view, it can be committed without any dependency with enum
reloption. Either we first commit this patch, and then that test module will
test that enum support, or first we commit that test moudule without testing
enum support, and add tests into this enum patch.

I would prefer to commit enum first, and I would promise that I will add thus
module to the top of my dev TODO list. If it is not ok for you, then please
tell me about it and set this patch as "Waiting for author" and I will do the
test patch first.

> > 2.5 May be this src/test/modules dummy index is subject to another patch.
> > So I will start working on it right now, but we will do this work not
> > dependent to any other patches. And just add there what we need when it
> > is ready. I would actually add there some code that checks all types of
> > options, because we actually never check that option value from reloptons
> > successfully reaches extension code. I did this checks manually, when I
> > wrote that bigger reloption patch, but there is no facilities to do
> > regularly check is via test suit. Creating dummy index will create such
> > facilities.
>
> bloom is a user-facing extension, while src/test/modules are normally
> not installed (there is an exception for MSVC anyway..), so stressing
> add_enum_reloption in src/test/modules looks more appropriate to me,
> except if you can define an option which is useful for the user and is
> an enum.
Thank you for pointing me right direction. 've been waiting for it. Now I can
go on :)) So let's it be src/test/modules module...