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/