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

Remove large-int-id String serialization middleware [DO NOT MERGE YET] #44791

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

camsaul
Copy link
Member

@camsaul camsaul commented Jun 26, 2024

Part of the fix for #5816

Since JS stores all numbers as 64-bit doubles by default it can't parse integer literals larger than 9007199254740991 in JSON query results. As a hack we serialized those numbers to strings. Going forward we'll just tweak our JSON parser to parse them to BigInts instead.

This PR removes the icky middleware that serialized large integers to strings. The main goal here is to see if any backend tests break and need to be updated, or if we have e2e tests around this stuff

See Slack discussion https://metaboat.slack.com/archives/C04DN5VRQM6/p1719433869236109

@camsaul camsaul changed the title Remove large-int-id middleware Remove large-int-id middleware [DO NOT MERGE YET] Jun 26, 2024
@metabase-bot metabase-bot bot added the .Team/QueryProcessor :hammer_and_wrench: label Jun 26, 2024
@camsaul camsaul changed the title Remove large-int-id middleware [DO NOT MERGE YET] Remove large-int-id String serialization middleware [DO NOT MERGE YET] Jun 26, 2024
Copy link

replay-io bot commented Jun 26, 2024

Status Complete ↗︎
Commit 3392283
Results
5 Failed
⚠️ 1 Flaky
2705 Passed

@alxnddr alxnddr mentioned this pull request Jun 27, 2024
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
.Team/QueryProcessor :hammer_and_wrench:
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

1 participant