On Tue, Jan 28, 2020 at 10:35 AM Tom Lane <t...@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
v1-0001-Teach-MemoryContext-infrastructure-not-to-depend-.patch
Description: Binary data