Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request updates the SQLite source to preserve JSON number precision (such as large integers) by using json.RawMessage and json.Valid instead of unmarshaling to any, and adds corresponding unit and integration tests. The reviewer noted that using json.Valid on arbitrary strings can lead to unexpected type conversions for plain text columns (e.g., numeric strings or booleans) and suggested restricting the JSON detection heuristic to strings enclosed in braces or brackets.
| if jsonString, ok := val.(string); ok { | ||
| var unmarshaledData any | ||
| if json.Unmarshal([]byte(jsonString), &unmarshaledData) == nil { | ||
| row.Add(name, unmarshaledData) | ||
| jsonData := json.RawMessage(jsonString) | ||
| if json.Valid(jsonData) { | ||
| row.Add(name, jsonData) | ||
| continue | ||
| } | ||
| } |
There was a problem hiding this comment.
Using json.Valid on any string can lead to unexpected type conversions for plain text columns. For example, if a column contains a plain numeric string like "12345" (e.g., a zip code or phone number) or a boolean string like "true", json.Valid will return true. This causes the string to be treated as a JSON raw message and serialized as a JSON number or boolean in the final output, losing its original string type and potentially leading to bugs.
To prevent this and improve performance, we should restrict the JSON detection heuristic to strings that look like JSON objects or arrays (i.e., starting with { and ending with }, or starting with [ and ending with ]).
| if jsonString, ok := val.(string); ok { | |
| var unmarshaledData any | |
| if json.Unmarshal([]byte(jsonString), &unmarshaledData) == nil { | |
| row.Add(name, unmarshaledData) | |
| jsonData := json.RawMessage(jsonString) | |
| if json.Valid(jsonData) { | |
| row.Add(name, jsonData) | |
| continue | |
| } | |
| } | |
| if jsonString, ok := val.(string); ok { | |
| if len(jsonString) >= 2 && ((jsonString[0] == '{' && jsonString[len(jsonString)-1] == '}') || (jsonString[0] == '[' && jsonString[len(jsonString)-1] == ']')) { | |
| jsonData := json.RawMessage(jsonString) | |
| if json.Valid(jsonData) { | |
| row.Add(name, jsonData) | |
| continue | |
| } | |
| } | |
| } |
|
The scalar detection is intentionally unchanged: base Restricting detection to objects and arrays would change the existing scalar behavior and belongs to the separate detection work discussed in #3660. I am keeping this fix focused on precision and retaining the current JSON detection contract. |
Description
Fixes #4430. SQLite JSON results containing 64-bit IDs are rounded on the server:
json_object('id', 9007199254740993)reaches the tool response as9007199254740992.Keep valid JSON as
json.RawMessageinstead of decoding numbers throughany/float64. Both SQLite tools share this path; ordinary text retains its existing fallback. Related #3660 optimizes JSON detection and row serialization; it does not preserve numeric precision.The real SQLite unit and public HTTP regression both fail on
06ea49dwith the rounded ID and pass after the fix. SQLite source/tool unit packages, all fourTestSQLiteExecuteSqlToolcases, source-package vet, scoped golangci-lint 2.15.0 and goimports formatting pass. Go 1.27.3; module files unchanged.PR Checklist
Validation scope
The existing HTTP fixture was temporarily pointed at a free local port because 5000 was occupied; the port changes were restored. The regression exercises the real temporary database and tool endpoint. No external database or model service was used. The full repository integration matrix and coverage comparison were not run.