Email to hackers for test coverage

Started by movead.li@highgo.caover 6 years ago15 messages

over 6 years ago

3 attachment(s)

Hello hackers,

One of the area that didn't get much attention in the community
recently is analysing and increasing the code coverage of
PostgreSQL regession test suite. I have started working on the
code coverage by running the GCOV code coverage analysis tool in
order to analyse the current code coverage and come up with test
cases to increase the code coverage. This is going to be a long
exercise so my plan is do it incrementaly. I will be analysing
some area of untested code and then coming up with test cases to
test those lines of code in regression and then moving on next
area of untested code and so on.

So far I have come up with 3 test cases to increase the code
coverage of PostgreSQL regression test suite.

I have performed the regression run for this exercise on this commit:
(Commit version 75c1921cd6c868c5995b88113b4463a4830b9a27):

The regression is executed with make check-world command and the
results are gathered using 'make coverage-html' command.

Below are the lines of untested code that i have analysed and the
test cases added to regression to test these as part of regression.

1. src/include/utils/float.h:140

Analyze:
This is an error report line when converting a big float8 value
which a float4 can not storage to float4.

Test case:
Add a test case as below in file float4.sql:
select float4(1234567890123456789012345678901234567890::float8);

2. src/include/utils/float.h:145

Analyze:
This is an error report line when converting a small float8 value
which a float4 can not storage to float4.

Test case:
Add a test case as below in file float4.sql:
select float4(0.0000000000000000000000000000000000000000000001::float8);

3.src/include/utils/sortsupport.h:264

Analyze:
It is reverse sorting for the data type that has abbreviated for
sort, for example macaddr, uuid, numeric, network and I choose
numeric to do it.

Test cast:
Add a test case as below in file numeric.sql:
INSERT INTO num_input_test(n1) values('99999999999999999999999999.998');
INSERT INTO num_input_test(n1) values('99999999999999999999999999.997');
SELECT * FROM num_input_test ORDER BY n1 DESC;

Result and patch

By adding the test cases, the test coverage of float.h increased from
97.7% to 100% and sortsupport.h increased from 76.7% to 80.0%.

The increase in code coverage can be seen in the before and after
pictures of GCOV test coverage analysis summary.

The attached patch contain the test cases added in regression for
increasing the coverage.

--
Movead Li

Attachments:

before.pngapplication/octet-stream; name=before.pngDownload

after.pngapplication/octet-stream; name=after.pngDownload

regression_20190820.patchapplication/octet-stream; name=regression_20190820.patchDownload

diff --git a/src/test/regress/expected/float4.out b/src/test/regress/expected/float4.out
index 901abb1d27..0f0dbe0689 100644
--- a/src/test/regress/expected/float4.out
+++ b/src/test/regress/expected/float4.out
@@ -939,3 +939,8 @@ drop cascades to cast from xfloat4 to real
 drop cascades to cast from real to xfloat4
 drop cascades to cast from xfloat4 to integer
 drop cascades to cast from integer to xfloat4
+-- Add test case for float4() type fonversion function
+select float4(1234567890123456789012345678901234567890::float8);
+ERROR:  value out of range: overflow
+select float4(0.0000000000000000000000000000000000000000000001::float8);
+ERROR:  value out of range: underflow
diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out
index 1cb3c3bfab..289c930265 100644
--- a/src/test/regress/expected/numeric.out
+++ b/src/test/regress/expected/numeric.out
@@ -1426,6 +1426,8 @@ INSERT INTO num_input_test(n1) VALUES ('555.50');
 INSERT INTO num_input_test(n1) VALUES ('-555.50');
 INSERT INTO num_input_test(n1) VALUES ('NaN ');
 INSERT INTO num_input_test(n1) VALUES ('        nan');
+INSERT INTO num_input_test(n1) values('99999999999999999999999999.998');
+INSERT INTO num_input_test(n1) values('99999999999999999999999999.997');
 -- bad inputs
 INSERT INTO num_input_test(n1) VALUES ('     ');
 ERROR:  invalid input syntax for type numeric: "     "
@@ -1459,17 +1461,19 @@ INSERT INTO num_input_test(n1) VALUES (' N aN ');
 ERROR:  invalid input syntax for type numeric: " N aN "
 LINE 1: INSERT INTO num_input_test(n1) VALUES (' N aN ');
                                                ^
