From: | Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> |
---|---|
To: | Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> |
Cc: | Boszormenyi Zoltan <zb(at)cybertec(dot)at>, pgsql-hackers(at)postgresql(dot)org, Hans-Juergen Schoenig <hs(at)cybertec(dot)at>, Leonardo Bispo de Oliveira <leonardo(dot)oliveira(at)voipfuture(dot)com> |
Subject: | Re: DatumGetInetP buggy |
Date: | 2011-11-08 17:53:03 |
Message-ID: | 4EB96C7F.9050102@enterprisedb.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 08.11.2011 17:06, Tom Lane wrote:
> Heikki Linnakangas<heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
>> Hmm, it seems to be intentional, but I agree it's very much contrary to
>> the usual convention that DatumGetXXXP() returns a detoasted and
>> depacked datum. I think we should change it. I propose the attached
>> patch. It changes DatumGetInetP() to do PG_DETOAST_DATUM(), and adds new
>> DatumGetInetPP() macro to return the packed version. I also moved the
>> access macros like ip_family() from network.c to inet.h, so that they're
>> available for whoever wants to look at the fields without having to depack.
>
> No objection to making the DatumGet macro names conform to common
> convention, but I'm not thrilled with moving those special-purpose
> accessor macros into wider circulation. It's not necessary and the
> macros don't work unless used in a particular way per the comment,
> so I don't think they can be considered general purpose.
Ok.
What do people think of backpatching this? I'm inclined to backpatch, on
the grounds that the macro that detoasts and depacks should always work
(ie. after this patch), even if the depacking isn't necessary and
introduces an extra palloc+copy, but the reverse is not true.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
From | Date | Subject | |
---|---|---|---|
Next Message | Greg Smith | 2011-11-08 18:07:02 | Re: Measuring relation free space |
Previous Message | Teodor Sigaev | 2011-11-08 17:49:28 | ERROR: MergeAppend child's targetlist doesn't match MergeAppend |