Re: [PATCH] get rid of StdRdOptions, use individual binary reloptions representation for each relation kind instead - Mailing list pgsql-hackers

В письме от 1 марта 2018 16:15:32 пользователь Andres Freund написал:

> > This is part or my bigger patch
> > https://www.postgresql.org/message-id/flat/2146419.veIEZdk4E4@x200m#21464
> > 19.veIEZdk4E4@x200m we've decided to commit by smaller parts.
>
> I've not read that thread. Is this supposed to be a first step towards
> something larger?

I've started from the idea of opclass options. Same as here
https://commitfest.postgresql.org/17/1559/
And found out that current reloptions code is not flexible at all, and could
not be easily reused for other kind of options (that are not reloptions).

So I tried to change reloptions code to make universal, and then use it for
any option purposes.

In the patch https://commitfest.postgresql.org/14/992/ I created a new file
options.c, that works with options as with an abstraction. It has some option
definition structure, that has all information about how this options should
be parsed and transformed into binary representation, I called this structure
OptionsCatalog, but name can be changed. And it also have all kind of
functions that do all needed tranformation with options.

Then reloptions.c uses functions from options options.c for all options
transformations, and takes care about relation specificity of reloptions.

While doing all of these, I first of all had to adjust code that was written
around reloptions, so that there code would stay in tune with new reloptions
code. And second, I met places where current reloption related code were
imperfect, and I decided to change it too...

Since I get a really big patch as a result, it was decided to commit it in
parts.

This patch is the adjusting of reloption related code. It does not change a
great deal, but when you first commit it, the main reloptions refactoring
patch will no longer look like a big mess, it will became much more logical.

Enum options patch is more about making coder a little bit more perfect. It is
not really needed for the main patch, but it is more easy to introduce it
before, then after.
https://commitfest.postgresql.org/17/1489/

There is one more patch, that deals with toast.* options for tables with no
TOAST, but we can set it aside, it is independent enough, to deal with it
later...
https://commitfest.postgresql.org/17/1486/

The patch with reloptions tests I've written while working on this patch, were
already commited
https://commitfest.postgresql.org/15/1314/


> > So for example if you set custom fillfactor value for some index, then it
> > will lead to allocating 84 bytes of memory (sizeof StdRdOptions on i386)
> > and only 8 bytes will be actually used (varlena header and fillfactor
> > value). 74 bytes is not much, but allocating them for each index for no
> > particular reason is bad idea, as I think.
> I'm not sure this is a particularly strong motivator though?  Does the
> patch have a goal besides saving a few bytes?

My real motivation for this patch is to make code more uniform. So the patch
over it will be much clearer. And to make result code more clear and uniform
too.

I guess long time ago, the StdRdOptions were the way to make code uniform.
There were only one binary representation of options, and all relation were
using it. Quite uniform.

But now, some relation kinds uses it's own options binary representations.
Some still uses StdRdOptions, but only some elements from it. It is not
uniform enough for me.

If start using individual binary representation for each relation kind, then
code will be uniform and homogeneous again, and my next patch over it will be
more easy to read and understand.

> > Moreover when I wrote my big reloption refactoring patch, I came to "one
> > reloption kind - one binary representation" philosophy. It allows to write
> > code with more logic in it.
> I have no idea what this means?
I hope I managed to explain it above... May be I am not good enough with
explaining, or with English. Or with explaining in English. If it is not clear
enough, please tell me, what points are not clear, I'll try to explain more...


> Are you aiming this for v11 or v12?
I hope to commit StdRdOptions and Enum_Options patches before v11.
Hope it does not go against any rules, since there patches does not change any
big functionality, just do some refactoring and optimization, of the code that
already exists.

As for main reloptions refactoring patch, if there is a chance to get commited
before v11 release, it would be great. If not, I hope to post it to
commitfest, right after v11 were released, so it would be possible to commit
opclass options upon it before v12 release.


--
Do code for fun.