Re: datum passed to macro which expects a pointer

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

In response to

Browse pgsql-patches by date

  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