test(client): preserve BigInt precision in relationJoins queries#29009
Conversation
WalkthroughThe changes add type casting for JSON parsing in data mapping and introduce comprehensive functional tests for BigInt precision preservation with the relationJoins preview feature across multiple database providers. Changes
Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
- Add `safeJsonParse()` function that pre-processes JSON strings to quote large integers (16+ digits) before parsing, preventing precision loss - Update `data-mapper.ts`, `in-memory-processing.ts`, and `serialize-sql.ts` to use `safeJsonParse()` instead of `JSON.parse()` when parsing JSON strings that may contain BigInt values - Add comprehensive test suite for `safeJsonParse()` in `utils.test.ts` When using driver adapters with `relationJoins` preview feature, PostgreSQL returns related data as JSON-aggregated columns via lateral joins. Standard `JSON.parse()` converts integers exceeding `Number.MAX_SAFE_INTEGER` to JavaScript Number, silently corrupting the values. For example, `312590077454712834` becomes `312590077454712800`. This fix ensures BigInt foreign key values in nested relations are preserved by converting large integers to strings during JSON parsing. The existing data mapper then converts these strings to the correct BigInt type.
220b556 to
e71b882
Compare
Merging this PR will not alter performance
Comparing Footnotes
|
Skip the expensive regex replacement when no 16+ digit integers exist
in the JSON string. This handles the common case where JSON contains
no large integers that would lose precision.
The simple `/\d{16}/.test()` check is O(n) worst case but returns early
for most strings, making it effectively O(1) for typical JSON payloads
without BigInts.
Performance FixI've pushed a commit that addresses the 97.68% performance regression flagged by CodSpeed. Root CauseThe original implementation ran an expensive regex replacement on every JSON parse call, even when no large integers existed in the data. SolutionAdded a fast-path check that skips the regex replacement for the common case: const HAS_LARGE_INT = /\d{16}/
export function safeJsonParse(json: string): unknown {
// Fast path: skip expensive regex if no 16+ digit sequences exist
if (!HAS_LARGE_INT.test(json)) {
return JSON.parse(json)
}
// Slow path: only when large integers might be present
// ...
}Why This Works
All existing tests pass. Waiting for CI to confirm the performance improvement. |
There was a problem hiding this comment.
Pull request overview
This PR fixes a critical bug where BigInt values lose precision when using driver adapters with the relationJoins preview feature. When relation data is fetched via PostgreSQL lateral joins and returned as JSON-aggregated columns, standard JSON.parse() converts large integers exceeding Number.MAX_SAFE_INTEGER to JavaScript Numbers, causing silent data corruption.
Changes:
- Added
safeJsonParse()function that pre-processes JSON strings to quote large integers (16+ digits) before parsing - Replaced
JSON.parse()calls withsafeJsonParse()in data processing modules - Added comprehensive test coverage (15 tests) for the new functionality
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/client-engine-runtime/src/utils.ts | Implements safeJsonParse() with regex-based detection and conversion of 16+ digit integers to strings |
| packages/client-engine-runtime/src/utils.test.ts | Adds 15 unit tests covering various edge cases for safeJsonParse() |
| packages/client-engine-runtime/src/interpreter/serialize-sql.ts | Replaces JSON.parse() with safeJsonParse() when deserializing JSON column types |
| packages/client-engine-runtime/src/interpreter/in-memory-processing.ts | Replaces JSON.parse() with safeJsonParse() during in-memory record processing |
| packages/client-engine-runtime/src/interpreter/data-mapper.ts | Replaces JSON.parse() with safeJsonParse() when mapping array/object data |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/client-engine-runtime/src/utils.ts`:
- Line 146: The current LARGE_INT_PATTERN regex incorrectly matches digits
inside JSON string literals; update LARGE_INT_PATTERN to a two-alternative regex
that first matches string literals and second matches large integers outside
strings (e.g. use the suggested
/"(?:[^"\\]|\\.)*"|(:\s*|[,\[]\s*)(-?\d{16,})(?=\s*[,}\]])/g) and update the
replacement logic where LARGE_INT_PATTERN is used to return the full match
unchanged when capture group 2 is undefined (meaning a string literal was
matched) and only transform when group 2 contains the large integer; reference
LARGE_INT_PATTERN in utils.ts and ensure the replacer checks the second capture
group before modifying the match.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (1)
packages/client-engine-runtime/src/utils.ts
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for new file names (e.g.,query-utils.ts,filter-operators.test.ts)
Avoid creating barrel files (index.tsthat re-export from other modules). Import directly from the source file unless the./index.tsfile already exists
Avoid adding useless code comments that do not add new information. Do not write inline comments that describe what the code does; only write comments explaining why the code was written or relevant context/background
Write documentation comments for exported items using proper doc comment syntax
Files:
packages/client-engine-runtime/src/utils.ts
🧠 Learnings (1)
📚 Learning: 2026-01-05T16:15:18.374Z
Learnt from: jacek-prisma
Repo: prisma/prisma PR: 28992
File: packages/client/tests/functional/issues/28591-mapped-enums/test.ts:9-18
Timestamp: 2026-01-05T16:15:18.374Z
Learning: Prisma: After the TML-1663 fix, when using enums with map directives, only the mapped (lowercase) input values are accepted. For example, enum SuggestionStatus { PENDING map("pending") } will accept 'pending' but not 'PENDING'. This is a behavioral change from before. Update tests and usage accordingly to validate inputs against mapped values, and ensure any input normalization or validation relies on the mapped string rather than the enum name.
Applied to files:
packages/client-engine-runtime/src/utils.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: Agent
- GitHub Check: CodeQL analysis (javascript-typescript)
🔇 Additional comments (1)
packages/client-engine-runtime/src/utils.ts (1)
168-181: Good implementation with effective fast-path optimization.The fast-path check using a simple
/\d{16}/test before applying the expensive global regex replacement is a smart optimization that avoids overhead for typical JSON payloads without large integers. The documentation clearly explains the problem being solved (precision loss with relationJoins) and the approach taken.Assuming the regex edge case noted above is addressed or deemed acceptable for the known input sources, the implementation is sound.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Address review feedback: - Fix regex to skip string literals by adding capturing group - Update comment wording for consistency (16 or more digits) - Use safeJsonParse in test for round-trip consistency - Add tests for large integers inside string values
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/client-engine-runtime/src/utils.test.ts`:
- Around line 3-105: Add a regression test in
packages/client-engine-runtime/src/utils.test.ts to verify safeJsonParse
correctly handles top-level scalar JSON numbers that are large (i.e., a JSON
string that is just a large integer like "312590077454712834" or negative
variant) and returns the value as a string rather than a corrupted Number;
locate the existing safeJsonParse test suite (describe('safeJsonParse')) and add
a new test case that calls safeJsonParse with a top-level numeric JSON string
and asserts the returned value equals the stringified integer to ensure scalar
handling isn't regressed.
In `@packages/client-engine-runtime/src/utils.ts`:
- Around line 166-183: safeJsonParse currently only rewrites large ints when
they follow `:`, `,`, or `[` (LARGE_INT_PATTERN), so a JSON payload that's just
a top-level large integer still parses as a Number; update safeJsonParse to
detect full-string large-integer payloads and quote them before JSON.parse (or
extend LARGE_INT_PATTERN to also match start/end-of-string numeric-only input).
Concretely, in safeJsonParse check json.trim() against a full-number regex (e.g.
optional sign + 16+ digits) or add anchors to LARGE_INT_PATTERN, and when
matched wrap the entire numeric string in quotes (or transform to a quoted
value) so JSON.parse returns a string and preserves precision; keep existing
logic for other cases using HAS_LARGE_INT and LARGE_INT_PATTERN unchanged.
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
packages/client-engine-runtime/src/utils.test.tspackages/client-engine-runtime/src/utils.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for new file names (e.g.,query-utils.ts,filter-operators.test.ts)
Avoid creating barrel files (index.tsthat re-export from other modules). Import directly from the source file unless the./index.tsfile already exists
Avoid adding useless code comments that do not add new information. Do not write inline comments that describe what the code does; only write comments explaining why the code was written or relevant context/background
Write documentation comments for exported items using proper doc comment syntax
Files:
packages/client-engine-runtime/src/utils.test.tspackages/client-engine-runtime/src/utils.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Test files named
*.test.tsare excluded from build output via esbuild config; place tests alongside source files
Files:
packages/client-engine-runtime/src/utils.test.ts
🧠 Learnings (1)
📚 Learning: 2026-01-05T16:15:18.374Z
Learnt from: jacek-prisma
Repo: prisma/prisma PR: 28992
File: packages/client/tests/functional/issues/28591-mapped-enums/test.ts:9-18
Timestamp: 2026-01-05T16:15:18.374Z
Learning: Prisma: After the TML-1663 fix, when using enums with map directives, only the mapped (lowercase) input values are accepted. For example, enum SuggestionStatus { PENDING map("pending") } will accept 'pending' but not 'PENDING'. This is a behavioral change from before. Update tests and usage accordingly to validate inputs against mapped values, and ensure any input normalization or validation relies on the mapped string rather than the enum name.
Applied to files:
packages/client-engine-runtime/src/utils.test.tspackages/client-engine-runtime/src/utils.ts
🧬 Code graph analysis (1)
packages/client-engine-runtime/src/utils.test.ts (1)
packages/client-engine-runtime/src/utils.ts (2)
safeJsonParse(166-184)safeJsonStringify(119-128)
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
Address review feedback: - Add TOP_LEVEL_LARGE_INT pattern for bare scalar integers - Return string directly when JSON is just a large integer - Add regression test for top-level large integers This handles edge cases like JSON columns storing numeric scalars.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/client-engine-runtime/src/utils.test.ts`:
- Around line 131-134: The test assigns result from
safeJsonParse(safeJsonStringify(obj)) and then indexes into result.user which
causes a TypeScript error because safeJsonParse returns unknown; update the test
to assert the expected shape by casting the round-trip value (the result
variable) to the appropriate type (or to any if a quick fix) so subsequent
property access (result.user.id, result.user.files[0].fileId) type-checks;
locate the usage of safeJsonParse and safeJsonStringify in the test and add the
type assertion to the result declaration.
♻️ Duplicate comments (1)
packages/client-engine-runtime/src/utils.ts (1)
179-183: Handle leading/trailing whitespace in top-level integer check.
JSON.parsetolerates leading/trailing whitespace (e.g.,JSON.parse(' 123 ')returns123), butTOP_LEVEL_LARGE_INTrequires an exact match. Input like' 312590077454712834 'would bypass the top-level check, skip the slow path'sLARGE_INT_PATTERN(which requires a prefix character), and fall through toJSON.parse, losing precision.Proposed fix
// Handle top-level large integers (JSON can be a bare scalar value). // The main regex only matches integers after :, ,, or [ so this case needs special handling. - if (TOP_LEVEL_LARGE_INT.test(json)) { - return json + const trimmed = json.trim() + if (TOP_LEVEL_LARGE_INT.test(trimmed)) { + return trimmed }
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
packages/client-engine-runtime/src/utils.test.tspackages/client-engine-runtime/src/utils.ts
🧰 Additional context used
📓 Path-based instructions (2)
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for new file names (e.g.,query-utils.ts,filter-operators.test.ts)
Avoid creating barrel files (index.tsthat re-export from other modules). Import directly from the source file unless the./index.tsfile already exists
Avoid adding useless code comments that do not add new information. Do not write inline comments that describe what the code does; only write comments explaining why the code was written or relevant context/background
Write documentation comments for exported items using proper doc comment syntax
Files:
packages/client-engine-runtime/src/utils.tspackages/client-engine-runtime/src/utils.test.ts
**/*.test.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Test files named
*.test.tsare excluded from build output via esbuild config; place tests alongside source files
Files:
packages/client-engine-runtime/src/utils.test.ts
🧠 Learnings (1)
📚 Learning: 2026-01-05T16:15:18.374Z
Learnt from: jacek-prisma
Repo: prisma/prisma PR: 28992
File: packages/client/tests/functional/issues/28591-mapped-enums/test.ts:9-18
Timestamp: 2026-01-05T16:15:18.374Z
Learning: Prisma: After the TML-1663 fix, when using enums with map directives, only the mapped (lowercase) input values are accepted. For example, enum SuggestionStatus { PENDING map("pending") } will accept 'pending' but not 'PENDING'. This is a behavioral change from before. Update tests and usage accordingly to validate inputs against mapped values, and ensure any input normalization or validation relies on the mapped string rather than the enum name.
Applied to files:
packages/client-engine-runtime/src/utils.tspackages/client-engine-runtime/src/utils.test.ts
🧬 Code graph analysis (1)
packages/client-engine-runtime/src/utils.test.ts (1)
packages/client-engine-runtime/src/utils.ts (2)
safeJsonParse(172-196)safeJsonStringify(119-128)
🔇 Additional comments (3)
packages/client-engine-runtime/src/utils.test.ts (1)
1-111: Comprehensive test coverage for safeJsonParse.The tests thoroughly cover the key scenarios: regular JSON, arrays, nested objects, top-level large integers (both positive and negative), boundary cases (15-digit safe vs 16-digit conversion), string literal preservation, and escaped quotes. The precision loss demonstration test (lines 97-110) effectively documents the problem being solved.
packages/client-engine-runtime/src/utils.ts (2)
130-156: Well-documented regex patterns with clear capturing group explanations.The documentation thoroughly explains the pattern structure, capturing groups, and purpose. The two-alternative approach correctly handles string literals to prevent JSON corruption.
185-196: Replacement logic correctly preserves string literals.The two-alternative regex approach with the
stringLiteral !== undefinedcheck ensures that large integers inside JSON string values are preserved unchanged, while numeric values are quoted to prevent precision loss. This addresses the JSON corruption issue identified in previous reviews.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 5 out of 5 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Thank you for the pull request. I'm not sure about this approach to be honest. I would prefer if we added the casts to the SQL queries so that those values are strings in the JSON in the first place. |
…ation test The test case was duplicating coverage already provided by "converts large integers (18 digits) to strings to preserve precision".
i'm not happy with the approach as well - but had to highlight the problem as encountered in actual app |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/client-engine-runtime/src/utils.test.ts`:
- Around line 29-34: Update the test comment in the 'converts 16+ digit integers
to strings (may exceed MAX_SAFE_INTEGER)' test to accurately state that the
chosen value 9007199254740991 equals Number.MAX_SAFE_INTEGER (not exceeds it)
and that the implementation conservatively converts all 16‑digit or longer
integers to strings; locate the comment in the test surrounding the
safeJsonParse call and replace the wording to reflect that behavior and the
exact relation to MAX_SAFE_INTEGER.
Please take a look at prisma/prisma-engines#5745, if i got it correctly |
|
Replying to review threads: Thread 1 (aqrln - "this looks superfluous"): Removed the redundant test in commit 1cfaff0. Thread 2 (CodeRabbit - comment accuracy): This test was for Threads 3-5 (Copilot - missing safeJsonParse): The PR description was outdated. The fix is now implemented at the SQL level in prisma-engines (prisma/prisma-engines#5745), where BigInt values are cast to The |
## Problem
When using `relationJoins` with BigInt fields in Prisma 7, JavaScript's
`JSON.parse` loses precision for integers larger than
`Number.MAX_SAFE_INTEGER` (2^53 - 1).
This happens because PostgreSQL's `JSONB_BUILD_OBJECT` returns BigInt
values as JSON numbers, which JavaScript cannot represent precisely.
Example:
```
// Original BigInt ID: 312590077454712834
// After JSON.parse: 312590077454712830 (corrupted!)
```
## Solution
Cast BigInt columns to `::text` inside `JSONB_BUILD_OBJECT` calls,
similar to how MONEY is already cast to `::numeric`.
```sql
-- Before
JSONB_BUILD_OBJECT('id', "id")
-- After
JSONB_BUILD_OBJECT('id', "id"::text)
```
This ensures BigInt values are returned as JSON strings, preserving full
precision when parsed in JavaScript.
## Changes
- Added BigInt → `::text` cast in `visit_json_build_obj_expr`
(quaint/src/visitor/postgres.rs)
- Added test case for BigInt casting
## Related
- prisma/prisma#29009 (TypeScript-level workaround in prisma fork)
## Test plan
- [x] Added unit test for BigInt casting in `test_json_build_object`
- [ ] CI tests pass
---------
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
@polaz tests now show that the bug still exists in MySQL and, more surprisingly, in CockroachDB. MySQL needs a similar fix in its implementation of Would you be interested in fixing those two as well? If not, then let's exclude them from this test for now and create the corresponding issues. |
|
I addressed the MySQL/CockroachDB precision issue in prisma-engines: prisma/prisma-engines#5752 (casts BigInt/INT8 to text in JSON builders, plus tests). Once Wasm updates land, the functional tests should pass for those providers as well. |
`Value` is `unknown` and `JSON.parse` returns `any`, so the cast adds nothing.
## Problem
When using `relationJoins` with BigInt fields in Prisma 7, JavaScript's
`JSON.parse` loses precision for integers larger than
`Number.MAX_SAFE_INTEGER` (2^53 - 1).
This happens because PostgreSQL's `JSONB_BUILD_OBJECT` returns BigInt
values as JSON numbers, which JavaScript cannot represent precisely.
Example:
```
// Original BigInt ID: 312590077454712834
// After JSON.parse: 312590077454712830 (corrupted!)
```
## Solution
Cast BigInt columns to `::text` inside `JSONB_BUILD_OBJECT` calls,
similar to how MONEY is already cast to `::numeric`.
```sql
-- Before
JSONB_BUILD_OBJECT('id', "id")
-- After
JSONB_BUILD_OBJECT('id', "id"::text)
```
This ensures BigInt values are returned as JSON strings, preserving full
precision when parsed in JavaScript.
## Changes
- Added BigInt → `::text` cast in `visit_json_build_obj_expr`
(quaint/src/visitor/postgres.rs)
- Added test case for BigInt casting
## Related
- prisma/prisma#29009 (TypeScript-level workaround in prisma fork)
## Test plan
- [x] Added unit test for BigInt casting in `test_json_build_object`
- [ ] CI tests pass
---------
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Summary
This PR adds a regression test for a critical bug where BigInt values lose precision when using driver adapters (like
@prisma/adapter-pg) with therelationJoinspreview feature enabled.Fixes #29010
The Problem
When
previewFeatures = ["relationJoins"]is enabled, Prisma uses lateral joins to fetch related data. The related records are returned as JSON-aggregated columns. When parsing this JSON data, standardJSON.parse()converts large integers (exceedingNumber.MAX_SAFE_INTEGER) to JavaScript Numbers, silently corrupting the values.Example corruption:
312590077454712834312590077454712800(precision lost!)The Fix
The fix is implemented at the SQL level in
prisma-engines:JSONB_BUILD_OBJECT(fix(quaint): cast BigInt to text in JSON aggregation prisma-engines#5745)BigInt columns are cast to string before JSON aggregation, so values arrive as strings and are correctly parsed without precision loss.
Changes in this PR:
packages/client/tests/functional/issues/29010-bigint-precision-relation-joins/as Valuecast) indata-mapper.tsReproduction
Schema
Test Code
Test Plan
Breaking Changes
None. This is a bugfix that preserves intended behavior.
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.