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 | Resend email |
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 |
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 |