Re: DatumGetInetP buggy

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

In response to

Responses

Browse pgsql-hackers by date

  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