On Fri, Sep 20, 2019 at 09:16:58AM +0900, Michael Paquier wrote:
> On Thu, Sep 19, 2019 at 02:13:23PM +0300, Nikolay Shaplov wrote:
>> What a good catch! dummy_index already proved to be useful ;-)
>
> Yes, the testing around custom reloptions is rather poor now, so this
> module has value. I still don't like much its shape though, so I
> began hacking on it for integration, and I wanted to get that part
> done in this CF :)
So... I have looked at the patch of upthread in details, and as I
suspected the module is over-designed. First, on HEAD the coverage of
reloptions.c is 86.6%, with your patch we get at 94.1%, and with the
attached I reach 95.1% thanks to the addition of a string parameter
with a NULL default value and a NULL description, for roughly half the
code size.
The GUCs are also basically not necessary, as you can just replace the
various WARNING calls (please don't call elog on anything which can be
reached by the user!) by lookups at reloptions in pg_class. Once this
is removed, the whole code gets more simple, and there is no point in
having neither a separate header nor a different set of files and the
size of the whole module gets really reduced.
I still need to do an extra pass on the code (particularly the AM
part), but I think that we could commit that. Please note that I
included the fix for the lockmode I sent today so as the patch can be
tested:
https://www.postgresql.org/message-id/20190920013831.GD1844@paquier.xyz
Thoughts?
--
Michael