On Wed, Feb 5, 2020 at 10:23 PM Andres Freund <and...@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