On Wed, Feb 5, 2020 at 10:23 PM Andres Freund <andres@anarazel.de> wrote:
> I still don't get what reason there is to not use T_MemoryContext as the
> magic number, instead of something randomly new. It's really not
> problematic to expose those numerical values. And it means that the
> first bit of visual inspection is going to be the same as it always has
> been, and the same as it works for most other types one regularly
> inspects in postgres.
Without trying to say that my thought process is necessarily correct,
I can explain my thought process.
Many years ago, I had Tom slap down a patch of mine for making
something participate in the Node system unnecessarily. He pointed
out, then and at other times, that there was no reason for everything
to be part of the giant enum just because many things need to be. At
the time, I was a bit perplexed by his feedback, but over time I've
come to see the point. We've got lots of "enum" fields all over the
backend whose purpose it is to decide whether a particular object is
of one sub-type or another. We've also got NodeTag, which is that same
thing at a very large scale. I used to think that the reason why we
had jammed everything into NodeTag was just programming laziness or
some kind of desire for consistency, but now I think that the real
point is that making something a node allows it to participate in
readfuncs.c, outfuncs.c, copyfuncs.c, equalfuncs.c; so that a complex
data structure made entirely of nodes can be easily copied,
serialized, etc. The MemoryContext stuff participates in none of that
machinery, and it would be difficult to make it do so, and therefore
does not really need to be part of the Node system at all. The fact
that it *is* a part of the node system is just a historical accident,
or so I think. Sure, it's not an inconvenient thing to see a NodeTag
on random stuff that you're inspecting with a debugger, but if we took
that argument to its logical conclusion we would, I think, end up
needing to add node tags to a lot of stuff that doesn't have them now
- like TupleTableSlots, for example.
Also, as a general rule, every Node of a given type is expected to
have the same structure, which wouldn't be true here, because there
are multiple types of memory contexts that can exist, and
T_MemoryContext would identify them all. It's true that there are some
other weird exceptions, but it doesn't seem like a great idea to
create more.
Between those concerns, and those I wrote about in my last post, it
seemed to me that it made more sense to try to break the dependency
between palloc.h and nodes.h rather than to embrace it.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company