[walidcavelius]

Description

Unmarshal decodes attributes into a map[string]any and then re-marshals them into the target struct. Every number goes through float64 on the way, so integers above 2^53 are rounded even when the field is int64 or uint64 (9007199254740993 arrives as 9007199254740992).

This adds an opt-in UnmarshalUseNumber() option:

• Without the option: nothing changes. Attributes are decoded into the map during the first pass, as before.
• With the option: the option is passed to the document before decoding, so resource objects keep the raw attribute bytes (a reference to the input, not a copy) and decode them directly into the target with UseNumber. Typed integer fields get every digit and interface{} values (e.g. map[string]any) hold json.Number instead of float64. This also applies to data arrays, included resources and relationship data.

Attribute value	Default (unchanged)	UnmarshalUseNumber()
9007199254740993 → int64	9007199254740992	9007199254740993
18446744073709551615 → uint64	error	18446744073709551615
9007199254740993 → map[string]any	float64(9007199254740992)	json.Number("9007199254740993")
100.0 → int64	100	error

Why it's opt-in

With the option, every number in an interface{} value becomes json.Number, small ones included. Code expecting float64 (type assertions, case float64, comparisons like v == 3.0, reflection, some encoders: YAML writes json.Number as a quoted string, see DataDog/datadog-agent#58306) would break. Decoding directly is also stricter: non-integer literals like 100.0 no longer decode into integer fields, which the float64 round-trip allowed.

Additional context

Tests are in TestUnmarshalNumbers, including a data array, an included resource reached through a relationship, and attributes in relationship data. go test -race ./... and golangci-lint v1.59.1 pass locally (Go 1.27 for tests; Go 1.21/1.22 binaries don't run on my macOS, so CI covers those).

Benchmarks:

• BenchmarkUnmarshal didn't reach attribute decoding: its targets were *interface{}, the meta target wasn't a pointer, and the error was discarded. It now uses a fresh concrete target per iteration and fails on error.
• BenchmarkUnmarshalNumbers is new and covers both modes with a 4 KiB string, 1,000 integers, and 32 resources referencing one included resource.
• Without the option, decoding is unchanged: same time and bytes/op as main within noise (benchstat, n=6, runs interleaved).
• With UnmarshalUseNumber(), it's a trade-off. Number-heavy payloads allocate about a third less and decode faster. Large strings allocate 20–30% more, because json.Decoder buffers the whole value. An included resource referenced many times allocates about twice as much, since it's decoded from the raw bytes for each reference.

Resource and document meta are not changed.

🤖 Generated with Claude Code

[datadog-prod-us1-3]

_{This comment will be updated automatically if new data arrives.
🔗 Commit SHA: b28a10f | Docs | Give us feedback!}

[jack-edmonds-dd]

IIUC all the extra logic in jsonapi.go (hasLongDigitRun, floatNonIntegers) is there so we can fix typed int64 fields by default while still preserving the old side effect of the
map[string]any -> []byte round-trip (which shaved .0 off floats like 100.0 -> 100 so they could unmarshal into int64).

Since we only saw one deprecated route affected by typed int64, could we put both fixes behind UnmarshalUseNumber() so we don't need hasLongDigitRun or floatNonIntegers at all?

Specifically, in resourceObject.UnmarshalJSON we could capture attributes as a json.RawMessage (like we already do for relationships). Then in unmarshalAttributes:

• Old path (!m.useNumber): unmarshal the json.RawMessage into map[string]any, remarshal to []byte, and json.Unmarshal(b, v)
• Opt-in path (m.useNumber): decode the json.RawMessage directly into v with d.UseNumber(), which preserves precision for both typed int64 fields and map[string]any.

[nogates]

The precision fix looks sound, but I found an allocation regression on the default path and a problem with the benchmark cited in the PR description.

The two attribute fields do not both get populated during decoding: auxRaw.Attrs shadows the embedded Attributes map, which stays nil. Assigning auxRaw.Attrs to ro.rawAttributes only copies the slice header. However, json.RawMessage.UnmarshalJSON copies the attribute bytes, so the default path now retains an additional raw buffer while still decoding a map, re-marshalling it, and decoding the target. The opt-in path avoids that map/re-marshalling, but json.Decoder allocates its own buffer for the complete attribute value.

Shared included resources also matter: the default path now rebuilds the attribute map each time the same included resource is decoded through a relationship. Previously, the parsed attribute map was reused.

The existing benchmark does not measure successful attribute decoding. At unmarshal_test.go:1058, bm.target is any, so this call:

_ = Unmarshal(data, &bm.target, bm.opts...)

returns got type "*interface {}" expected one of "struct" for the single-resource cases, before reaching attribute decoding. The error is discarded. I reproduced this and confirmed that passing a concrete struct pointer succeeds. This benchmark defect predates this PR, but the performance claims based on it do not establish the cost of successful decoding.

I ran separate benchmarks comparing base 1ae852c8f1300bdf3334f0578f9adcfaf820b734 with this PR at 186e504909f4cf38151ffb274b35503f8cbb20ca. Each iteration uses a fresh concrete destination, checks the error, and retains the result in a package-level sink. Input construction is outside the timed loop; normal member-name validation remains enabled.

Environment: Go 1.27.3, darwin/arm64, Apple M4 Max. Command on each snapshot:

go test -run '^$' -bench '^BenchmarkReviewMemory$' -benchmem -benchtime=200ms -count=3

The table shows the median of three samples for each metric. These are small local measurements, not statistically established performance guarantees. B/op is total allocation per call, not peak live heap or RSS.

Payload	Version/mode	ns/op	B/op	allocs/op
64-byte string	Base/default	4,115	2,616	52
64-byte string	PR/default	4,448	2,753	54
64-byte string	PR/UseNumber	3,976	2,935	49
4 KiB string	Base/default	22,745	24,519	55
4 KiB string	PR/default	26,081	29,463	57
4 KiB string	PR/UseNumber	25,291	36,853	57
1 MiB string	Base/default	4,613,825	5,942,532	70
1 MiB string	PR/default	5,351,118	6,943,009	75
1 MiB string	PR/UseNumber	4,757,004	8,416,356	82
1,000 integers	Base/default	249,105	138,961	4,086
1,000 integers	PR/default	257,799	143,176	4,088
1,000 integers	PR/UseNumber	158,133	95,421	2,077
32 references to one included resource	Base/default	535,588	717,436	2,609
32 references to one included resource	PR/default	768,318	999,220	2,863
32 references to one included resource	PR/UseNumber	632,734	1,481,451	2,905

The string cases contain one string attribute; the integer case contains a typed []int64 with values 0–999. The shared-resource case has 32 primary resources referencing the same included resource containing an 8 KiB string.

For example, the 4 KiB string allocates 20% more bytes on the default path and 50% more with UseNumber, relative to base. The shared-resource case allocates 39% more by default and 106% more with UseNumber. Conversely, the integer-array case allocates 31% fewer bytes with UseNumber, so the tradeoff depends on the payload.

These intermediate buffers are temporary and collectible, but increase allocation/GC pressure. Interface-valued numeric results also change representation from float64 to json.Number, so retained output memory can differ too; I did not measure retained heap separately.

go test -race ./... passed, as did additional checks that the attribute map stays nil during initial decoding and that the option preserves integer precision through an included relationship.

I would fix the existing benchmark, add committed benchmarks for both modes with representative payloads, and explicitly assess the default-path regression before merging. Ideally, callers who do not enable the option would retain the existing decoding path.

Benchmark source used on the PR snapshot

package jsonapi
import ("encoding/json";"strings";"strconv";"testing")
type reviewItem struct {
 ID string `jsonapi:"primary,items" json:"-"`
 Blob string `jsonapi:"attribute" json:"blob"`
 Numbers []int64 `jsonapi:"attribute" json:"numbers"`
 Related *reviewItem `jsonapi:"relationship" json:"related"`
}
var reviewSink any
func BenchmarkReviewMemory(b *testing.B) {
 modes:=[]struct{name string;opts []UnmarshalOption}{{"default",nil},{"useNumber",[]UnmarshalOption{UnmarshalUseNumber()}}}
 for _,mode:=range modes {
  for _,size:=range []int{64,4096,1048576} {
   blob,_:=json.Marshal(strings.Repeat("a",size))
   data:=[]byte(`{"data":{"type":"items","id":"1","attributes":{"blob":`+string(blob)+`}}}`)
   b.Run("string"+strconv.Itoa(size)+"/"+mode.name,func(b *testing.B){b.ReportAllocs();b.SetBytes(int64(len(data)));for i:=0;i<b.N;i++ {var v reviewItem;if err:=Unmarshal(data,&v,mode.opts...);err!=nil{b.Fatal(err)};reviewSink=&v}})
  }
  nums:=make([]int64,1000);for i:=range nums {nums[i]=int64(i)};raw,_:=json.Marshal(nums)
  data:=[]byte(`{"data":{"type":"items","id":"1","attributes":{"numbers":`+string(raw)+`}}}`)
  b.Run("numbers1000/"+mode.name,func(b *testing.B){b.ReportAllocs();for i:=0;i<b.N;i++ {var v reviewItem;if err:=Unmarshal(data,&v,mode.opts...);err!=nil{b.Fatal(err)};reviewSink=&v}})
  blob,_:=json.Marshal(strings.Repeat("a",8192));parts:=make([]string,32)
  for i:=range parts {parts[i]=`{"type":"items","id":"`+strconv.Itoa(i)+`","relationships":{"related":{"data":{"type":"items","id":"shared"}}}}`}
  shared:=[]byte(`{"data":[`+strings.Join(parts,",")+`],"included":[{"type":"items","id":"shared","attributes":{"blob":`+string(blob)+`}}]}`)
  b.Run("sharedIncluded32/"+mode.name,func(b *testing.B){b.ReportAllocs();for i:=0;i<b.N;i++ {var v []reviewItem;if err:=Unmarshal(shared,&v,mode.opts...);err!=nil{b.Fatal(err)};reviewSink=v}})
 }
}

For the base snapshot, remove the useNumber entry from modes; the rest of the benchmark is identical.