В письме от пятница, 4 июня 2021 г. 3:19:09 MSK пользователь Jeff Davis написал:
> > Moving reloptions to AM code is the goal I am slowly moving to. I've > > started > > some time ago with big patch > > https://commitfest.postgresql.org/14/992/ and > > have been told to split it into smaller parts. So I did, and this > > patch is > > last step that cleans options related things up, and then actual > > moving can be > > done. > > Thank you for working on this. Welcome! Sorry for slow reply, I am on summer vacations now, no chance for fast replies now :-) > Can you outline the plan for moving these options to the table AM to > make sure this patch is a step in the right direction? Yes, I can. First you can see the whole patch, the way it was several years ago: https://commitfest.postgresql.org/15/992/, reloptions8a.diff The things I would say is accurate for postgres ~11, it may have been changed since I last payed attention to them. So, there are three general places where options can be stored: 1. Global boolRelOpts, intRelOpts, realRelOpts, stringRelOpts in src/backend/ access/common/reloptions.c for in-core access methods. 2. custom_options array of accessable via add_bool_reloption, add_int_reloption, add_real_reloption, add_string_reloption for access methods from contribs. (See reloptions.c too) 3. And also each access method has an array of relopt_parse_elt[] that is also about reloptions. 1 and 2 are technically arrays of relopt_get, and store information what kind of options do we have. 3 is array of relopt_parse_elt[] that store information how options should be stored into binary representation. My idea was to merge relopt_get and relopt_parse_elt into one structure (in my patch it is "option_definition_basic"), each access method, that needs options, should have a set of option_definition_basic for their options (called option_definition_basic in my patch, may be should be renamed) and this set of option_definitions is the only data that is needed to parse option into binary representation. So in access method instead of am_option function we will have amrelopt_catalog function that returns "options defenition set" for this am, and this definition set is used by option parser to parse options. So, it my explanation is not quite clear, please ask questions, I will try to answer them. > I was trying to work through this problem as well[1], and there are a > few complications. > > * Which options apply to any relation (of any table AM), and which > apply to only heaps? As far as I can tell, the only one that seems > heap-specific is "fillfactor". From my point of view, each relation kind has it's own set of options. The fact, that almost every kind has a fillfactor is just a coincidence. If we try to do some optimization here, we will be buried under the complexity of it soon. So they are _different_ options just having the same name. > * Toast tables can be any AM, as well, so if we accept new reloptions > for a custom AM, we also need to accept options for toast tables of > that AM. When I wrote this patch, AM was introduced only to index relations. I do not how it is implemented for heap now, but there should be some logic in it. If toast tables has it's own AM, then option definition set should be stored there, and we should find a way to work with it, somehow. > * Implementation-wise, the bytea representation of the options is not > very easy to extend. Should we have a new text field in the catalog to > hold the custom options? I am not really understanding this question. Normally all options can be well represented as binary structure stored at bytea. I see no problem here. If we need some unusual behaviour, we can use string option with custom validation function. This should cover almost all needs I can imagine. ======= So it you are interested in having better option implementation, and has no ideas of your own, I would suggest to revive my patch and try to commit it. What I need first of all is a reviewer. Testing and coauthoring will also be apriciated. My original big patch, I gave you link to, have been split into several parts. The last minor part, that should be commit in advance, and have not been commit yet is https://commitfest.postgresql.org/33/2370/ If you join as a reviewer this would be splendid! :-) -- Nikolay Shaplov dh...@nataraj.su (e-mail, jabber) @dhyan:nataraj.su (matrix)
