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

Reply via email to