fix(orm): properly stringify typed-json values for postgres

[ymc9]

Summary by CodeRabbit

• Bug Fixes

  • Improved JSON field handling for custom type definitions in PostgreSQL, including proper stringification and array support.
  • Enhanced persistence and retrieval of complex JSON data structures.

• Tests

  • Added regression test validating JSON and custom-typed field operations with nested objects and arrays.

_{✏️ Tip: You can customize this high-level summary in your review settings.}

[coderabbitai]

Walkthrough

This pull request enhances PostgreSQL CRUD dialect to differentiate TypeDef field handling from regular JSON fields. It introduces an isTypeDef check in transformPrimitive to conditionally stringify TypeDef values and adds regression tests validating JSON and typed-JSON persistence with PostgreSQL.

Changes

Cohort / File(s)	Summary
PostgreSQL dialect enhancement packages/orm/src/client/crud/dialects/postgresql.ts	Added isTypeDef import from query-utils. Modified transformPrimitive control flow to introduce a new branch for TypeDef fields that stringifies input values (both scalars and arrays) if not already strings. Reordered array handling so TypeDef check precedes Array.isArray branch. Non-TypeDef array behavior unchanged. Updated comments documenting TypeDef, scalar Json, and Json[] handling.
TypeDef and JSON regression test tests/regression/test/issue-493.test.ts	Added new regression test file validating JSON and typed-JSON handling with PostgreSQL. Defines schema with inline custom types (InlineButton, BotButton) and models using JSON fields. Test verifies plain JSON persistence, JSON array persistence, and complex nested object creation with typed-JSON array fields and hydration.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

• Control flow logic in transformPrimitive: Verify the reordering of TypeDef and Array.isArray checks maintains backward compatibility and correctly handles edge cases
• Stringification logic: Confirm that stringifying TypeDef values for both scalar and array cases is appropriate and doesn't break downstream serialization
• Test coverage adequacy: Ensure the regression test sufficiently exercises TypeDef scenarios with nested custom types and array handling

Possibly related PRs

• merge dev to main (v3.0.0-beta.28) #478: Directly modifies PostgreSQL CRUD dialect's JSON handling and adds isTypeDef usage with typed-JSON tests
• merge dev to main #261: Modifies transformPrimitive logic in PostgreSQL dialect for primitive transformation
• fix: workaround node-pg's issue with passing array to non-array JSON fields #74: Alters JSON/typed-JSON serialization by stringifying inputs in the PostgreSQL dialect

Poem

🐰 Hops through the TypeDef fields with glee,
Stringify this, stringify that, you see!
PostgreSQL dances, tests validate true,
JSON and typed-JSON, both old and new!
A rabbit's delight, this logic so neat,
Making persistence complete! ✨

Pre-merge checks and finishing touches

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately describes the primary change: fixing proper stringification of typed-JSON values specifically for PostgreSQL, which aligns with the core modification in the postgresql.ts file.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✨ Finishing touches

• 📝 Generate docstrings

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch fix/issue-493

---

📜 Recent review details

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 0d74a02 and 1ceacd1.

📒 Files selected for processing (2)

• packages/orm/src/client/crud/dialects/postgresql.ts (2 hunks)
• tests/regression/test/issue-493.test.ts (1 hunks)

🧰 Additional context used

🧠 Learnings (5)

📓 Common learnings

Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 460
File: packages/orm/src/client/crud/dialects/postgresql.ts:109-112
Timestamp: 2025-12-02T03:11:40.158Z
Learning: In the PostgreSQL dialect for ZenStack ORM, DateTime fields are mapped to PostgreSQL's 'timestamp' type (without time zone), not 'timestamptz'. This means jsonb_build_object serializes them as ISO 8601 strings without timezone suffixes, requiring explicit 'Z' appending for UTC interpretation.

📚 Learning: 2025-11-26T01:55:04.540Z

Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/e2e/**/*.{ts,tsx} : E2E tests should validate real-world schema compatibility with established projects

Applied to files:

• tests/regression/test/issue-493.test.ts

