Re: [PATCH][PROPOSAL] Add enum releation option type

From: Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>
To: Nikolay Shaplov <dhyan(at)nataraj(dot)su>
Cc: pgsql-hackers(at)lists(dot)postgresql(dot)org, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, Nikita Glukhov <n(dot)gluhov(at)postgrespro(dot)ru>, "pgsql-hackers(at)postgresql(dot)org" <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: [PATCH][PROPOSAL] Add enum releation option type
Date: 2019-09-16 21:45:31
Message-ID: 20190916214531.GA15390@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2019-Sep-13, Nikolay Shaplov wrote:

> I've played with it around, and did some googling, but without much success.
> If we are moving this way (an this way seems to be good one), I need you help,
> because this thing is beyond my C knowledge, I will not manage it myself.

So I kinda did it ... and didn't like the result very much.

Partly, maybe the problem is not one that this patch introduces, but
just a pre-existing problem in reloptions: namely, does an access/
module (gist, rel) depend on reloptions, or does reloptions.c depend on
access modules? It seems that there is no actual modularity separation
between these; currently both seem to depend on each other, if only
because there's a central list (arrays) of valid reloptions. If we did
away with that and instead had each module defined its own reloptions,
maybe that problem would go away. But how would that work?

Also: I'm not sure about the stuff that coerces the enum value to an int
generically; that works fine with my compiler but I'm not sure it's
kosher.

Thirdly: I'm not sure that what I suggested is syntactically valid,
because C doesn't let you define an array in an initializator in the
middle of another array. If there is, it requires some syntax trick
that I'm not familiar with. So I had to put the valid values arrays
separate from the main enum options, after all, which kinda sucks.

Finally (but this is easily fixable), with this patch the
allocate_reloption stuff is broken (needs to pstrdup() the "Valid values
are" message, which needs to be passed as a new argument).

All in all, I don't know what to think of this. It seemed like a good
idea, but maybe in practice it's not so great.

Do I hear other opinions?

--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment Content-Type Size
enum-reloptions-7.patch text/x-diff 13.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Thomas Munro 2019-09-16 22:03:20 Re: POC: Cleaning up orphaned files using undo logs
Previous Message Nikita Glukhov 2019-09-16 21:28:30 Re: SQL/JSON: functions