Ok, ignore my previous message. I've read the patch now and that's not an
issue. The old code path is not commented out, it's #ifdef'd conditionally on
HAVE_LONG_INT_64 is right (well it seems right, it's a bit hard to tell in
patch form).
A few comments:
1) Please don't include configure in your patch. I don't know why it's checked
into CVS but it is so that means manually removing it from any patch. It's
usually a huge portion of the diff so it's worth removing.
2) The genbki.sh change could be a bit tricky for multi-platform builds (ie
OSX). I don't really see an alternative so it's just something to note for
the folks setting that up (Hi Dave).
Actually there is an alternative but I prefer the approach you've taken.
The alternative would be to have a special value in the catalogs for 8-bit
maybe-pass-by-value data types and handle the check at run-time.
Another alternative would be to have initdb fix up these values in C code
instead of fixing them directly in the bki scripts. That seems like more
hassle than it's worth though and a bigger break with the rest.
3) You could get rid of a bunch of #ifndef HAVE_LONG_INT_64 snippets by having
a #define like INT64PASSBYVALUE which is defined to be either "true" or
"false". It might start getting confusing having three different defines
for the same thing though. But personally I hate having more #ifdefs than
necessary, it makes it hard to read the code.
4) Your problems with tsearch and timestamp etc raise an interesting problem.
We don't need to mark this in pg_control because it's a purely a run-time
issue and doesn't affect on-disk storage. However it does affect ABI
compatibility with modules. Perhaps it should be added to
PG_MODULE_MAGIC_DATA.
Actually, why isn't sizeof(Datum) in there already? Do we have any
protection against loading 64-bit compiled modules in a 32-bit server or
vice versa?
But generally this is something I've been wanting to do for a while and
basically the same approach I would have taken. It seems sound to me.
--
Gregory Stark
EnterpriseDB http://www.enterprisedb.com
Ask me about EnterpriseDB's RemoteDBA services!