-SELECT * FROM num_input_test;
-   n1    
----------
-     123
- 3245874
-  -93853
-  555.50
- -555.50
-     NaN
-     NaN
-(7 rows)
+SELECT * FROM num_input_test ORDER BY n1 DESC;
+               n1               
+--------------------------------
+                            NaN
+                            NaN
+ 99999999999999999999999999.998
+ 99999999999999999999999999.997
+                        3245874
+                         555.50
+                            123
+                        -555.50
+                         -93853
+(9 rows)
 
 --
 -- Test some corner cases for multiplication
diff --git a/src/test/regress/sql/float4.sql b/src/test/regress/sql/float4.sql
index afdb469dc8..fecc0a7d17 100644
--- a/src/test/regress/sql/float4.sql
+++ b/src/test/regress/sql/float4.sql
@@ -345,3 +345,7 @@ select float4send(flt) as ibits,
 
 -- clean up, lest opr_sanity complain
 drop type xfloat4 cascade;
+
+-- Add test case for float4() type fonversion function
+select float4(1234567890123456789012345678901234567890::float8);
+select float4(0.0000000000000000000000000000000000000000000001::float8);
\ No newline at end of file
diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql
index a939412359..51895c426b 100644
--- a/src/test/regress/sql/numeric.sql
+++ b/src/test/regress/sql/numeric.sql
@@ -839,6 +839,8 @@ INSERT INTO num_input_test(n1) VALUES ('555.50');
 INSERT INTO num_input_test(n1) VALUES ('-555.50');
 INSERT INTO num_input_test(n1) VALUES ('NaN ');
 INSERT INTO num_input_test(n1) VALUES ('        nan');
+INSERT INTO num_input_test(n1) values('99999999999999999999999999.998');
+INSERT INTO num_input_test(n1) values('99999999999999999999999999.997');
 
 -- bad inputs
 INSERT INTO num_input_test(n1) VALUES ('     ');
@@ -850,7 +852,7 @@ INSERT INTO num_input_test(n1) VALUES ('5. 0   ');
 INSERT INTO num_input_test(n1) VALUES ('');
 INSERT INTO num_input_test(n1) VALUES (' N aN ');
 
-SELECT * FROM num_input_test;
+SELECT * FROM num_input_test ORDER BY n1 DESC;
 
 --
 -- Test some corner cases for multiplication

Ahsan Hadi

ahsan.hadi@gmail.com

over 6 years ago

