Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test case fails with mysql 8.0.22 #160

Open
xnox opened this issue Nov 11, 2020 · 11 comments
Open

test case fails with mysql 8.0.22 #160

xnox opened this issue Nov 11, 2020 · 11 comments

Comments

@xnox
Copy link

xnox commented Nov 11, 2020

https://github.com/gooddata/DBD-MariaDB/blob/9e9b5f9abf50080a17fd406425ae2f7bf35c4461/t/rt122541-decimals.t#L17

with 8.0.22, which had changes to ROUND() the above returns 1.7 and test case fails.

I'm not sure if the mysql changes are intentional and good, or if the test case assertion now needs to become micro-point release aware and accept either 1.70 or 1.7 as valid results.

Maybe sql query or the perl syntax to compare equivalence, rather than exact string. I don't speak perl.

Grep for ROUND() in https://dev.mysql.com/doc/relnotes/mysql/8.0/en/news-8-0-22.html

@xnox
Copy link
Author

xnox commented Nov 11, 2020

I don't speak perl

--- libdbd-mariadb-perl-1.21.orig/t/rt122541-decimals.t
+++ libdbd-mariadb-perl-1.21/t/rt122541-decimals.t
@@ -14,6 +14,6 @@ plan tests => 2;
 
 for my $mariadb_server_prepare (0, 1) {
 	$dbh->{mariadb_server_prepare} = $mariadb_server_prepare;
-	is $dbh->selectrow_arrayref('SELECT round(degrees(0.00043) * 69, 2)')->[0], '1.70',
+	like $dbh->selectrow_arrayref('SELECT round(degrees(0.00043) * 69, 2)')->[0], qr/1.70?/,
 		'floats with fixed-length of decimals returns correct value for mariadb_server_prepare=' . $mariadb_server_prepare;
 }

@choroba
Copy link
Member

choroba commented Nov 11, 2020

I'd use qr/^1\.70?$/ but otherwise seems to capture the change.

@pali
Copy link
Member

pali commented Apr 7, 2022

Well, that test t/rt122541-decimals.t checks that bugs https://rt.cpan.org/Ticket/Display.html?id=122541 perl5-dbi/DBD-mysql#170 were fixed and ZEROFILL_FLAG (appending trailing zeros) works in DBD::MariaDB correctly. Therefore removal of trailing zeros from expected result is wrong as this test does not do anything.

If MySQL server changed behavior of some functions then it is required to write some new test for ZEROFILL_FLAG to ensure that this flags is implemented correctly.

@pali
Copy link
Member

pali commented Aug 13, 2023

@xnox Could you please run this command against your MySQL server?

mysql --column-type-info -e 'SELECT round(degrees(0.00043) * 69, 2);'

Argument --column-type-info is important as it prints type information about result. Based on this I could try to prepare some fix.

For example against MariaDB 10.3.39 it returns:

Field   1:  `round(degrees(0.00043) * 69, 2)`
Catalog:    `def`
Database:   ``
Table:      ``
Org_table:  ``
Type:       DOUBLE
Collation:  binary (63)
Length:     19
Max_length: 4
Decimals:   2
Flags:      NOT_NULL BINARY NUM


+---------------------------------+
| round(degrees(0.00043) * 69, 2) |
+---------------------------------+
|                            1.70 |
+---------------------------------+

@pali
Copy link
Member

pali commented Aug 16, 2023

@maxbarry: Maybe you could help here too to provide output from above command?

@maxbarry
Copy link

~$ sudo mysql --column-type-info -e 'SELECT round(degrees(0.00043) * 69, 2);'
Field   1:  `round(degrees(0.00043) * 69, 2)`
Catalog:    `def`
Database:   ``
Table:      ``
Org_table:  ``
Type:       DOUBLE
Collation:  binary (63)
Length:     23
Max_length: 3
Decimals:   31
Flags:      NOT_NULL BINARY NUM 


+---------------------------------+
| round(degrees(0.00043) * 69, 2) |
+---------------------------------+
|                             1.7 |
+---------------------------------+
~$ mysql --version
mysql  Ver 8.0.34-0ubuntu0.23.04.1 for Linux on x86_64 ((Ubuntu))

@pali
Copy link
Member

pali commented Aug 16, 2023

Thank you very much for the output.

Now I see that "Decimals" field is reported incorrectly. Values below 31 means number of fixed decimal digits. Value 31 and above are special, 31 means NOT_FIXED_DEC (number of decimal digits is not fixed).

So this looks like a bug that SQL function ROUND ignores number of digits or does not return correct information about decimal digits anymore, since that MySQL 8.0.22 version. I guess that this issue was introduced during fixing other issues mentioned in the changelog.

I'm really not sure what I can do with it...

@pali
Copy link
Member

pali commented Aug 18, 2023

@choroba any idea what we can do here? Skip the test at all as server does not return correct information needed for testing this particular feature?

@choroba
Copy link
Member

choroba commented Aug 18, 2023

Has the problem been reported upstream? Skipping the test doesn't seem right to me, as if someone depends on the correct behaviour, they probably don't want to get "all tests pass"...

@pali
Copy link
Member

pali commented Aug 18, 2023

I do not know if anybody reported it (have not found anything in bugs.mysql.com tracker). So I sent an email for clarification about this issue, you can watch your mailbox...

pali added a commit to pali/DBD-MariaDB that referenced this issue Aug 21, 2023
Unfortunately it looks like that MySQL 8.0.22+ server versions have broken
SQL ROUND() function, it returns incorrect number of rounded decimals in
column metadata information, which DBD::MariaDB uses internally for
converting returned value via MySQL network protocol to Perl scalar.

Skip this tests for affected MySQL server versions until we figure out what
to do with this test case. This at least allows to install DBD::MariaDB
with enabled tests during install phase.

See: perl5-dbi#160
@pali
Copy link
Member

pali commented Aug 21, 2023

I'm not sure when I will receive response regarding this issue, so I created pull request #192 for fixing MySQL 8.0 support and as temporary solution I chosen: Disable this problematic test on affected MySQL versions and add a new test which does not use ROUND() function but CAST() to DECIMAL type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants