expression: round function for int should use round half up rule

[feitian124]

What problem does this PR solve?

Issue Number: close #26993

Problem Summary:

Result of function round(50, -2) is 100 in mysql, but 0 in tidb.

#21324 expression: change the round rule for approximate value to round to nearest even,
but 50 is exact-value, should uses the “round half up” rule

What is changed and how it works?

What's Changed:
change func Round(f float64, dec int) to func Round(f float64, dec int, r ...RoundRule) to accept a r ...RoundRule parameter,
which use RoundNearestEven as default if not specified.

Check List

Tests

• Unit test
• Integration test
• Manual test (add detailed scripts or steps below)
• No code

Release note

None

[ti-chi-bot]

[REVIEW NOTIFICATION]

This pull request has not been approved.

To complete the pull request process, please ask the reviewers in the list to review by filling /cc @reviewer in the comment.
After your PR has acquired the required number of LGTMs, you can assign this pull request to the committer in the list by filling /assign @committer in the comment to help you merge this pull request.

The full list of commands accepted by this bot can be found here.

Reviewer can indicate their review by submitting an approval review.
Reviewer can cancel approval by submitting a request changes review.

[feitian124]

/run-all-tests

[feitian124]

/cc @ichn-hu @wshwsh12 @XuHuaiyu
i see you in #21324 so maybe you are interested
this is my first time slove real tidb issue, if some wrong or need improve, please let me know.
thanks.

[feitian124]

sorry test fails, my change should have no impact on other codes. need time dig..

[ichn-hu]

the quotation mark is Chinese quoation

[ichn-hu]

Suggested change

	// Round rounds the argument f to dec decimal places use “round to nearest even” rule as default.
	// Round rounds the argument f to dec decimal places use "round to nearest even" rule as default.

[ichn-hu]

It looks too hacky to me, perhaps we can just add a rule parameter to the signature of this function and change all the places where it is used.

[wshwsh12]

Maybe we should add a function func RoundInt64(f int64, dec int) int64?

[feitian124]

yes, i came up with 2 change way:

• current one
  • prons: less change, adhere to golang which is only float64 version of round also; and since it is float64, it make sense use RoundNearestEven rule as default
  • crons: a little performance affact
• addtional methons like RoundInt64
  • prons: better performance, straightforward, maybe adhere to mysql implements better
  • crons: more change

[feitian124]

each have its own advantage, if you agree later one is better, i can update

[riteme]

I think it's better that integer round does not reuse floating-point round, because Float64 can't represent all integer values and will introduce inaccuracy in computation.

I can offer an example that shows why floating-point round isn't suitable:

In TiDB:

mysql> select round(123456789, -5);
+----------------------+
| round(123456789, -5) |
+----------------------+
|            123499999 |
+----------------------+
1 row in set (0.001 sec)

In MySQL 8.0:

mysql> select round(123456789, -5);
+----------------------+
| round(123456789, -5) |
+----------------------+
|            123500000 |
+----------------------+
1 row in set (0.006 sec)

TiDB's output is expected for floating-point round even if you use "round to even" rule: https://coliru.stacked-crooked.com/a/a3dbb35ff61b3944.

[riteme]

By the way, MySQL 8 will report an error if round result overflows:

mysql> select round(18446744073709551615, -19);
ERROR 1690 (22003): BIGINT UNSIGNED value is out of range in 'round(18446744073709551615,-(19))'

[feitian124]

thank you for suggestion, i will add a round function for integer, and maybe adjust some old names to make them consistent

[feitian124]

TiDB's output is expected for floating-point round even if you use "round to even" rule: https://coliru.stacked-crooked.com/a/a3dbb35ff61b3944.

	f := 123499999.99999999
	i := int64(f)
	i2 := math.Round(f)
	fmt.Printf("i: %+v, i2: %+v", i, i2)
	// OutPut: i: 123499999, i2: 1.235e+08

for tidb round(123456789, -5) get 123499999 is caused by convert int64(f), can be fixed by use convert math.Round(f)

[riteme]

can be fixed by use convert math.Round(f)

I can offer another example:

TiDB:

mysql> select round(2146213728964879326, -15);
+---------------------------------+
| round(2146213728964879326, -15) |
+---------------------------------+
|             2145999999999999744 |
+---------------------------------+
1 row in set (0.000 sec)

MySQL 8.0:

mysql> select round(2146213728964879326, -15);
+---------------------------------+
| round(2146213728964879326, -15) |
+---------------------------------+
|             2146000000000000000 |
+---------------------------------+
1 row in set (0.001 sec)

I guess this one cannot be simply fixed by math.Round.

[feitian124]

yes, you are right, round is not as easy as thought, see the link in my comments.
I will add your example as test case and hopefully get a perfect solution.

[feitian124]

seems correct rounding is not easy, https://www.cockroachlabs.com/blog/rounding-implementations-in-go/
need more research

[feitian124]

yes, i came up with 2 change way:

• current one

  • prons: less change, adhere to golang which is only float64 version of round also; and since it is float64, it make sense use RoundNearestEven rule as default
  • crons: a little performance affact

• addtional methons like RoundInt64

  • prons: better performance, straightforward, maybe adhere to mysql implements better
  • crons: more change

this pr is deprecated, see #27403 which try resolve this issue use the second way

[tisonkun]

@feitian124 shall we close this PR then?