Re: proposal: minscale, rtrim, btrim functions for numeric - Mailing list pgsql-hackers

From Karl O. Pinc
Subject Re: proposal: minscale, rtrim, btrim functions for numeric
Date
Msg-id 20191209121522.0e97e8f4@slate.karlpinc.com
Whole thread Raw
In response to Re: proposal: minscale, rtrim, btrim functions for numeric  ("Karl O. Pinc" <kop@meme.com>)
Responses Re: proposal: minscale, rtrim, btrim functions for numeric  ("Karl O. Pinc" <kop@meme.com>)
Re: proposal: minscale, rtrim, btrim functions for numeric  (Pavel Stehule <pavel.stehule@gmail.com>)
List 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@meme.com>
Free Software:  "You don't pay back, you pay forward."
                 -- Robert A. Heinlein



pgsql-hackers by date:

Previous
From: Julien Delplanque
Date:
Subject: Re: Questions about PostgreSQL implementation details
Next
From: Andrew Dunstan
Date:
Subject: Re: Windows buildfarm members vs. new async-notify isolation test