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

В письме от четверг, 3 января 2019 г. 18:12:05 MSK пользователь Alvaro Herrera
написал:
> Attached version 7, with some renaming and rewording of comments.
> (I also pgindented. Some things are not pretty because of lack of
> typedefs.list patching ... a minor issue at worst.)
Thanks! Imported it into my code tree..

> I'm still not satisfied with the way the builtin enum tables are passed.
> Can we settle for using add_enum_reloption instead at initialization
> time? Maybe that would be less ugly. Alternatively, we can have
> another member in relopt_enum which is a function pointer that returns
> the array of relopt_enum_elt_def. Not sure at which point to call that
> function, though.
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.

> I think it would be great to have a new enum option in, say,
> contrib/bloom, to make sure the add_enum_reloption stuff works
> correctly. If there's nothing obvious to add there, let's add something
> to src/test/modules.
As far as I can see there is no obvious place in bloom where we can add enum
options.

So I see several options here:

1. I can try to create some dummy index in src/test/modules. And it would be
very useful for many other tests. For example we will have no real string
reloption when this patch is applied. But it may take a reasonable time to do,
because I've never created indexes before, and I do not have many time-slots
for postgres development in my schedule.

2. Actually I am going to offer patch that will remove difference between
build-in and extension reloptions, all reloptions will use same API. After
applying that patch we would not need to test add_[type]_reloption calls
separately. So may be it would be good enough to check that add_enum_reloption
works right now (add an enum option to bloom, check that it works, and do not
commit that code anywhere) and then wait until we get rid of
add_[type]_reloption API.
This will save us some time. I am a bit worrying about Nikita Glukhov patch it
is better to have reloptions reworked before adding opclass options.

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.

For me all options are good enough, so it is for you too choose, I will do as
you advices.