📚 Learning: 2025-11-26T01:55:04.540Z

Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to tests/**/type*.{ts,tsx} : Ensure TypeScript inference and type coverage are validated through type coverage tests

Applied to files:

• tests/regression/test/issue-493.test.ts

📚 Learning: 2025-11-26T01:55:04.540Z

Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/orm/**/*.test.{ts,tsx} : ORM package tests should include comprehensive client API tests and policy tests

Applied to files:

• tests/regression/test/issue-493.test.ts

📚 Learning: 2025-10-21T16:09:31.218Z

Learnt from: ymc9
Repo: zenstackhq/zenstack-v3 PR: 319
File: packages/runtime/src/client/executor/zenstack-query-executor.ts:63-72
Timestamp: 2025-10-21T16:09:31.218Z
Learning: In ZenStack, TypeDefs can be inherited by models. When a TypeDef contains fields with `map` attributes, those mapped field names need to be processed by the QueryNameMapper since they become part of the inheriting model's schema. Therefore, when checking if a schema has mapped names (e.g., in `schemaHasMappedNames`), both `schema.models` and `schema.typeDefs` must be inspected for `@map` and `map` attributes.

Applied to files:

• packages/orm/src/client/crud/dialects/postgresql.ts

🧬 Code graph analysis (2)

tests/regression/test/issue-493.test.ts (1)

packages/testtools/src/client.ts (1)

• createTestClient (101-248)

packages/orm/src/client/crud/dialects/postgresql.ts (1)

packages/orm/src/client/query-utils.ts (1)

• isTypeDef (201-203)

⏰ 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). (4)

• GitHub Check: Agent
• GitHub Check: build-test (20.x, postgresql)
• GitHub Check: build-test (20.x, sqlite)
• GitHub Check: claude-review

🔇 Additional comments (6)

tests/regression/test/issue-493.test.ts (3)

1-5: LGTM!

The imports and test structure are appropriate for a regression test validating JSON/typed-JSON handling.

---

6-41: LGTM!

The schema definition comprehensively tests the distinction between plain JSON fields and typed-JSON fields (both scalar and array), including nested custom types. This provides excellent coverage for validating the PostgreSQL dialect changes.

---

43-93: LGTM!

The test assertions comprehensively validate:

• Plain JSON non-array and array persistence (lines 45-65)
• Complex typed-JSON structures with nested custom types (lines 67-92)

The test effectively ensures that TypeDef fields are properly stringified for PostgreSQL while maintaining correct behavior for plain JSON fields.

packages/orm/src/client/crud/dialects/postgresql.ts (3)

25-25: LGTM!

The isTypeDef import is correctly added and necessary for the new TypeDef handling logic.

---

56-59: LGTM!

The comment accurately explains the node-postgres quirk that necessitates JSON stringification for certain field types.

---

60-96: TypeDef stringification logic correctly implemented.

The fix properly handles typed-JSON fields for PostgreSQL:

• TypeDef check comes first (line 60), correctly treating TypeDef arrays as scalar Json that must be stringified
• The typeof value !== 'string' guard (line 64) correctly preserves pre-stringified default values
• Array handling (lines 69-76) correctly distinguishes between scalar Json fields (stringify) and Json[] fields (recursive transform)
• Logic references the node-postgres issue [CI] Bump version 3.0.0-beta.19 #374 workaround appropriately

SQLite handles TypeDef fields consistently by checking type in schema.typeDefs and stringifying uniformly. Both implementations are correct per their database requirements.

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[claude]

Claude finished @ymc9's task —— View job

---

PR Review: Properly stringify typed-json values for PostgreSQL

Reviewing the changes for code quality, potential issues, performance, and security concerns.

Todo List:

• Read and analyze the PostgreSQL dialect changes
• Review the regression test implementation
• Check for potential bugs and edge cases
• Assess performance implications
• Evaluate security considerations
• Provide comprehensive feedback

Code Review Summary

This PR addresses issue #493 by fixing how typed-JSON values are handled in PostgreSQL. The changes focus on proper JSON stringification for custom type definitions (isTypeDef).

