Gregory Stark írta:
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.
Noted.
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.
OK, this would also make the patch smaller.
Is pg_config_manual.h good for this setting?
Or which header would you suggest?
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.
I am looking into it.
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?
You can't mix 32-bit executables with 64-bit shared libraries, well,
without tricks.
I don't see any problem here.
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.
Thanks for commenting and encouragement.
--
----------------------------------
Zoltán Böszörményi
Cybertec Schönig & Schönig GmbH
http://www.postgresql.at/
--
Sent via pgsql-patches mailing list (pgsql-patches@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-patches