In reply to: movead.li@highgo.ca (#1)

Re: Email to hackers for test coverage

The subject of the email may be bit misleading however this is really good
exercise to analyse the current code of Postgres regression suite using
GCOV and then adding test cases incrementally to increase the test
coverage.

On Thu, 22 Aug 2019 at 2:46 PM, movead.li@highgo.ca <movead.li@highgo.ca>
wrote:

Show quoted text

Hello hackers,

One of the area that didn't get much attention in the community
recently is analysing and increasing the code coverage of
PostgreSQL regession test suite. I have started working on the
code coverage by running the GCOV code coverage analysis tool in
order to analyse the current code coverage and come up with test
cases to increase the code coverage. This is going to be a long
exercise so my plan is do it incrementaly. I will be analysing
some area of untested code and then coming up with test cases to
test those lines of code in regression and then moving on next
area of untested code and so on.

So far I have come up with 3 test cases to increase the code
coverage of PostgreSQL regression test suite.

I have performed the regression run for this exercise on this commit:
(Commit version 75c1921cd6c868c5995b88113b4463a4830b9a27):

The regression is executed with make check-world command and the
results are gathered using 'make coverage-html' command.

Below are the lines of untested code that i have analysed and the
test cases added to regression to test these as part of regression.

*1. src/include/utils/float.h:140*

Analyze:
This is an error report line when converting a big float8 value
which a float4 can not storage to float4.

Test case:
Add a test case as below in file float4.sql:
select float4(1234567890123456789012345678901234567890::float8);

*2. src/include/utils/float.h:145*

Analyze:
This is an error report line when converting a small float8 value
which a float4 can not storage to float4.

Test case:
Add a test case as below in file float4.sql:
select float4(0.0000000000000000000000000000000000000000000001::float8);

*3.src/include/utils/sortsupport.h:264*

Analyze:
It is reverse sorting for the data type that has abbreviated for
sort, for example macaddr, uuid, numeric, network and I choose
numeric to do it.

Test cast:
Add a test case as below in file numeric.sql:
INSERT INTO num_input_test(n1) values('99999999999999999999999999.998');
INSERT INTO num_input_test(n1) values('99999999999999999999999999.997');
SELECT * FROM num_input_test ORDER BY n1 DESC;

Result and patch

By adding the test cases, the test coverage of float.h increased from
97.7% to 100% and sortsupport.h increased from 76.7% to 80.0%.

The increase in code coverage can be seen in the before and after
pictures of GCOV test coverage analysis summary.

The attached patch contain the test cases added in regression for
increasing the coverage.

--
Movead Li

Ibrar Ahmed

ibrar.ahmad@gmail.com

over 6 years ago

In reply to: movead.li@highgo.ca (#1)

Re: Email to hackers for test coverage

On Thu, Aug 22, 2019 at 2:46 PM movead.li@highgo.ca <movead.li@highgo.ca>
wrote:

Hello hackers,

One of the area that didn't get much attention in the community
recently is analysing and increasing the code coverage of
PostgreSQL regession test suite. I have started working on the
code coverage by running the GCOV code coverage analysis tool in
order to analyse the current code coverage and come up with test
cases to increase the code coverage. This is going to be a long
exercise so my plan is do it incrementaly. I will be analysing
some area of untested code and then coming up with test cases to
test those lines of code in regression and then moving on next
area of untested code and so on.

So far I have come up with 3 test cases to increase the code
coverage of PostgreSQL regression test suite.

I have performed the regression run for this exercise on this commit:
(Commit version 75c1921cd6c868c5995b88113b4463a4830b9a27):

The regression is executed with make check-world command and the
results are gathered using 'make coverage-html' command.

Below are the lines of untested code that i have analysed and the
test cases added to regression to test these as part of regression.

*1. src/include/utils/float.h:140*

Analyze:
This is an error report line when converting a big float8 value
which a float4 can not storage to float4.

Test case:
Add a test case as below in file float4.sql:
select float4(1234567890123456789012345678901234567890::float8);

*2. src/include/utils/float.h:145*

Analyze:
This is an error report line when converting a small float8 value
which a float4 can not storage to float4.

Test case:
Add a test case as below in file float4.sql:
select float4(0.0000000000000000000000000000000000000000000001::float8);

*3.src/include/utils/sortsupport.h:264*

Analyze:
It is reverse sorting for the data type that has abbreviated for
sort, for example macaddr, uuid, numeric, network and I choose
numeric to do it.

Test cast:
Add a test case as below in file numeric.sql:
INSERT INTO num_input_test(n1) values('99999999999999999999999999.998');
INSERT INTO num_input_test(n1) values('99999999999999999999999999.997');
SELECT * FROM num_input_test ORDER BY n1 DESC;

Result and patch

By adding the test cases, the test coverage of float.h increased from
97.7% to 100% and sortsupport.h increased from 76.7% to 80.0%.

The increase in code coverage can be seen in the before and after
pictures of GCOV test coverage analysis summary.

The attached patch contain the test cases added in regression for
increasing the coverage.

--
Movead Li

Hi Movead,
Please add that to commitfest.

--
Ibrar Ahmed

movead.li@highgo.ca

over 6 years ago

In reply to: movead.li@highgo.ca (#1)

Re: Re: Email to hackers for test coverage

Hi Movead,
Please add that to commitfest.

Thanks and I just do it, it is
https://commitfest.postgresql.org/24/2258/

--
Movead Li

Michael Paquier

michael@paquier.xyz

over 6 years ago

In reply to: movead.li@highgo.ca (#4)

Re: Re: Email to hackers for test coverage

On Sat, Aug 24, 2019 at 11:23:32PM +0800, movead.li@highgo.ca wrote:

Thanks and I just do it, it is
https://commitfest.postgresql.org/24/2258/

Your patch has forgotten to update the alternate output in
float4-misrounded-input.out.
--
Michael

movead.li@highgo.ca

over 6 years ago

In reply to: movead.li@highgo.ca (#1)

1 attachment(s)

Re: Re: Email to hackers for test coverage

On Mon, 26 Aug 2019 12:48:40 +0800 michael@paquier.xyz wrote

Your patch has forgotten to update the alternate output in
float4-misrounded-input.out.

Thanks for your remind, I have modified the patch and now it is
'regression_20190826.patch' in attachment, and I have done a successful
test on Cygwin.

--
Movead

Attachments:

regression_20190826.patchapplication/octet-stream; name=regression_20190826.patchDownload

diff --git a/src/test/regress/expected/float4-misrounded-input.out b/src/test/regress/expected/float4-misrounded-input.out
index d21e1fba1f..150692a812 100644
--- a/src/test/regress/expected/float4-misrounded-input.out
+++ b/src/test/regress/expected/float4-misrounded-input.out
@@ -939,3 +939,8 @@ drop cascades to cast from xfloat4 to real
 drop cascades to cast from real to xfloat4
 drop cascades to cast from xfloat4 to integer
 drop cascades to cast from integer to xfloat4
+-- Add test case for float4() type fonversion function
+select float4(1234567890123456789012345678901234567890::float8);
+ERROR:  value out of range: overflow
+select float4(0.0000000000000000000000000000000000000000000001::float8);
+ERROR:  value out of range: underflow
diff --git a/src/test/regress/expected/float4.out b/src/test/regress/expected/float4.out
index 901abb1d27..0f0dbe0689 100644
--- a/src/test/regress/expected/float4.out
+++ b/src/test/regress/expected/float4.out
@@ -939,3 +939,8 @@ drop cascades to cast from xfloat4 to real
 drop cascades to cast from real to xfloat4
 drop cascades to cast from xfloat4 to integer
 drop cascades to cast from integer to xfloat4
+-- Add test case for float4() type fonversion function
+select float4(1234567890123456789012345678901234567890::float8);
+ERROR:  value out of range: overflow
+select float4(0.0000000000000000000000000000000000000000000001::float8);
+ERROR:  value out of range: underflow
diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out
index 1cb3c3bfab..289c930265 100644
--- a/src/test/regress/expected/numeric.out
+++ b/src/test/regress/expected/numeric.out
@@ -1426,6 +1426,8 @@ INSERT INTO num_input_test(n1) VALUES ('555.50');
 INSERT INTO num_input_test(n1) VALUES ('-555.50');
 INSERT INTO num_input_test(n1) VALUES ('NaN ');
 INSERT INTO num_input_test(n1) VALUES ('        nan');
+INSERT INTO num_input_test(n1) values('99999999999999999999999999.998');
+INSERT INTO num_input_test(n1) values('99999999999999999999999999.997');
 -- bad inputs
 INSERT INTO num_input_test(n1) VALUES ('     ');
 ERROR:  invalid input syntax for type numeric: "     "
@@ -1459,17 +1461,19 @@ INSERT INTO num_input_test(n1) VALUES (' N aN ');
 ERROR:  invalid input syntax for type numeric: " N aN "
 LINE 1: INSERT INTO num_input_test(n1) VALUES (' N aN ');
                                                ^
-SELECT * FROM num_input_test;
-   n1    
----------
-     123
- 3245874
-  -93853
-  555.50
- -555.50
-     NaN
-     NaN
-(7 rows)
+SELECT * FROM num_input_test ORDER BY n1 DESC;
+               n1               
+--------------------------------
+                            NaN
+                            NaN
+ 99999999999999999999999999.998
+ 99999999999999999999999999.997
+                        3245874
+                         555.50
+                            123
+                        -555.50
+                         -93853
+(9 rows)
 
 --
 -- Test some corner cases for multiplication
diff --git a/src/test/regress/sql/float4.sql b/src/test/regress/sql/float4.sql
index afdb469dc8..fecc0a7d17 100644
--- a/src/test/regress/sql/float4.sql
+++ b/src/test/regress/sql/float4.sql
@@ -345,3 +345,7 @@ select float4send(flt) as ibits,
 
 -- clean up, lest opr_sanity complain
 drop type xfloat4 cascade;
+
+-- Add test case for float4() type fonversion function
+select float4(1234567890123456789012345678901234567890::float8);
+select float4(0.0000000000000000000000000000000000000000000001::float8);
\ No newline at end of file
diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql
index a939412359..51895c426b 100644
--- a/src/test/regress/sql/numeric.sql
+++ b/src/test/regress/sql/numeric.sql
@@ -839,6 +839,8 @@ INSERT INTO num_input_test(n1) VALUES ('555.50');
 INSERT INTO num_input_test(n1) VALUES ('-555.50');
 INSERT INTO num_input_test(n1) VALUES ('NaN ');
 INSERT INTO num_input_test(n1) VALUES ('        nan');
+INSERT INTO num_input_test(n1) values('99999999999999999999999999.998');
+INSERT INTO num_input_test(n1) values('99999999999999999999999999.997');
 
 -- bad inputs
 INSERT INTO num_input_test(n1) VALUES ('     ');
@@ -850,7 +852,7 @@ INSERT INTO num_input_test(n1) VALUES ('5. 0   ');
 INSERT INTO num_input_test(n1) VALUES ('');
 INSERT INTO num_input_test(n1) VALUES (' N aN ');
 
-SELECT * FROM num_input_test;
+SELECT * FROM num_input_test ORDER BY n1 DESC;
 
 --
 -- Test some corner cases for multiplication

Michael Paquier

michael@paquier.xyz

over 6 years ago

In reply to: movead.li@highgo.ca (#6)

Re: Re: Email to hackers for test coverage

On Mon, Aug 26, 2019 at 05:10:59PM +0800, movead.li@highgo.ca wrote:

Thanks for your remind, I have modified the patch and now it is
'regression_20190826.patch' in attachment, and I have done a successful
test on Cygwin.

There is a section in float4.sql which deals with overflow and
underflow, so wouldn't it be better to move the tests there? You
could just trigger the failures with that:
=# insert into float4_tbl values ('-10e-70'::float8);
ERROR: 22003: value out of range: underflow
LOCATION: check_float4_val, float.h:145
=# insert into float4_tbl values ('-10e70'::float8);
ERROR: 22003: value out of range: overflow
LOCATION: check_float4_val, float.h:140

I would also test all four patterns: 10e70, 10e-70, -10e70, -10e-70.

For the numeric part, this improves the case of
ApplySortAbbrevFullComparator() where both values are not NULL. Could
things be done so as the other code paths are fully covered? One
INSERT is fine by the way to add the extra coverage.
--
Michael

movead.li@highgo.ca

over 6 years ago

In reply to: movead.li@highgo.ca (#1)

1 attachment(s)

Re: Re: Email to hackers for test coverage

On Tue, 27 Aug 2019 14:07:48 +0800 michael@paquier.xyz wrote:

There is a section in float4.sql which deals with overflow and
underflow, so wouldn't it be better to move the tests there? You
could just trigger the failures with that:
=# insert into float4_tbl values ('-10e-70'::float8);
ERROR: 22003: value out of range: underflow
LOCATION: check_float4_val, float.h:145
=# insert into float4_tbl values ('-10e70'::float8);
ERROR: 22003: value out of range: overflow
LOCATION: check_float4_val, float.h:140
I would also test all four patterns: 10e70, 10e-70, -10e70, -10e-70.

I think your way is much better, so I change the patch and it is
'regression_20190827.patch' now.

For the numeric part, this improves the case of
ApplySortAbbrevFullComparator() where both values are not NULL. Could
things be done so as the other code paths are fully covered? One
INSERT is fine by the way to add the extra coverage.

There are code lines related to NULL values in
ApplySortAbbrevFullComparator(), but I think the code lines for
comparing a NULL and a NOT NULL can be never reached, because it is
handled in ApplySortComparator() which is called before
ApplySortAbbrevFullComparator(). So I think it is no use to INSERT
a NULL value.

--
Movead

Attachments:

regression_20190827.patchapplication/octet-stream; name=regression_20190827.patchDownload

diff --git a/src/test/regress/expected/float4-misrounded-input.out b/src/test/regress/expected/float4-misrounded-input.out
index d21e1fba1f..6c89af6394 100644
--- a/src/test/regress/expected/float4-misrounded-input.out
+++ b/src/test/regress/expected/float4-misrounded-input.out
@@ -24,6 +24,14 @@ INSERT INTO FLOAT4_TBL(f1) VALUES ('-10e-70');
 ERROR:  "-10e-70" is out of range for type real
 LINE 1: INSERT INTO FLOAT4_TBL(f1) VALUES ('-10e-70');
                                            ^
+INSERT INTO FLOAT4_TBL(f1) VALUES ('10e70'::float8);
+ERROR:  value out of range: overflow
+INSERT INTO FLOAT4_TBL(f1) VALUES ('-10e70'::float8);
+ERROR:  value out of range: overflow
+INSERT INTO FLOAT4_TBL(f1) VALUES ('10e-70'::float8);
+ERROR:  value out of range: underflow
+INSERT INTO FLOAT4_TBL(f1) VALUES ('-10e-70'::float8);
+ERROR:  value out of range: underflow
 INSERT INTO FLOAT4_TBL(f1) VALUES ('10e400');
 ERROR:  "10e400" is out of range for type real
 LINE 1: INSERT INTO FLOAT4_TBL(f1) VALUES ('10e400');
diff --git a/src/test/regress/expected/float4.out b/src/test/regress/expected/float4.out
index 901abb1d27..d6c22c1752 100644
--- a/src/test/regress/expected/float4.out
+++ b/src/test/regress/expected/float4.out
@@ -24,6 +24,14 @@ INSERT INTO FLOAT4_TBL(f1) VALUES ('-10e-70');
 ERROR:  "-10e-70" is out of range for type real
 LINE 1: INSERT INTO FLOAT4_TBL(f1) VALUES ('-10e-70');
                                            ^
+INSERT INTO FLOAT4_TBL(f1) VALUES ('10e70'::float8);
+ERROR:  value out of range: overflow
+INSERT INTO FLOAT4_TBL(f1) VALUES ('-10e70'::float8);
+ERROR:  value out of range: overflow
+INSERT INTO FLOAT4_TBL(f1) VALUES ('10e-70'::float8);
+ERROR:  value out of range: underflow
+INSERT INTO FLOAT4_TBL(f1) VALUES ('-10e-70'::float8);
+ERROR:  value out of range: underflow
 INSERT INTO FLOAT4_TBL(f1) VALUES ('10e400');
 ERROR:  "10e400" is out of range for type real
 LINE 1: INSERT INTO FLOAT4_TBL(f1) VALUES ('10e400');
diff --git a/src/test/regress/expected/numeric.out b/src/test/regress/expected/numeric.out
index 1cb3c3bfab..289c930265 100644
--- a/src/test/regress/expected/numeric.out
+++ b/src/test/regress/expected/numeric.out
@@ -1426,6 +1426,8 @@ INSERT INTO num_input_test(n1) VALUES ('555.50');
 INSERT INTO num_input_test(n1) VALUES ('-555.50');
 INSERT INTO num_input_test(n1) VALUES ('NaN ');
 INSERT INTO num_input_test(n1) VALUES ('        nan');
+INSERT INTO num_input_test(n1) values('99999999999999999999999999.998');
+INSERT INTO num_input_test(n1) values('99999999999999999999999999.997');
 -- bad inputs
 INSERT INTO num_input_test(n1) VALUES ('     ');
 ERROR:  invalid input syntax for type numeric: "     "
@@ -1459,17 +1461,19 @@ INSERT INTO num_input_test(n1) VALUES (' N aN ');
 ERROR:  invalid input syntax for type numeric: " N aN "
 LINE 1: INSERT INTO num_input_test(n1) VALUES (' N aN ');
                                                ^
-SELECT * FROM num_input_test;
-   n1    
----------
-     123
- 3245874
-  -93853
-  555.50
- -555.50
-     NaN
-     NaN
-(7 rows)
+SELECT * FROM num_input_test ORDER BY n1 DESC;
+               n1               
+--------------------------------
+                            NaN
+                            NaN
+ 99999999999999999999999999.998
+ 99999999999999999999999999.997
+                        3245874
+                         555.50
+                            123
+                        -555.50
+                         -93853
+(9 rows)
 
 --
 -- Test some corner cases for multiplication
diff --git a/src/test/regress/sql/float4.sql b/src/test/regress/sql/float4.sql
index afdb469dc8..393d98fb14 100644
--- a/src/test/regress/sql/float4.sql
+++ b/src/test/regress/sql/float4.sql
@@ -16,6 +16,11 @@ INSERT INTO FLOAT4_TBL(f1) VALUES ('-10e70');
 INSERT INTO FLOAT4_TBL(f1) VALUES ('10e-70');
 INSERT INTO FLOAT4_TBL(f1) VALUES ('-10e-70');
 
+INSERT INTO FLOAT4_TBL(f1) VALUES ('10e70'::float8);
+INSERT INTO FLOAT4_TBL(f1) VALUES ('-10e70'::float8);
+INSERT INTO FLOAT4_TBL(f1) VALUES ('10e-70'::float8);
+INSERT INTO FLOAT4_TBL(f1) VALUES ('-10e-70'::float8);
+
 INSERT INTO FLOAT4_TBL(f1) VALUES ('10e400');
 INSERT INTO FLOAT4_TBL(f1) VALUES ('-10e400');
 INSERT INTO FLOAT4_TBL(f1) VALUES ('10e-400');
diff --git a/src/test/regress/sql/numeric.sql b/src/test/regress/sql/numeric.sql
index a939412359..51895c426b 100644
--- a/src/test/regress/sql/numeric.sql
+++ b/src/test/regress/sql/numeric.sql
@@ -839,6 +839,8 @@ INSERT INTO num_input_test(n1) VALUES ('555.50');
 INSERT INTO num_input_test(n1) VALUES ('-555.50');
 INSERT INTO num_input_test(n1) VALUES ('NaN ');
 INSERT INTO num_input_test(n1) VALUES ('        nan');
+INSERT INTO num_input_test(n1) values('99999999999999999999999999.998');
+INSERT INTO num_input_test(n1) values('99999999999999999999999999.997');
 
 -- bad inputs
 INSERT INTO num_input_test(n1) VALUES ('     ');
@@ -850,7 +852,7 @@ INSERT INTO num_input_test(n1) VALUES ('5. 0   ');
 INSERT INTO num_input_test(n1) VALUES ('');
 INSERT INTO num_input_test(n1) VALUES (' N aN ');
 
-SELECT * FROM num_input_test;
+SELECT * FROM num_input_test ORDER BY n1 DESC;
 
 --
 -- Test some corner cases for multiplication

Michael Paquier

michael@paquier.xyz

over 6 years ago

In reply to: movead.li@highgo.ca (#8)

Re: Re: Email to hackers for test coverage

On Tue, Aug 27, 2019 at 03:57:20PM +0800, movead.li@highgo.ca wrote:

I think your way is much better, so I change the patch and it is
'regression_20190827.patch' now.

Thanks for the new patch, I have committed the part for float4.

There are code lines related to NULL values in
ApplySortAbbrevFullComparator(), but I think the code lines for
comparing a NULL and a NOT NULL can be never reached, because it is
handled in ApplySortComparator() which is called before
ApplySortAbbrevFullComparator(). So I think it is no use to INSERT
a NULL value.

But I am not sold to that part yet, for three reasons:
- numeric is not a test suite designed for sorting, and hijacking it
to do so it not a good approach.
- it would be good to get coverage for the two extra code paths while
we are on it with NULL datums.
- There is no need for two INSERT queries, I am getting the same
coverage with only one.

Please note that I have not looked in details where we could put that,
but perhaps Robert and Peter G who worked on 4ea51cd to add support
for abbreviated keys have some ideas, so I am adding them in CC for
input.
--
Michael

#10

movead.li@highgo.ca

over 6 years ago

In reply to: movead.li@highgo.ca (#1)

Re: Re: Email to hackers for test coverage

On Wed, 28 Aug 2019 11:30:23 +0800 michael@paquier.xyz wrote

- numeric is not a test suite designed for sorting, and hijacking it
to do so it not a good approach.

Absolutely agreement. We can wait for repling from
Robert and Peter G.

- it would be good to get coverage for the two extra code paths while
we are on it with NULL datums.

The remained untested line in ApplySortComparator() can be never
reached I think, and I have explained it on last mail.
Or I don't fully understand what you mean?

- There is no need for two INSERT queries, I am getting the same
coverage with only one.

Yes It is my mistake, the key point is 'ORDER BY n1 DESC' not the
'INSERT'. Infact it can run the code line without any of the two INSERT.

--
Movead

#11

Peter Eisentraut

peter.eisentraut@2ndquadrant.com

over 6 years ago

In reply to: movead.li@highgo.ca (#1)

Re: Email to hackers for test coverage

On 2019-08-22 11:46, movead.li@highgo.ca wrote:

*1. src/include/utils/float.h:140*

Analyze:ï¿½
This is an error report line when converting a big float8 value
which a float4 can not storage to float4.

Test case:
Add a test case as below in file float4.sql:
select float4(1234567890123456789012345678901234567890::float8);

+-- Add test case for float4() type fonversion function

Check spelling

*2. src/include/utils/float.h:145*

Analyze:
This is an error report line when converting a small float8 value
which a float4 can not storage to float4.

Test case:
Add a test case as below in file float4.sql:
select float4(0.0000000000000000000000000000000000000000000001::float8);

*3.src/include/utils/sortsupport.h:264*

Analyze:
It is reverse sorting for the data type that has abbreviated for
sort, for example macaddr, uuid, numeric, network and I choose
numeric to do it.

Test cast:
Add a test case as below in file numeric.sql:
INSERT INTO num_input_test(n1) values('99999999999999999999999999.998');
INSERT INTO num_input_test(n1) values('99999999999999999999999999.997');
SELECT * FROM num_input_test ORDER BY n1 DESC;

INSERT INTO num_input_test(n1) VALUES ('        nan');
+INSERT INTO num_input_test(n1) values('99999999999999999999999999.998');
+INSERT INTO num_input_test(n1) values('99999999999999999999999999.997');

Make spaces and capitalization match surrounding code.

Result and patch

By adding the test cases, the test coverage of ï¿½float.h increased from
97.7% to 100% and sortsupport.h increased from 76.7% to 80.0%.

That's fine, but I suggest that if you really want to make an impact in
test coverage, look to increase function coverage rather than line
coverage. Or look for files that are not covered at all.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#12

Peter Geoghegan

pg@bowt.ie

over 6 years ago

In reply to: Michael Paquier (#9)

Re: Re: Email to hackers for test coverage

On Tue, Aug 27, 2019 at 8:30 PM Michael Paquier <michael@paquier.xyz> wrote:

Please note that I have not looked in details where we could put that,
but perhaps Robert and Peter G who worked on 4ea51cd to add support
for abbreviated keys have some ideas, so I am adding them in CC for
input.

Movead is correct -- the NULL handling within
ApplySortAbbrevFullComparator() cannot actually be used currently. I
wouldn't change anything about the code, though, since it's useful to
defensively handle NULLs.

--
Peter Geoghegan

#13

movead.li@highgo.ca

over 6 years ago

In reply to: movead.li@highgo.ca (#1)

Re: Re: Email to hackers for test coverage

On 2019-08-29 00:43, peter.eisentraut@2ndquadrant.com wrote:

Make spaces and capitalization match surrounding code.
That's fine, but I suggest that if you really want to make an impact in
test coverage, look to increase function coverage rather than line
coverage. Or look for files that are not covered at all.

Thanks for pointing all the things, I will reconsider my way on
code coverage work.

--
Movead

#14

Ahsan Hadi

ahsan.hadi@gmail.com

over 6 years ago

In reply to: Peter Eisentraut (#11)

Re: Email to hackers for test coverage

On Wed, Aug 28, 2019 at 9:43 PM Peter Eisentraut <
peter.eisentraut@2ndquadrant.com> wrote:

On 2019-08-22 11:46, movead.li@highgo.ca wrote:

*1. src/include/utils/float.h:140*

Analyze:
This is an error report line when converting a big float8 value
which a float4 can not storage to float4.

Test case:
Add a test case as below in file float4.sql:
select float4(1234567890123456789012345678901234567890::float8);

+-- Add test case for float4() type fonversion function

Check spelling

*2. src/include/utils/float.h:145*

Analyze:
This is an error report line when converting a small float8 value
which a float4 can not storage to float4.

Test case:
Add a test case as below in file float4.sql:
select float4(0.0000000000000000000000000000000000000000000001::float8);

*3.src/include/utils/sortsupport.h:264*

Analyze:
It is reverse sorting for the data type that has abbreviated for
sort, for example macaddr, uuid, numeric, network and I choose
numeric to do it.

Test cast:
Add a test case as below in file numeric.sql:
INSERT INTO num_input_test(n1) values('99999999999999999999999999.998');
INSERT INTO num_input_test(n1) values('99999999999999999999999999.997');
SELECT * FROM num_input_test ORDER BY n1 DESC;
INSERT INTO num_input_test(n1) VALUES ('        nan');
+INSERT INTO num_input_test(n1) values('99999999999999999999999999.998');
+INSERT INTO num_input_test(n1) values('99999999999999999999999999.997');
Make spaces and capitalization match surrounding code.

Result and patch

By adding the test cases, the test coverage of float.h increased from
97.7% to 100% and sortsupport.h increased from 76.7% to 80.0%.

That's fine, but I suggest that if you really want to make an impact in
test coverage, look to increase function coverage rather than line
coverage. Or look for files that are not covered at all.

Show quoted text

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#15

Michael Paquier

michael@paquier.xyz

over 6 years ago

In reply to: Peter Geoghegan (#12)

Re: Re: Email to hackers for test coverage

On Wed, Aug 28, 2019 at 11:27:15AM -0700, Peter Geoghegan wrote:

Movead is correct -- the NULL handling within
ApplySortAbbrevFullComparator() cannot actually be used currently. I
wouldn't change anything about the code, though, since it's useful to
defensively handle NULLs.

No objections with this line of thoughts. Thanks, Peter. Please note
that I have marked the original patch as committed in the CF app. If
there are tests to improve the coverage, let's do that on a new
thread. I am still not sure where I would put tests dedicated to
abbreviated keys, but let's sort out that if necessary later.
--
Michael