On Tue, Jan 28, 2020 at 10:35 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
> I don't actually believe that private context types in extensions is
> a very likely use-case, so on the whole I'd just as soon leave this
> alone. If we did want to do something, I'd vote for one NodeTag
> code T_MemoryContext and then a secondary ID field that's an enum
> over specific memory context types.
I generally like this idea, but I'd like to propose that we instead
replace the NodeTag with a 4-byte magic number. I was studying how
feasible it would be to make memory contexts available in frontend
code, and it doesn't look all that bad, but one of the downsides is
that nodes/memnodes.h includes nodes/nodes.h, and I think it would not
be a good idea to make frontend code depend on nodes/nodes.h, which
seems like it's really a backend-only piece of infrastructure. Using a
magic number would allow us to avoid that, and it doesn't really cost
anything, because the memory context nodes really don't participate in
any of that infrastructure anyway.
Along with that, I think we could also change MemoryContextIsValid()
to just check the magic number and not validate the type field. I
think the spirit of the code is just to check that we've got some kind
of memory context rather than random garbage in memory, and checking
the magic number is good enough for that. It's possibly a little
better than what we have now, since a node tag is a small integer,
which might be more likely to occur at a random pointer address. At
any rate I think it's no worse.
Proposed patch attached.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
