Skip to content

Conversation

@varundeepsaini
Copy link
Contributor

Fix precision loss when loading state with large integer IDs

Problem

json.Unmarshal decodes numbers as float64 when the destination is any, causing precision loss for integers > 2^53 (like job IDs).

Solution

Use json.NewDecoder with UseNumber() to preserve integer precision.

Tests

Added unit tests for dstate package, including one that verifies the precision fix.

@varundeepsaini
Copy link
Contributor Author

@andrewnester @denik
can you review this? I'll need this to fix the flaky tests in #4105

@github-actions
Copy link

An authorized user can trigger integration tests manually by following the instructions below:

Trigger:
go/deco-tests-run/cli

Inputs:

  • PR number: 4136
  • Commit SHA: e580eee2fc36a1def4a5cd9e23e91f1e0adea460

Checks will be approved automatically on success.

@varundeepsaini
Copy link
Contributor Author

@denik fixed the linter error, coudl you please queue the tests again

@denik
Copy link
Contributor

denik commented Dec 12, 2025

Thanks @varundeepsaini this is a great find and your fix is valid. I have somewhat more general one here: #4138

@varundeepsaini
Copy link
Contributor Author

thanks, i'll close this pr then, and rebase + update the tests in the reverse dependancy pr

github-merge-queue bot pushed a commit that referenced this pull request Dec 12, 2025
## Changes
Instead of parsing into any and then converting into the target type, we
now store json.RawMessage and then parse into target type when needed.

## Why
One issue it fixes is that JSON parser by default parses integers as
float64, losing precision. This was found by @varundeepsaini and a fix
is proposed in #4136

This PR is more general though, fully relying on the type to parse
itself and it should more efficient as well as we're not walking the
type twice & we're not having 2 copies of it.

## Tests
New test where int64 field is initialized with large numbers. It fails
on main.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants