MemoryContext and NodeTags
I'm rewriting parts of PL/Java to be more secure. One of the areas where
I'd like to improve things concerns ownership of allocated structures.
Many structures, such as TupleDesc, HeapTuple, ErrorData, etc. can be
copied into another MemoryContext for safe keeping. PL/Java uses this
when creating Java wrappers for such objects.
Prior to the rewrite, I maintained mappings from pointers to Java
wrappers in a hash table. Now, I instead have a special MemoryContext
that can hold a reference to the Java wrapper in the chunk header. Both
simpler and more efficient (I also have wet dreams about a future
MemoryContext that allocates shared memory). But, at present, and
because of this macro:
/*
* MemoryContextIsValid
* True iff memory context is valid.
*
* Add new context types to the set accepted by this macro.
*/
#define MemoryContextIsValid(context) \
((context) != NULL && \
(IsA((context), AllocSetContext)))
I have to cheat and claim that this MemoryContext has the NodeType of
T_AllocSetContext.
I have a proposal:
The NodeTag T_MemoryContext has the value of 600 and the next occupied
entry is T_Value which is 650.
- Reserve half of that range for PostgreSQL specific contexts (today you
only use one), and the other half for custom contexts.
- Change the above macro to consider values between 601 and 649 as valid
tags. The likelihood of an invalid context hitting that range is second
to none.
- Accept patches to nodes/nodes.h for new custom tags (properly
motivated of course).
What do you think? Would a patch that implements this proposal and adds
a T_PLJavaContext NodeTag be accepted?
Regards,
Thomas Hallgren
Thomas Hallgren <thomas.hallgren@home.se> writes:
I have a proposal:
The NodeTag T_MemoryContext has the value of 600 and the next occupied
entry is T_Value which is 650.
- Reserve half of that range for PostgreSQL specific contexts (today you
only use one), and the other half for custom contexts.
OK.
- Accept patches to nodes/nodes.h for new custom tags (properly
motivated of course).
No. Define 'em yourself.
regards, tom lane
tgl@sss.pgh.pa.us wrote:
Thomas Hallgren <thomas.hallgren@home.se> writes:
I have a proposal:
The NodeTag T_MemoryContext has the value of 600 and the next occupied
entry is T_Value which is 650.
- Reserve half of that range for PostgreSQL specific contexts (today you
only use one), and the other half for custom contexts.OK.
- Accept patches to nodes/nodes.h for new custom tags (properly
motivated of course).No. Define 'em yourself.
OK, I can do that. But I have a couple of reasons why I think that it
would be a good idea to get my definitions into node.h:
- If more module authors want to do similar things, they would not risk
defining overlapping tags.
- The NodeTag is an enum. Code that defines tags that are supposed to
"fit in" becomes ugly.
- The IsA macro can be used.
- You (PostgreSQL core) want full control over the tags. If all tags are
in nodes.h, you can move tags to other number ranges without creating a
hassle for people like me.
Regards,
Thomas Hallgren.
Thomas Hallgren <thomas.hallgren@home.se> writes:
tgl@sss.pgh.pa.us wrote:
No. Define 'em yourself.
OK, I can do that. But I have a couple of reasons why I think that it
would be a good idea to get my definitions into node.h:
- If more module authors want to do similar things, they would not risk
defining overlapping tags.
Only for those module authors who manage to get their tags accepted;
and even for them, only for PG versions later than when they start
working. Not much of an extension mechanism, is it?
- The NodeTag is an enum. Code that defines tags that are supposed to
"fit in" becomes ugly.
I don't see anyone trying to "switch" over MemoryContext tags, so this
is really pretty irrelevant. AFAICS it should work just fine to do
#define T_FooNode ((NodeTag) (T_FirstPrivateNode + 1))
- The IsA macro can be used.
Still can AFAICS --- that macro knows nothing about the enum, just about
the convention that Foo and T_Foo are related names.
- You (PostgreSQL core) want full control over the tags. If all tags are
in nodes.h, you can move tags to other number ranges without creating a
hassle for people like me.
As long as you define your tag as T_Something + N, that still holds.
regards, tom lane
tgl@sss.pgh.pa.us wrote:
Not much of an extension mechanism, is it?
Yes it is. If you are a module writer and want to define your own tag,
the chances are pretty hight that you would look for available tags in
the current CVS HEAD before you go ahead, thus avoiding any collision.
First come, first served. You will of course need #ifdef's for backward
compatibility but at some point in time, they can be removed (as I now
do with a lot of stuff for 7.x and custom variable classes as I'm
dropping the 7.x support).
If you don't get your tag accepted then you're on your own of course.
Then again, if the core team has reservations to accepting your tag
there's a bigger problem somewhere.
I don't see anyone trying to "switch" over MemoryContext tags, so this
is really pretty irrelevant. AFAICS it should work just fine to do#define T_FooNode ((NodeTag) (T_FirstPrivateNode + 1))
I don't see how #define rectifies ugliness. It's horrible in the
debugger and it screws up code-completion etc. in any IDE. Not being
able to use a "switch" is a minor problem.
- You (PostgreSQL core) want full control over the tags. If all tags are
in nodes.h, you can move tags to other number ranges without creating a
hassle for people like me.As long as you define your tag as T_Something + N, that still holds.
No, that's false. Assume a range is full and you need to expand it.
Someone within that range has to move. Everyone uses the same T_Something...
Regards,
Thomas Hallgren