server: Make float rounding compatible with mysql for types: float, float(m,n) and double(m,n)

[johan-j]

What problem does this PR solve?

Issue Number: close #22791

Problem Summary:

• Tidb type float, have to much precision compared to mysql
• Tidb not printing decimals for custom sized float and double types.

What is changed and how it works?

What's Changed:
This resolve issue #22791: query result for float type is incompatible with previous version

When #21692 was resolved, it created a new problem reported in #22823 (float/double not displaying decimals)
This resolves issues: #22791, #21692, and a new issue with float I found while working on this (mentioned in comment on issue #22791)

Default width float issue:

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

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

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

For this case there was actually even a test case. But the test case expected -34028236 as result, which ment it passed even though the value is to precis for mysql.

Previously we relied on go's built in strconv.AppendFloat to both round and format. However after much effort I came up with the solution to treat rounding and formatting separately.
Instead now it works in 2 steps:

1. Call new rounding function that returns in mysql precision (if needed)
2. Use strconv.AppendFloat to format output with correct number of decimals.

This also means that the documentation at: https://docs.pingcap.com/tidb/stable/mysql-compatibility#incompatibility-caused-by-deprecated-features
could mention that deprecated types are now compatible with mysql, even if its encouraged to migrate to Decimal. Many big companies still run older versions like 5.7 of mysql and have no plans to migrate. So keeping it compatible might still be important to get some companies to use TiDB.

The new output formatter supports the below scenarios (which is also the doc/comment in the code):

/*
	Formats a float value into a string (byte slice) and returns.

	The rounding/formatting is split into the following cases:
	1. Scientific notation (1.23.e4): Used for big numbers except when value comes from a custom width column or from the select part of the query
	1. mysql.TypeDouble: return string with as much precision as needed (precision = -1)
	2. mysql.TypeFloat: return a rounded value that corresponds to mysql's (precision = 5)
	3. mysql.TypeDouble(m,n):
		3.1 If the columnLength < defaultMySQLPrec64, return string with as much precision as needed (precision = n)
		3.2 else: return rounded string to precision defaultMySQLPrec64, and padded with 0's.
	4. mysql.TypeFloat(m,n): return a rounded value rounded with (precision = 5) and formatted with (precision = n)

	Note: 	Custom float/double column width is deprecated in mysql 8.0.14.
			However for compatibility, we still try to make the output the same as mysql for these types.
*/

Check List

Tests

• Unit test

Release note

• Resolve issue where float type was not returned in the same way as mysql8.0.
• Resolve issue where custom length double/float was returned in incorrect format.

[johan-j]

Closing this PR this since mysql 8.0.14 have deprecated double(m,n) and float(m,n).

[johan-j]

I re-open this PR again since I found there are still float formatting issues even for the float type with default column width.

[johan-j]

I deleted this test because for a column of 3 decimals, the input is already rounded when we store it. So the result is correct in an integration test.
However the result from the function test will not be correct. Let me know if you want me to implement this in the output formatter too, even though its truncated on input.

[johan-j]

Hi @djshow832 , thank you for your comment. Do you or @AilinKid (who is also assinged to this PR) have anymore feedback on this MR? Thanks in advance to have a look!

[johan-j]

seems like you guys has no interest to resolve these bugs, so closing this PR instead.

[AilinKid]

I'm so sorry for the late response to your PR cause my notification mechanism is broken recently. Even for now, the float problem is still one of the big compatibility problems on our list.
You can refer to some details from: #11328

I'm not sure whether your pull request can solve this problem, so let's review it first, and if you are interested you can add a more detailed design about your work.

More again, thanks for your contribution. @johan-j

[johan-j]

thank you @AilinKid for showing interest in this pull request. I hope it can resolve the compatibility issues. I put quite some effort into making this PR.

"and if you are interested you can add a more detailed design about your work."
Any suggestion what information is missing? I thought the comment on how the formatter works is already quite detailed. Im happy to clarify where needed! but some specific questions I can answer to make it more clear?

[AilinKid]

better add a mysql referrence link here, and could you descript the imcompatiable behavior with mysql here a little bit?

[AilinKid]

Suggested change

	if colDecimal > 0 && colDecimal != mysql.NotFixedDec {
	if colDecimal > 0 && int(colDecimal) != mysql.NotFixedDec {

[AilinKid]

it's same as the default logic

[AilinKid]

Rest LGTM, thx for your detailed work.

[ti-chi-bot]

@johan-j: PR needs rebase.

Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository.