Re: Is custom MemoryContext prohibited? - Mailing list pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Feb 3, 2020 at 7:26 PM Andres Freund <andres@anarazel.de> wrote:
>> It seems like a good idea to still have an additional identifier for
>> each node type, for some cross checking. How about just frobbing the
>> pointer to the MemoryContextMethod slightly, and storing that in an
>> additional field? That'd be something fairly unlikely to ever be a false
>> positive, and it doesn't require dereferencing any additional memory.

> That would be fine as an internal sanity check, but if Tom is unhappy
> with the idea of having to try to make sense of a function pointer,
> he's probably going to be even less happy about trying to make sense
> of a frobbed pointer. And I would actually agree with him on that
> point.

Yeah, it seems a bit overcomplicated for what it accomplishes.

> I think we're all pursuing slightly different goals here. KaiGai's
> main goal is to make it possible for third-party code to add new kinds
> of memory contexts. My main goal is to make memory contexts not depend
> on backend-only infrastructure. Tom is concerned about debuggability.
> Your concern here is about sanity checking. There's some overlap
> between those goals but the absolute best thing for any given one of
> them might be really bad for one of the other ones; hopefully we can
> find some compromise that gets everybody the things they care about
> most.

Good summary.  I think that the combination of a magic number to identify
"this is a memory context struct" and an enum to identify the specific
type of context should meet all these goals moderately well:

* Third-party context types would have to force the compiler to take
context-type values that weren't among the known enum values ---
although they could ask us to reserve a value by adding an otherwise-
unreferenced-by-core-code enum entry, and I don't really see why
we shouldn't accept such requests.

* Frontend and backend would have to share the enum, but the list
is short enough that that shouldn't be a killer maintenance problem.
(Also, the enum field would be pretty much write-only from the
code's standpoint, so even if two programs were out of sync on it,
there would be at worst a debugging hazard.)

* The enum does what I want for debuggability, especially if we
back-stop it with a name string in the methods struct as you suggested.

* The magic value does what Andres wants for sanity checking.

            regards, tom lane