From: | Gavin Sherry <swm(at)alcove(dot)com(dot)au> |
---|---|
To: | Gregory Stark <stark(at)enterprisedb(dot)com> |
Cc: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, pgsql-patches(at)postgresql(dot)org |
Subject: | Re: datum passed to macro which expects a pointer |
Date: | 2008-04-13 01:35:24 |
Message-ID: | 20080413013524.GJ26250@europa.idg.com.au |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-patches |
On Sun, Apr 13, 2008 at 01:42:02AM +0100, Gregory Stark wrote:
> "Gavin Sherry" <swm(at)alcove(dot)com(dot)au> writes:
>
> > On Sat, Apr 12, 2008 at 07:07:48PM -0400, Tom Lane wrote:
> >> Gavin Sherry <swm(at)alcove(dot)com(dot)au> writes:
> >> > I wish. It was actually thrown up when we (Greenplum) changed the macros
> >> > to be inline functions as part of changing Datum to be 8 bytes.
> >>
> >> Hmmm ... Datum has been 8 bytes for many years, on 64-bit machines.
> >> What is it you're trying to accomplish by making it wider on 32-bitters?
> >
> > I miss stated there. This was actually about making key 64 bit types
> > pass by value instead of pass by reference.
>
> There was a patch to do this posted recently here as well.
>
> http://archives.postgresql.org/pgsql-patches/2008-03/msg00335.php
>
> Hm. I suppose it's true that you could make Datum 64-bit even on 32-bit
> machines and make int8 and float8 pass-by-value. Seems unlikely to be a net
> win though.
A very quick scan showed me that one bet is missed in this patch which
we learned about the hard way: write_auth_file() assumes timestamptz is
pass by reference.
I'm also not sure if endianness is completely covered in the patch but
it looks fairly accurate. I think PointerGetDatum() may need the union
trick (it's late where I am).
There were other places in the code which were assuming Datums were
equivalent to pointers. I'll dig them up.
Also, it means we can clean up parts of numeric.c which special case
calls from aggregates.
Seems like a pretty clean patch though.
Thanks,
Gavin
From | Date | Subject | |
---|---|---|---|
Next Message | Tom Lane | 2008-04-13 05:01:19 | Re: [PATCH] sh: Add support Renesas SuperH |
Previous Message | Gregory Stark | 2008-04-13 00:42:02 | Re: datum passed to macro which expects a pointer |