From: | Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com> |
---|---|
To: | Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr> |
Cc: | PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Greatest Common Divisor |
Date: | 2019-12-29 13:58:49 |
Message-ID: | 9890a02a-94d4-537c-9f4a-ff02a5414720@2ndquadrant.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On 29/12/2019 08:30, Fabien COELHO wrote:
>
>>> I'm wondering what it should do on N, 0 and 0, 0. Raise an error?
>>> Return 0? Return 1? return N? There should be some logic and comments
>>> explaining it.
>>
>> Well, gcd(N, 0) is N, and gcd(0, 0) is 0, so I don't see an issue here?
>
> I think that there should be a comment.
Done.
>>> It does not seem that fixing the sign afterwards is the right thing to
>>> do. I'd rather turn both arguments positive before doing the descent.
>>
>> Why isn't it the right thing to do?
>
> Because I do not trust C modulo as I had a lot of problems with it? :-)
>
> If it works, but it should deserve a clear comment explaining why.
Surely such a comment should be on the mod functions and not in this patch.
>
>>> Which raises an issue with INT_MIN by the way, which has no positive:-(
>>
>> That's an argument against abs-ing the input values. It's only an issue
>> with gcd(INT_MIN, INT_MIN) which currently returns INT_MIN.
>
> That should be an error instead, because -INT_MIN cannot be represented?
Why should it error? Is INT_MIN not a valid divisor of INT_MIN? I
added a comment instead.
>>> Also, the usual approach is to exchange args so that the largest is
>>> first, because there may be a software emulation if the hardware does
>>> not implement modulo. At least it was the case with some sparc
>>> processors 25 years ago:-)
>>
>> The args will exchange themselves.
>
> Yep, but after a possibly expensive software-emulated modulo operation?
>
I'll just trust you on this. Swap added.
Thanks!
--
Vik
Attachment | Content-Type | Size |
---|---|---|
gcd.0003.patch | text/x-patch | 7.7 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2019-12-29 16:16:47 | Re: Recognizing superuser in pg_hba.conf |
Previous Message | Dave Cramer | 2019-12-29 13:35:32 | Re: let's make the list of reportable GUCs configurable (was Re: Add %r substitution for psql prompts to show recovery status) |