fix(adapter-pg): properly serialize string values for JSON fields#29351
fix(adapter-pg): properly serialize string values for JSON fields#29351buyuan-dev wants to merge 3 commits intoprisma:mainfrom
Conversation
Fixes prisma#29330 When writing plain string values to JSON fields in PostgreSQL, the adapter was passing the string directly to node-postgres without JSON.stringify. PostgreSQL expects valid JSON, so plain strings would fail with invalid input syntax for type json. The fix ensures string values for JSON fields are properly serialized using JSON.stringify before being passed to the postgres driver.
|
|
WalkthroughPostgreSQL adapter now stringifies values for JSON scalar types so PostgreSQL receives valid JSON; new functional tests plus test matrix and Prisma schema files are added to validate JSON string-serialization behavior (SQLite excluded). Changes
🚥 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🧪 Generate unit tests (beta)
📝 Coding Plan
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: f9a71a27-97ad-495e-8db9-a4872bd321ed
📒 Files selected for processing (1)
packages/adapter-pg/src/conversion.ts
| @@ -1,4 +1,4 @@ | |||
| import { ArgType, type ColumnType, ColumnTypeEnum } from '@prisma/driver-adapter-utils' | |||
| import { ArgType, type ColumnType, ColumnTypeEnum } from '@prisma/driver-adapter-utils' | |||
There was a problem hiding this comment.
Remove BOM character from file.
There appears to be a Byte Order Mark (U+FEFF) character at the beginning of the file before import. This invisible character can cause issues with some build tools and linters.
🔧 Proposed fix
-import { ArgType, type ColumnType, ColumnTypeEnum } from '@prisma/driver-adapter-utils'
+import { ArgType, type ColumnType, ColumnTypeEnum } from '@prisma/driver-adapter-utils'📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| import { ArgType, type ColumnType, ColumnTypeEnum } from '@prisma/driver-adapter-utils' | |
| import { ArgType, type ColumnType, ColumnTypeEnum } from '@prisma/driver-adapter-utils' |
jacek-prisma
left a comment
There was a problem hiding this comment.
This needs a regression test based on the issue that is being addressed, you can have a look at packages/client/tests/functional/issues/29309-datetime-cursor for reference
|
@jacek-prisma ✅ Regression tests have been added, following the structure of #29309. Test coverage:
Test files:
Please let me know if the tests meet the requirements. Thanks! |
There was a problem hiding this comment.
Actionable comments posted: 2
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 6143bed5-1db9-4700-862a-0921decdd0f9
📒 Files selected for processing (3)
packages/client/tests/functional/issues/29330-json-string-serialization/_matrix.tspackages/client/tests/functional/issues/29330-json-string-serialization/prisma/_schema.tspackages/client/tests/functional/issues/29330-json-string-serialization/tests.ts
| // Test plain string value in JSON field | ||
| const document = await prisma.document.create({ | ||
| data: { | ||
| content: 'hello world', | ||
| metadata: { author: 'test' }, | ||
| }, | ||
| }) | ||
|
|
||
| expect(document.content).toBe('hello world') | ||
| expect(document.metadata).toEqual({ author: 'test' }) | ||
|
|
||
| // Test with findMany | ||
| const found = await prisma.document.findFirst({ |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
Remove stale/non-informative inline comments.
Line 21 says “findMany” but the code calls findFirst, and both comments here describe what the code does rather than why.
As per coding guidelines: “Avoid adding useless code comments that do not add new information. Only write inline comments explaining Why (context, background, GitHub issues, decisions), not What or How.”
packages/client/tests/functional/issues/29330-json-string-serialization/tests.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
♻️ Duplicate comments (1)
packages/client/tests/functional/issues/29330-json-string-serialization/tests.ts (1)
10-22:⚠️ Potential issue | 🟡 MinorRemove stale/non-informative inline comments.
These comments describe what the code does, and Line 21 is stale (
findFirstis used, notfindMany).As per coding guidelines: “Avoid adding useless code comments that do not add new information. Only write inline comments explaining Why (context, background, GitHub issues, decisions), not What or How.”Proposed cleanup
- // Test plain string value in JSON field const document = await prisma.document.create({ data: { content: 'hello world', metadata: { author: 'test' }, }, }) expect(document.content).toBe('hello world') expect(document.metadata).toEqual({ author: 'test' }) - // Test with findMany const found = await prisma.document.findFirst({ where: { id: document.id }, })
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: e8cc6b10-ae3c-4de6-997b-37ca7183802e
📒 Files selected for processing (1)
packages/client/tests/functional/issues/29330-json-string-serialization/tests.ts
|
? Automated check: Token is configured correctly. I can now post comments and push fixes automatically. |
|
Looks like |
Summary
Fixes a bug where plain string values could not be written to JSON fields in PostgreSQL.
Problem
When writing a plain string to a JSON field:
PostgreSQL would reject with:
This happened because the adapter was passing the string directly to node-postgres without JSON.stringify.
Fix
Ensure string values for JSON fields are properly serialized using JSON.stringify before being passed to the postgres driver.
Testing
Fixes #29330
Summary by CodeRabbit
Bug Fixes
Tests