Re: Greatest Common Divisor

From: Vik Fearing <vik(dot)fearing(at)2ndquadrant(dot)com>
To: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>, Fabien COELHO <coelho(at)cri(dot)ensmp(dot)fr>, Chapman Flack <chap(at)anastigmatix(dot)net>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Greatest Common Divisor
Date: 2020-01-03 18:34:55
Message-ID: 6855727c-3836-21cc-18f9-e79227b99079@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 02/01/2020 16:12, Tom Lane wrote:
> Stephen Frost <sfrost(at)snowman(dot)net> writes:
>> * Dean Rasheed (dean(dot)a(dot)rasheed(at)gmail(dot)com) wrote:
>>> I'm not objecting to adding it, I'm just curious. In fact, I think
>>> that if we do add this, then we should probably add lcm() at the same
>>> time, since handling its overflow cases is sufficiently non-trivial to
>>> justify not requiring users to have to implement it themselves.
>> I tend to agree with this.
> Does this impact the decision about whether we need a variant for
> numeric? I was leaning against that, primarily because (a)
> it'd introduce a set of questions about what to do with non-integral
> inputs, and (b) it'd make the patch quite a lot larger, I imagine.
> But a variant of lcm() that returns numeric would have much more
> resistance to overflow.
>
> Maybe we could just define "lcm(bigint, bigint) returns numeric"
> and figure that that covers all cases, but it feels slightly
> weird. You couldn't do lcm(lcm(a,b),c) without casting.
> I guess that particular use-case could be addressed with
> "lcm(variadic bigint[]) returns numeric", but that's getting
> really odd.

Okay.  Here is a version that should handle everyone's comments.

gcd() is now strictly positive, so INT_MIN is no longer a valid result.

I added an lcm() function. It returns the same type as its arguments so
overflow is possible.  I made this choice because int2mul returns int2
(and same for its friends).  One can just cast to a bigger integer if
needed.

Because of that, I added a version of gcd() and lcm() for numeric.  This
was my first time working with numeric so reviewers should pay extra
attention there, please.

Patch attached.

--

Vik Fearing

Attachment Content-Type Size
gcd.0004.patch text/x-patch 23.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2020-01-03 18:36:01 Re: [PATCH] Increase the maximum value track_activity_query_size
Previous Message Mahendra Singh Thalor 2020-01-03 18:25:49 Re: \d is not showing global(normal) table info if we create temporary table with same name as global table