Robert Haas <robertmhaas@gmail.com> writes:
> 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.
Yeah, there's something to be said for that. It's unlikely that it'd
ever make sense for us to have copy/equal/write/read/etc support for
memory context headers, so having them be part of the Node taxonomy
doesn't seem very necessary.
> Along with that, I think we could also change MemoryContextIsValid()
> to just check the magic number and not validate the type field.
Right, that's isomorphic to what I was imagining: there'd be just
one check not N.
> Proposed patch attached.
I strongly object to having the subtype field be just "char".
I want it to be declared "MemoryContextType", so that gdb will
still be telling me explicitly what struct type this really is.
I realize that you probably did that for alignment reasons, but
maybe we could shave the magic number down to 2 bytes, and then
rearrange the field order? Or just not sweat so much about
wasting a longword here. Having those bools up at the front
is pretty ugly anyway.
regards, tom lane