Skip site navigation (1) Skip section navigation (2)

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

From: Gregory Stark <stark(at)enterprisedb(dot)com>
To: "Zoltan Boszormenyi" <zb(at)cybertec(dot)at>
Cc: <pgsql-patches(at)postgresql(dot)org>, "Hans-Juergen Schoenig" <hs(at)cybertec(dot)at>
Subject: Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
Date: 2008-03-24 23:25:18
Message-ID: 87lk47ogbl.fsf@oxford.xeocode.com (view raw or flat)
Thread:
Lists: pgsql-patches
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!

In response to

Responses

pgsql-patches by date

Next:From: Zoltan BoszormenyiDate: 2008-03-24 23:38:33
Subject: Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1
Previous:From: Zoltan BoszormenyiDate: 2008-03-24 23:15:05
Subject: Re: int8/float8/time/timestamp[tz]/float4 passed by value, was Re: Fix HAVE_LONG[_LONG]_INT_64 to really define to 1

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group