Re: [HACKERS] taking stdbool.h into use

From: Peter Eisentraut <peter(dot)eisentraut(at)2ndquadrant(dot)com>
To: Michael Paquier <michael(dot)paquier(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>, Thomas Munro <thomas(dot)munro(at)enterprisedb(dot)com>, Kevin Grittner <kgrittn(at)gmail(dot)com>
Subject: Re: [HACKERS] taking stdbool.h into use
Date: 2018-01-23 16:33:56
Message-ID: 1d3c6c73-3748-83be-22e7-8ec8e24916a3@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox
Thread:
Lists: pgsql-hackers

On 1/16/18 01:14, Michael Paquier wrote:
>> I don't see it either. I think the actually useful parts of that patch
>> were to declare record_image_cmp's result correctly as int rather than
>> bool, and to cope with varlena fields of unequal size. Unfortunately
>> there seems to be no contemporaneous mailing list discussion, so
>> it's not clear what Kevin based this change on.
>
> This was a hotfix after a buildfarm breakage, the base commit being
> f566515.
>
>> Want to try reverting the GET_X_BYTES() parts of it and see if the
>> buildfarm complains?
>
> So, Peter, are you planning to do so?

Here is a proposed patch set to clean this up. First, add some test
coverage for record_image_cmp. (There wasn't any, only for
record_image_eq as part of MV testing.) Then, remove the GET_ macros
from record_image_{eq,cmp}. And finally remove all that byte-masking
stuff from postgres.h.

>> Note if that if we simplify the GetDatum macros, it's possible that
>> record_image_cmp would change behavior, since it might now see signed not
>> unsigned values depending on whether the casts do sign extension or not.
>> Not sure if that'd be a problem.

I wasn't able to construct such a case. Everything still works unsigned
end-to-end, I think. But if you can think of a case, we can throw it
into the tests. The tests already contain cases of comparing positive
and negative integers.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachment Content-Type Size
0001-Add-tests-for-record_image_eq-and-record_image_cmp.patch text/plain 6.9 KB
0002-Remove-use-of-byte-masking-macros-in-record_image_cm.patch text/plain 2.6 KB
0003-Remove-byte-masking-macros-for-Datum-conversion-macr.patch text/plain 7.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Abinaya k 2018-01-23 16:37:49 Fwd: Regarding ambulkdelete, amvacuumcleanup index methods
Previous Message Stephen Frost 2018-01-23 16:31:26 Re: [HACKERS] PoC: full merge join on comparison clause