Patch float with trailing zero for JSON and add useFloatWithTrailingZero flag #1038
Conversation
chenhuiyeh
changed the title
|
It would be good to add some tests and a description or an issue link. |
chenhuiyeh
changed the title
chenhuiyeh
force-pushed
the
float_with_trailing_zero
branch
from
814aa96 to
9d46fb3
Compare
|
Hi, please fix the CI. And I'll take a look after you remove the draft status of PR. |
|
Hi, do you need some help? |
|
I was away, will be picking this up today. I'll reach out if I have any
questions. Thank you
…On Tue, Jun 3, 2025 at 2:57 AM lance6716 ***@***.***> wrote:
*lance6716* left a comment (go-mysql-org/go-mysql#1038)
<#1038 (comment)>
Hi, do you need some help?
—
Reply to this email directly, view it on GitHub
<#1038 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AICWJGSXBSUSUAORVULNSQD3BVBOZAVCNFSM6AAAAAB5SC73WGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDSMZTG44DAMRTHA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
This comment was marked as resolved.
This comment was marked as resolved.
| @@ -0,0 +1,403 @@ | |||
| package replication | |||
There was a problem hiding this comment.
I decided to pull the unit tests out specifically for this feature/fix
There was a problem hiding this comment.
rest lgtm. some linter errors
Error: replication/json_binary_test.go:372:14: unusedwrite: unused write to field tableIDSize (govet)
tableIDSize: 6,
^
Error: replication/json_binary_test.go:374:10: unusedwrite: unused write to field Version (govet)
Version: 2,
^
Error: replication/json_binary_test.go:381:14: unusedwrite: unused write to field tableIDSize (govet)
tableIDSize: 6,
^
Error: replication/json_binary_test.go:383:10: unusedwrite: unused write to field Version (govet)
Version: 2,
^
Error: replication/json_binary_test.go:297:[36](https://github.com/go-mysql-org/go-mysql/actions/runs/15475222894/job/43585097889?pr=1038#step:4:38): SA4026: in Go, the floating-point literal '-0.0' is the same as '0.0', it does not produce a negative zero (staticcheck)
input: FloatWithTrailingZero(-0.0),
^
| }, | ||
| } | ||
|
|
||
| // 5.0 as IEEE 754 double precision in little endian |
There was a problem hiding this comment.
nits: I think you can use https://pkg.go.dev/math#Float64bits and binary.BigEndian.PutUint64. This line with comment is also good enough.
| useFloatWithTrailingZero: true, | ||
| } | ||
|
|
||
| // Mock a simple JSON binary that would contain a double value |
There was a problem hiding this comment.
the codes are not aligned with these comments. Maybe delete this test?
| require.False(t, parser.useFloatWithTrailingZero) | ||
| } | ||
|
|
||
| func TestBinlogSyncerConfig_UseFloatWithTrailingZero(t *testing.T) { |
There was a problem hiding this comment.
nit: I think this test is also a bit trivial, we don't need it.
| }, | ||
| { | ||
| name: "negative zero", | ||
| input: FloatWithTrailingZero(-0.0), |
There was a problem hiding this comment.
Maybe you really want to test -0.0, but it can't be expressed by golang literal. I think you can manually get the IEEE 754 byte representation and check it in other test cases
This pull request enables support for MySQL 8+ for retaining trailing zeros in floating-point numbers in JSON format. This is achieved through the addition of the
useFloatWithTrailingZerosetting option within the go-mysql library. Note this only works with MySQL version 8 onwards.A patch addressing this feature was initially applied in ghostferry PR #368. This current PR seeks to integrate these updates into the upstream codebase to extend functionality for users and projects relying on this library.
This change is part of the broader initiative referenced in ghostferry PR #384.