Re: [PATCH] Finally split StdRdOptions into HeapOptions andToastOptions - Mailing list pgsql-hackers

From Michael Paquier
Subject Re: [PATCH] Finally split StdRdOptions into HeapOptions andToastOptions
Date
Msg-id 20200307010340.GA1531@paquier.xyz
Whole thread Raw
In response to Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions  (Nikolay Shaplov <dhyan@nataraj.su>)
Responses Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions  (Nikolay Shaplov <dhyan@nataraj.su>)
List pgsql-hackers
On Wed, Mar 04, 2020 at 10:58:31PM +0300, Nikolay Shaplov wrote:
> 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.

How do you think that this part should be changed then, if this needs
any changes?  It seems to me that we have a rather correct layer for
index AMs by requiring each one to define the available option set
using indoptions through their handler, with option fetching macros
located within each AM.

> Because if I first 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.

Then, for table AMs, it seems to me that you are right for long-term
perspective to have the toast-related options in reloptions.c, or
perhaps directly located within more toast-related file (?) as table
AMs interact with toast using heapam_relation_needs_toast_table and
such callbacks.  So for heap, moving the option handling to roughly
heapam_handler.c is a natural move, though this requires a redesign of
the existing structure to use option handling closer to what
indoptions does, but for tables.
--
Michael

Attachment

pgsql-hackers by date:

Previous
From: Nikita Glukhov
Date:
Subject: Re: Ltree syntax improvement
Next
From: Michael Paquier
Date:
Subject: Re: PG_COLOR not mentioned in docs of vacuumlo, oid2name and pgbench