Re: proposal: minscale, rtrim, btrim functions for numeric

From: "Karl O(dot) Pinc" <kop(at)meme(dot)com>
To: Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>, Marko Tiikkaja <marko(at)joh(dot)to>, Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)2ndquadrant(dot)com>, Robert Haas <robertmhaas(at)gmail(dot)com>
Subject: Re: proposal: minscale, rtrim, btrim functions for numeric
Date: 2019-12-09 18:15:22
Message-ID: 20191209121522.0e97e8f4@slate.karlpinc.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Pavel,

I've had some thoughts about the regression tests.

It wouldn't hurt to move them to right after the
scale() tests in numeric.sql.

I believe your tests are covering all the code paths
but it is not clear just what test does what.
I don't see a lot of comments in the tests so I don't
know that it'd be appropriate to put them in to
describe just what's tested. But in any case it
could be nice to choose values where it is at least
sort of apparent what part of the codebase is tested.

FWIW, although the code paths are covered, the possible
data permutations are not. E.g. I don't see a case
where scale > 0 and the NDIGITS of the last digit is full.

There are also some tests (the 0 and 0.00 tests) that duplicates
the execution path. In the 0 case I don't see a problem
but as a rule there's not a lot of point. Better test
values would (mostly) eliminate these.

So, my thoughts run along these lines:

select minscale(numeric 'NaN') is NULL; -- should be true
select minscale(NULL::numeric) is NULL; -- should be true
select minscale(0); -- no digits
select minscale(0.00); -- no digits again
select minscale(1.0); -- no scale
select minscale(1.1); -- scale 1
select minscale(1.12); -- scale 2
select minscale(1.123); -- scale 3
select minscale(1.1234); -- scale 4, filled digit
select minscale(1.12345); -- scale 5, 2 NDIGITS
select minscale(1.1000); -- 1 pos in NDIGITS
select minscale(1.1200); -- 2 pos in NDIGITS
select minscale(1.1230); -- 3 pos in NDIGITS
select minscale(1.1234); -- all pos in NDIGITS
select minscale(1.12345000); -- 2 NDIGITS
select minscale(1.123400000000); -- strip() required/done
select minscale(12345.123456789012345); -- "big" number
select minscale(-12345.12345); -- negative number
select minscale(1e100); -- very big number
select minscale(1e100::numeric + 0.1); -- big number with scale

I don't know why you chose some of your values so if there's
something you were testing for that the above does not cover
please include it.

So, a combination of white and black box testing. Having written
it out it seems like a lot of testing for such a simple function.
On the other hand I don't see a lot of cost in having all
these tests. Opinions welcome.

Regards,

Karl <kop(at)meme(dot)com>
Free Software: "You don't pay back, you pay forward."
-- Robert A. Heinlein

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andrew Dunstan 2019-12-09 18:24:53 Re: Windows buildfarm members vs. new async-notify isolation test
Previous Message Julien Delplanque 2019-12-09 18:09:54 Re: Questions about PostgreSQL implementation details