Re: Unsafe use of relation->rd_options without checking its type

I wrote:
> Now that I've seen this I wonder which other uses of rd_options are
> potentially broken. RelationIsUsedAsCatalogTable() is hardly the
> only macro that is assuming with little justification that it's
> applied to the right kind of reloptions.
>
> We could band-aid this by having the RelationIsUsedAsCatalogTable()
> macro check relation->relkind, ...

I've pushed a hack along those lines, so that we could fix the reported
problem in the back branches.

> ... but I'm inclined to think that the
> right fix going forward is to insist that StdRdOptions, ViewOptions,
> and the other in-memory representations of reloptions ought to be
> self-identifying somehow. We could add a field to them similar to
> nodeTag, but one of the things that was envisioned to begin with
> is relation types that have StdRdOptions as the first part of a
> larger struct. I'm not sure how to make that work with a tag.

After a bit of thought, I propose the following conventions:

1. All decoded reloptions structs (anything that Relation.rd_options
could point to) shall embed these common header fields:

typedef struct BaseRdOptions
{
int32 vl_len_; /* varlena header (do not touch directly!) */
int options_id; /* code identifying specific reloptions */
/* useful data follows */
} BaseRdOptions;

We'll keep the rule that these structs have a varlena length word, since
having the struct size stored that way allows easy copying with datumCopy.

2. Basic reloptions structs that share no payload fields with anything
else will be assigned options_id values in the range 1..255.

3. A struct type that wants to extend, say, StdRdOptions with some extra
fields would use an options_id that has StdRdOptions's code in its low
order byte and a more specific identity in the next higher byte.
Similarly, one could extend it again with some identity bits placed in the
third byte. There could be up to three levels of nesting before we run
out of space in the ID word, and that seems like probably enough.

4. So if you want to test whether a particular options struct has
StdRdOptions fields, you would need to execute a test like
"(rel->rd_options->options_id & 0xFF) == STDRDOPTIONS_ID".
This could and should be embedded in a standard test macro, of course.

5. The end goal would be to have all functions/macros that access
rd_options include explicit checks that the data is what they expect,
eg instead of naively doing this:

#define RelationGetFillFactor(relation, defaultff) ((relation)->rd_options ? ((StdRdOptions *) (relation)->rd_options)->fillfactor : (defaultff))

we should do this:

#define RelationGetFillFactor(relation, defaultff) ((relation)->rd_options && IsStdRdOptions((relation)->rd_options) ? ((StdRdOptions *) (relation)->rd_options)->fillfactor : (defaultff))

Unless somebody has an objection or better idea, I'll push forward
with making this happen in HEAD.

regards, tom lane