On Wed, 03 Jul 2002 10:21:40 -0400, Tom Lane <tgl@sss.pgh.pa.us>
wrote:
>Yikes. Once per routine would be one thing, but once per macro call?
There's not much of a difference, because most routines read or set
the oid only once. The most notable exception to this is heap_insert
with four oid accesses. A local variable might help here.
By having the assertion in the macro we can be sure it is called, when
it is needed, (and not called if not needed) and once we trust the new
code we can remove it easily.
>You might want to think about the fact that all the FooIsValid(fooptr)
>macros expand to just "fooptr != NULL", and not to some amazingly
>complete test of validity of the pointed-to object.
Ok, so I'll reduce the definition of AssertRelationHasOids(rel) to
just Assert(rel->rd_rel->relhasoids) and let the backend crash, if
either rel or rel->rd_rel is not a valid pointer. This seems good
enough for testing.
I'll let the AssertRelationHasOid calls in for now. They will go
away, when hasoids goes into TupleDesc.
>The way we usually handle that is #define symbols that increase the
>debuggability of a particular subsystem. Perhaps you could provide
>a symbol named something like DEBUG_TUPLE_ACCESS that causes
>more-paranoid versions of the tuple access macros to be chosen.
#ifdef DEBUG_TUPLE_ACCESS
#define AssertHoffIsValid(tup) \
AssertMacro((tup)->t_hoff == ExpectedLen(tup))
#else
#define AssertHoffIsValid(tup)
#endif
Where should DEBUG_TUPLE_ACCESS be defined or undef'd?
As regards your complaint about the patch making oid access dependent
on Relation, my POV is, that I did not introduce this dependence;
currently whether a tuple header is supposed to contain a valid oid or
not (apart from the fact, that it has a t_oid field anyway) *is*
dependent on Relation-...->hasoids, we have nothing better to look at.
If this dependence is unwanted, it shall be subject to another patch.
If there are no objections, my next steps will be
. submit a new version of this patch with the changes mentioned above
. make one or more patches that make the code compatible with both
the old and the new format (e.g. putting hasoid into TupleDesc),
attacking one issue at a time and keeping the code working after
each step
. and finally make a small patch that only changes htup.h
Servus
Manfred
