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

vstream: json integers get converted into scientific notation #8686

Closed
derekperkins opened this issue Aug 25, 2021 · 4 comments · Fixed by #12761
Closed

vstream: json integers get converted into scientific notation #8686

derekperkins opened this issue Aug 25, 2021 · 4 comments · Fixed by #12761

Comments

@derekperkins
Copy link
Member

derekperkins commented Aug 25, 2021

Overview of the Issue

As with other prior json bugs, this appears to be another place where the copy vs stream handles json differently. We are consuming this via vstream / messaging. I believe this error is in the stream phrase, but works correctly in the copy phase, though I'm not 100% sure on the distinction.

Original json

{"date": 1629849600, "keywordSourceId": 930701976723823, "keywordSourceVersionId": 210825230433}

Invalid json (though technically still correct)

{"keywordSourceVersionId":2.10825230433e+11,"date":1.6298496e+09,"keywordSourceId":9.30701976723823e+14}

Operating system and Environment details

vtgate: v10.0.2
vttablet: v11.0.0

cc @rohit-nayak-ps

@derekperkins
Copy link
Member Author

derekperkins commented Aug 26, 2021

@setassociative pointed out that the scientific notation isn't actually invalid, but it presents as an error when unmarshaling to an integer in Go.

assertInteger: can not decode float as int, error found in #10 byte of ...|rsionId":2.108252304

Example on the Go playground: https://play.golang.org/p/OAfcfo_Ur2I

I found the test cases showing that this is apparently expected behavior. I'm curious what the reasoning was for handling it that way, or if that is just how github.com/spyzhov/ajson handles it. I think we should change the behavior to not use scientific notation.
https://github.com/vitessio/vitess/blob/main/go/mysql/binlog_event_json_test.go#L95-L125

@rohit-nayak-ps
Copy link
Contributor

The ajson module only supports a single type Numeric (float64) as a catchall for all numeric types including signed and unsigned integers. I have forked ajson and added new integer types. This should fix the issue reported here. The updated json parser which uses this fork is at #9508.

It also looks like there are some flavor-related differences in how json is represented in the binlog or db. I was not able to initially reproduce your errors on 5.7 which is my default mysql version. It does reproduce on 8.0.21.

@derekperkins can you please take a look at the tests at

func TestJSONTypes(t *testing.T) {
and let me know if we should add additional ones? I have updated the old ones and added a few around integers.

@lisachenko-indriver
Copy link

We also hitting this issue, because our original MySQL table uses JSON fields heavily and during replication all big integers were replaced with float equivalents inside JSON.

@dbussink
Copy link
Contributor

dbussink commented Apr 6, 2023

#12761 is the more fundamental fix here which keeps as much as the relevant type information in the JSON document as possible.

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

Successfully merging a pull request may close this issue.

4 participants