Re: per-tablespace random_page_cost/seq_page_cost

On Tue, Oct 27, 2009 at 9:13 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> I confess that I'm a bit mystified about the design of the reloptions
>> stuff. It seems kind of odd to store structured data as text[]; it's
>> kind of the opposite of what I would normally recommend as good
>> database design.
>
> It's definitely a bit EAV-ish :-(.  But I don't see any particularly
> easy way to modify it to store bool/int/float parameters in their native
> types; do you?

Looking at this a little more, it seems that part of the motivation
for the existing design is that user-created AMs might require
arbitrary options, which therefore can't be wired into the system
catalogs.  There's no equivalent for tablespaces (we could add one
some day, I suppose), so there's less intrinsic reason to think we
have to do it that way.

It doesn't actually look like it would be too terrible to un-hard-wire
the on-disk representation.  The existing transformRelOptions() and
untransformRelOptions() code could be made to handle whatever is
stored in reloptions[]; but we could arrange to remove (in the case of
transform) or insert (in the case untransform) the DefElems for any
options stored elsewhere before those functions are called.  There is
an existing hack to do this for "oids" which could probably be cleaned
up and made part of some more general structure.

However... as you basically already said, it's not entirely clear that
this solves any real problem.  The problem is not so much that the
design is too tightly coupled to a particular storage representation
as that it is too tightly coupled to pg_class - starting with the name
reloptions, which would be inapposite for options associated with a
tablespace, schema, etc.  That hasn't stopped the foreign data wrapper
stuff from reaching in and unceremoniously borrowing a few functions
like transformRelOptions, whose comment is now mildly incorrect in
enumerating the ways the function is used.

I don't see anything in this code that is very rel-specific, so I
think it would be possible to implement spcoptions by just defining
RELOPT_KIND_TABLESPACE and ignoring the irony, but that has enough of
an unsavory feeling that I'm sure someone is going to complain about
it...  I suppose we could go through and systematically rename all
instances of reloptions to ent(ity)options or storageoptions or
gen(eric)options or somesuch...

...Robert