Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1

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/