Key Changes

packages/orm/src/client/crud/dialects/postgresql.ts:60-68

if (isTypeDef(this.schema, type)) {
    // type-def fields (regardless array or scalar) are stored as scalar `Json` and
    // their input values need to be stringified if not already (i.e., provided in
    // default values)
    if (typeof value !== 'string') {
        return JSON.stringify(value);
    } else {
        return value;
    }
}

Positive Aspects ✅

1. Clear Problem Definition: The fix targets a specific issue with PostgreSQL's node-pg driver incorrectly handling array values in JSON fields
2. Comprehensive Test Coverage: The regression test covers:
   • Plain JSON non-array fields
   • Plain JSON array fields
   • Typed-JSON with nested custom types
   • Both scalar and array typed-JSON scenarios
3. Good Documentation: Inline comments explain the rationale and reference the upstream issue
4. Defensive Programming: Checks if value is already stringified before re-stringifying

Potential Issues and Recommendations 🔍

1. Type Safety Concern

The change assumes all non-string values for typed-JSON should be stringified, but this could potentially stringify values that are already valid JSON (like null, boolean, number):

// Could be problematic if value is already a valid JSON primitive
if (typeof value !== 'string') {
    return JSON.stringify(value); // This would convert `42` to `"42"`
}

Recommendation: Consider more specific type checking:

if (isTypeDef(this.schema, type)) {
    if (typeof value === 'object' && value !== null) {
        return JSON.stringify(value);
    }
    return value;
}

2. Error Handling

No error handling for JSON.stringify() failures. While rare, complex objects with circular references could cause issues.

Recommendation: Add try-catch around stringification.

3. Performance Consideration

The function calls isTypeDef(this.schema, type) for every value transformation. For high-volume operations, this could be optimized by caching type definitions.

4. Test Coverage Gaps

The test doesn't cover edge cases:

• null values in typed-JSON fields
• Already stringified typed-JSON values
• Error scenarios (malformed JSON)
• Empty arrays/objects

Security Analysis 🔒

No security concerns identified. The changes only affect JSON serialization and don't introduce:

• SQL injection vulnerabilities
• Data exposure risks
• Authentication/authorization bypasses

Performance Impact 📊

Minimal performance impact. The changes add:

• One additional isTypeDef() check per value transformation
• JSON stringification only when necessary
• No significant algorithmic complexity changes

Code Quality 📝

Good overall code quality:

• Follows existing patterns in the codebase
• Maintains consistent error handling approach
• Uses clear, descriptive comments
• Proper import organization

Final Recommendation

✅ APPROVE with minor suggestions

The fix correctly addresses the PostgreSQL typed-JSON issue with a targeted solution. The comprehensive regression test ensures the fix works for the intended scenarios. While there are minor opportunities for improvement around type checking and error handling, the current implementation is solid and follows existing codebase patterns.

Consider the type safety enhancement mentioned above in a follow-up if needed, but this fix can be safely merged as-is.

---

[Copilot]

Pull request overview

This PR fixes a bug in the PostgreSQL dialect where typed-JSON values (custom types defined with type keyword and marked with @json) were not being properly stringified before storage. This addresses issue #493 where arrays of custom types were being incorrectly handled by the node-postgres driver.

Key changes:

• Added proper handling for typed-JSON fields in PostgreSQL's transformPrimitive method
• Ensures typed-JSON values are stringified before storage, consistent with SQLite's behavior
• Added comprehensive regression test covering both plain JSON and typed-JSON scenarios

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.

File	Description
tests/regression/test/issue-493.test.ts	Adds comprehensive regression test for PostgreSQL JSON and typed-JSON array handling
packages/orm/src/client/crud/dialects/postgresql.ts	Adds isTypeDef check and stringification logic for typed-JSON fields before array check

The implementation is correct and follows the established pattern from the SQLite dialect. The test coverage is thorough, testing plain JSON (both array and non-array) as well as typed-JSON with nested custom types. No issues were found during the review.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[ymc9]

fixes #493