Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions

В письме от понедельник, 9 декабря 2019 г. 12:11:17 MSK пользователь Michael
Paquier написал:
> On Fri, Dec 06, 2019 at 06:18:48PM +0300, Nikolay Shaplov wrote:
> > In the thread
> > https://www.postgresql.org/message-id/2620882.s52SJui4ql@x200m
> > I've suggested to split one big StdRdOption that is used for options
> > storage into into Options structures individual for each relkind and each
> > relam
> >
> > And here goes the last part of StrRdOptions removal patch, where
> > StdRdOptions is replaced with HeapOptions and ToastOptions.
>
> -typedef struct StdRdOptions
> +/*
> + * HeapOptions
> + * Binary representation of relation options for Heap relations.
> + */
> +typedef struct HeapOptions
>
> I think that it makes sense to split relation options dedicated to
> heap into their own parsing structure, because those options are
> actually related to the table AM heap. However, I think that this
> patch is not ambitious enough in the work which is done and that
> things could move into a more generic direction. At the end of the
> day, I'd like to think that we should have something like:
> - Heap-related reloptions are built as part of its AM handler in
> heapam_handler.c, with reloptions.c holding no more references to
> heap. At all.
> - The table AM option parsing follows a model close to what is done
> for indexes in terms of option parsing, moving the responsibility to
> define relation options to each table AM.
> - Toast is an interesting case, as table AMs may want to use toast
> tables. Or not. Robert may be able to comment more on that as he has
> worked in this area for bd12499.

Oh, yeah, I forget that relations now also have AM :-)

But the truth is that my goal is to move all code that defines all option
names, min/max values etc, move it inside am code. To move data from
boolRelOpts, intRelOpts, realRelOpts, enumRelOpts, enumRelOpts from
reloptions.c into the code that implement AMs that uses these options.

I did it for indexes in patch I've offered several years ago. Now we have also
relaion AM.

But I would prefer to fix index AM relioptions first, and then copy that
solution for relations.

Because if I frist copy AM solution from indexes to relation, then I will have
to fix it in two places.

So I would prefer to keep reloptions for relations in relations.c, only split
them into HeapOptions and ToastOptions, then change AM for indexes moving
option definition into AM's and then clone the solution for relations.

This seems to be most simple and most logical way.

PS. I've checked the patch against current master. No changes were needed, but
I am attaching a diff made against current master, just in case.