Implement jsonb_set - #3181
Conversation
|
|
SummaryCoverage spans normal nested and array updates, preservation of surrounding data, creation of missing values, null handling, malformed-path errors, and boundary cases involving unusually large numbers. It also checks whether supported update capabilities are visible to database tools, with the core update behavior broadly healthy but an important data-integrity edge case failing. Not safe to merge yet — this change can silently alter large integers and precise decimals during updates, creating a high-severity data-integrity risk even though ordinary update and error-handling behavior works. A separate medium-severity metadata discovery issue is unrelated to this pull request and is a flag for later rather than a merge driver. Tests run by ItoAdditional Findings DetailsThese findings are unrelated to the current changes but were observed during testing. 🟡 Function metadata hides supported JSON updates
Evidence PackageTip Reply with @itoqa to send us feedback on this test run. |
|
@fulghum DOLT
|
52b9a2c to
aa5c871
Compare
Commit: SummaryCoverage spans core JSON data updates and persistence, including replacing nested object and array values while preserving unrelated data and exact numeric precision. It also exercises edge and adversarial inputs such as missing or malformed values, invalid paths, whitespace and index boundaries, out-of-range insertion behavior, and session/transaction consistency. Safe to merge — all exercised behaviors passed, with no regressions, new failures, or previously flagged failures attributable to this PR. No merge blocker is indicated; the run is low risk. Tests run by Ito
Tip Reply with @itoqa to send us feedback on this test run. |
aa5c871 to
6bd3b54
Compare
Commit: SummaryCoverage spans normal and nested JSON updates, array and path edge cases, null and malformed input handling, copy safety, and exact numeric preservation. The update behavior is broadly healthy, but large or highly precise JSON numbers can be silently changed during storage, creating a serious data-integrity risk. Not safe to merge yet — this PR introduces a high-severity data-corruption issue in a supported JSON data path, where large or high-precision numbers lose their exact values before later updates or reads. This is a merge blocker rather than a minor edge-case concern. Tests run by ItoTip Reply with @itoqa to send us feedback on this test run. |
| return types.JSONDocument{Val: result}, nil | ||
| } | ||
|
|
||
| func jsonbValueToInterface(ctx *sql.Context, value any) (any, error) { |
There was a problem hiding this comment.
Large JSON numbers lose precision
What failed: The database accepted the JSON document, but changed its large numeric value while saving it. Invalid trailing JSON was rejected correctly and did not create a partial row, so the failure is the silent precision loss on the valid document.
Impact · Steps · Stub / mock · Analysis · Why this is likely a bug
- Severity: High
- Impact: Users who save very large or highly precise numbers in JSON data can have those values silently changed. Later reads and updates use the wrong value, which can cause incorrect records or calculations.
- Steps to Reproduce:
- Create a JSONB table with a value such as {"value":123456789012345678901234567890.123456789}.
- Read the stored JSONB value and note that the number has changed to 123456789012345680000000000000.
- Attempt an insert with two top-level JSON values and an update with trailing text; both should be rejected without changing the row.
- Run jsonb_set on the persisted value to add another field and read the result back.
- Compare the numeric field with the original input; it remains rounded even though the later update succeeds.
- Stub / mock content: No stubs, mocks, or bypasses were applied for this test in the recorded run.
- Code Analysis: The runtime value is consistent with the PR's JSONB conversion design. The PR added jsonb_set in server/functions/jsonb_set.go and its jsonbValueToInterface helper reads JSONBytes with pgtypes.DecodeJSONValue at lines 90-101. The PR also changed server/cast/jsonb.go so string JSONB values use the same decoder. In the defective revision, the shared conversion ultimately exposed numbers through the native float64 path; server/types/json_document.go lines 361-368 explicitly document that path as not precise enough and convert the float to an apd.Decimal only after the precision has already been lost. A 30-digit integer therefore becomes 123456789012345680000000000000 before jsonb_set can copy or update the document. The current source contains the targeted remediation: DecodeJSONValue at server/types/json_document.go lines 245-261 uses Decoder.UseNumber, converts each json.Number to an apd.Decimal at lines 264-289, and server/functions/json.go lines 31-53 and 84-105 retain validated bytes in preciseJSONDocument. That remediation confirms the required fix is localized to preserving exact JSON numeric representation across input, storage, and jsonb_set conversion rather than changing path-update logic.
- Why this is likely a bug: PostgreSQL JSONB numeric values are expected to retain their exact textual value, and the test plan specifically requires exact raw-wire preservation for large integers and high-precision decimals. The local SQL evidence shows the malformed writes were rejected atomically, but the valid value was changed from 123456789012345678901234567890.123456789 to 123456789012345680000000000000 before jsonb_set ran. This is silent data corruption in a supported database type, not a formatting difference or a test-only artifact. The practical fix is the targeted one already represented by the precise decoder and byte-preserving JSON wrapper: never route JSON numeric tokens through float64 before storage or jsonb_set updates.
Relevant code
server/functions/jsonb_set.go:90-101
func jsonbValueToInterface(ctx *sql.Context, value any) (any, error) {
unwrapped, err := sql.UnwrapAny(ctx, value)
if err != nil {
return nil, err
}
if bytesValue, ok := unwrapped.(types.JSONBytes); ok {
bytes, err := bytesValue.GetBytes(ctx)
if err != nil {
return nil, err
}
return pgtypes.DecodeJSONValue(bytes)
}server/types/json_document.go:245-261
func DecodeJSONValue(val []byte) (any, error) {
var decoded any
decoder := json.NewDecoder(bytes.NewReader(val))
decoder.UseNumber()
if err := decoder.Decode(&decoded); err != nil {
return nil, err
}
var trailing any
if err := decoder.Decode(&trailing); err != io.EOF {server/types/json_document.go:361-368
case float64:
// TODO: handle this as a proper numeric as float64 is not precise enough
d := new(apd.Decimal)
err = d.Scan(val)
if err != nil {
return nil, err
}
return JsonValueNumber(*d), nilserver/cast/jsonb.go:37-57
func jsonbGetInterface(ctx *sql.Context, val any) (any, error) {
switch v := val.(type) {
case sql.JSONWrapper:
return v.ToInterface(ctx)
case sql.StringWrapper:
s, err := v.Unwrap(ctx)
if err != nil {
return nil, err
}
result, err := pgtypes.DecodeJSONValue([]byte(s))Evidence Package
Copy prompt for an agent
Ito QA identified the following failure during automated PR testing. Please investigate and propose a fix.
**High severity — Large JSON numbers lose precision**
**What failed:** The database accepted the JSON document, but changed its large numeric value while saving it. Invalid trailing JSON was rejected correctly and did not create a partial row, so the failure is the silent precision loss on the valid document.
- **Impact:** Users who save very large or highly precise numbers in JSON data can have those values silently changed. Later reads and updates use the wrong value, which can cause incorrect records or calculations.
- **Steps to reproduce:**
1. Create a JSONB table with a value such as {"value":123456789012345678901234567890.123456789}.
2. Read the stored JSONB value and note that the number has changed to 123456789012345680000000000000.
3. Attempt an insert with two top-level JSON values and an update with trailing text; both should be rejected without changing the row.
4. Run jsonb_set on the persisted value to add another field and read the result back.
5. Compare the numeric field with the original input; it remains rounded even though the later update succeeds.
- **Stub / mock content:** No stubs, mocks, or bypasses were applied for this test in the recorded run.
- **Code analysis:** The runtime value is consistent with the PR's JSONB conversion design. The PR added jsonb_set in server/functions/jsonb_set.go and its jsonbValueToInterface helper reads JSONBytes with pgtypes.DecodeJSONValue at lines 90-101. The PR also changed server/cast/jsonb.go so string JSONB values use the same decoder. In the defective revision, the shared conversion ultimately exposed numbers through the native float64 path; server/types/json_document.go lines 361-368 explicitly document that path as not precise enough and convert the float to an apd.Decimal only after the precision has already been lost. A 30-digit integer therefore becomes 123456789012345680000000000000 before jsonb_set can copy or update the document. The current source contains the targeted remediation: DecodeJSONValue at server/types/json_document.go lines 245-261 uses Decoder.UseNumber, converts each json.Number to an apd.Decimal at lines 264-289, and server/functions/json.go lines 31-53 and 84-105 retain validated bytes in preciseJSONDocument. That remediation confirms the required fix is localized to preserving exact JSON numeric representation across input, storage, and jsonb_set conversion rather than changing path-update logic.
- **Why this is likely a bug:** PostgreSQL JSONB numeric values are expected to retain their exact textual value, and the test plan specifically requires exact raw-wire preservation for large integers and high-precision decimals. The local SQL evidence shows the malformed writes were rejected atomically, but the valid value was changed from 123456789012345678901234567890.123456789 to 123456789012345680000000000000 before jsonb_set ran. This is silent data corruption in a supported database type, not a formatting difference or a test-only artifact. The practical fix is the targeted one already represented by the precise decoder and byte-preserving JSON wrapper: never route JSON numeric tokens through float64 before storage or jsonb_set updates.
**Relevant code:**
`server/functions/jsonb_set.go:90-101`
~~~go
func jsonbValueToInterface(ctx *sql.Context, value any) (any, error) {
unwrapped, err := sql.UnwrapAny(ctx, value)
if err != nil {
return nil, err
}
if bytesValue, ok := unwrapped.(types.JSONBytes); ok {
bytes, err := bytesValue.GetBytes(ctx)
if err != nil {
return nil, err
}
return pgtypes.DecodeJSONValue(bytes)
}
~~~
`server/types/json_document.go:245-261`
~~~go
func DecodeJSONValue(val []byte) (any, error) {
var decoded any
decoder := json.NewDecoder(bytes.NewReader(val))
decoder.UseNumber()
if err := decoder.Decode(&decoded); err != nil {
return nil, err
}
var trailing any
if err := decoder.Decode(&trailing); err != io.EOF {
~~~
`server/types/json_document.go:361-368`
~~~go
case float64:
// TODO: handle this as a proper numeric as float64 is not precise enough
d := new(apd.Decimal)
err = d.Scan(val)
if err != nil {
return nil, err
}
return JsonValueNumber(*d), nil
~~~
`server/cast/jsonb.go:37-57`
~~~go
func jsonbGetInterface(ctx *sql.Context, val any) (any, error) {
switch v := val.(type) {
case sql.JSONWrapper:
return v.ToInterface(ctx)
case sql.StringWrapper:
s, err := v.Unwrap(ctx)
if err != nil {
return nil, err
}
result, err := pgtypes.DecodeJSONValue([]byte(s))
~~~

Implements PostgreSQL-compatible
jsonb_setsupport, including the optionalcreate_if_missingargument.Handles object and array paths, negative indexes, missing paths, strict NULL semantics, copy-on-write behavior, and PostgreSQL-compatible errors for invalid paths and scalar targets.
Part of #3099