On Sat, 2024-03-30 at 23:33 +0200, Alexander Korotkov wrote:
> I've pushed 0001, 0002 and 0006.
Sorry to jump in to this discussion so late. I had worked on something
like the custom reloptions (0002) in the past, and there were some
complications that don't seem to be addressed in commit c95c25f9af.
* At minimum I think it needs some direction (comments, docs, tests)
that show how it's supposed to be used.
* The bytea returned by the reloptions() method is not in a trivial
format. It's a StdRelOptions struct with string values stored after the
end of the struct. To build the bytea internally, there's some
infrastructure like allocateRelOptStruct() and fillRelOptions(), and
it's not very easy to extend those to support a few custom options.
* If we ever decide to add a string option to StdRdOptions, I think the
design breaks, because the code that looks for those string values
wouldn't know how to skip over the custom options. Perhaps we can just
promise to never do that, but we should make it explicit somehow.
* Most existing heap reloptions (other than fillfactor) are used by
other parts of the system (like autovacuum) so should be considered
valid for any AM. Most AMs will just want to add a handful of their own
options on top, so it would be good to demonstrate how this should be
done.
* There are still places that are inappropriately calling
heap_reloptions directly. For instance, in ProcessUtilitySlow(), it
seems to assume that a toast table is a heap?
Regards,
Jeff Davis
