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