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

From: Michael Paquier <michael(at)paquier(dot)xyz>
To: Nikolay Shaplov <dhyan(at)nataraj(dot)su>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, Amit Langote <amitlangote09(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
Subject: Re: [PATCH] Finally split StdRdOptions into HeapOptions and ToastOptions
Date: 2020-03-07 01:03:40
Message-ID: 20200307010340.GA1531@paquier.xyz
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: 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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Michael Paquier 2020-03-07 01:09:23 Re: PG_COLOR not mentioned in docs of vacuumlo, oid2name and pgbench
Previous Message Nikita Glukhov 2020-03-07 00:59:15 Re: Ltree syntax improvement