Re: Is custom MemoryContext prohibited?

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Andres Freund <andres(at)anarazel(dot)de>, Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>, Kohei KaiGai <kaigai(at)heterodb(dot)com>, Pg Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Is custom MemoryContext prohibited?
Date: 2020-02-05 15:20:05
Message-ID: 3320.1580916005@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)com> writes:
> On Mon, Feb 3, 2020 at 7:26 PM Andres Freund <andres(at)anarazel(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Tom Lane 2020-02-05 15:21:11 Re: Add %x to PROMPT1 and PROMPT2
Previous Message Robert Haas 2020-02-05 15:13:44 Re: Is custom MemoryContext prohibited?