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

query result for float type is incompatible with previous version #22791

Closed
cfzjywxk opened this issue Feb 18, 2021 · 10 comments · May be fixed by #22823
Closed

query result for float type is incompatible with previous version #22791

cfzjywxk opened this issue Feb 18, 2021 · 10 comments · May be fixed by #22823
Assignees
Labels
severity/major sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug. type/compatibility wontfix This issue will not be fixed.
Milestone

Comments

@cfzjywxk
Copy link
Contributor

Bug Report

Please answer these questions before submitting your issue. Thanks!

1. Minimal reproduce step (Required)

create table tf(f1 float(10,2));
insert into tf values("-34028234.66");
select * from tf;

2. What did you expect to see? (Required)

mysql> select * from tf;
+--------------+
| f1           |
+--------------+
| -34028236.00 |
+--------------+
1 row in set (0.00 sec)

This result is the same with tidb version v4.0 and mysql.

3. What did you see instead (Required)

mysql> select * from tf;
+-----------+
| f1        |
+-----------+
| -34028236 |
+-----------+
1 row in set (0.01 sec)

Seems the float type logic is changed in the master branch and incompatibility is introduced.

4. What is your TiDB version? (Required)

master version with commit hash 9cb9b69b61a76e9cc3f8cfa27f86635c2496cf16

@cfzjywxk cfzjywxk added type/bug The issue is confirmed as a bug. sig/execution SIG execution labels Feb 18, 2021
@b41sh
Copy link
Member

b41sh commented Feb 19, 2021

MySQL don't support FLOAT(M,D) and DOUBLE(M,D) since 8.0.17
https://dev.mysql.com/doc/refman/8.0/en/floating-point-types.html
we ignored decimal in PR 21788

@cfzjywxk
Copy link
Contributor Author

@b41sh Got, do we have related document to remind users about this incompatibily doing upgrade.

@b41sh
Copy link
Member

b41sh commented Feb 19, 2021

@cfzjywxk This is recently modified, the document should not have been modified yet.

@johan-j
Copy link
Contributor

johan-j commented Feb 19, 2021

There is still issues with float type.
To reproduce:
create table t(a float);
insert into t( -34028236.66);

mysql8:

+-----------+
| a         |
+-----------+
| -34028200 |
+-----------+

tidb:
+-----------+
| a         |
+-----------+
| -34028236 |
+-----------+

The issue reported here in (22791), seems to have started after https://github.com/pingcap/tidb/pull/21788.


@cfzjywxk cfzjywxk added the type/bug The issue is confirmed as a bug. label Feb 19, 2021
@johan-j
Copy link
Contributor

johan-j commented Feb 25, 2021

/assign

@johan-j
Copy link
Contributor

johan-j commented Feb 26, 2021

Im working on this in #22823

The issue I mention in the comment above:

create table t(a float);
insert into t(a) values(-34028236.66);

Can be solved without solving the deprecated double(m,n)/float(m,n) compatibility issue mentioned in this issue.
So Im thinking if I should create a new issue for this instead, to track them separately?
So 2 issues:

Solving the float(m,n) problem I can do by printing in scientific format (which mysql does not), the same way as: #21692. If this is OK solution, I can implement that and solve both.

johan-j added a commit to johan-j/tidb that referenced this issue Feb 28, 2021
This resolve issue pingcap#22791: query result for float type is incompatible with previous version

It seems like this broke in the fix for issue pingcap#21692. In mysql, e-format
is only returned when used in the query, for example:
select 1e15, will print 1e15 back.
However, when stored in a table mysel will print it in decimal form.
issue pingcap#21692 resolved this by checking for empty table. However, that
fix applies for everything rather than only for e-format. And that
caused the new issue pingcap#22791.

This resolves both pingcap#22791 and pingcap#21692.

It also resolve an issue where for a default width float column we do
not round to the mysql default precision.
@XuHuaiyu XuHuaiyu added sig/sql-infra SIG: SQL Infra and removed sig/execution SIG execution labels Mar 2, 2021
@jebter jebter added this to the v5.0.0 ga milestone Mar 21, 2021
@zimulala
Copy link
Contributor

/cc @XuHuaiyu

@xiongjiwei
Copy link
Contributor

we have a document that says the difference

@bb7133
Copy link
Member

bb7133 commented Sep 9, 2021

Close it since we decide not to fix it.

@bb7133 bb7133 closed this as completed Sep 9, 2021
@bb7133 bb7133 added the wontfix This issue will not be fixed. label Sep 9, 2021
@github-actions
Copy link

github-actions bot commented Sep 9, 2021

Please check whether the issue should be labeled with 'affects-x.y' or 'backport-x.y.z',
and then remove 'needs-more-info' label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
severity/major sig/sql-infra SIG: SQL Infra type/bug The issue is confirmed as a bug. type/compatibility wontfix This issue will not be fixed.
Projects
None yet
9 participants