feat: add support for jsonpath column type in PostgreSQL
#11684
Conversation
commit: |
WalkthroughAdds PostgreSQL "jsonpath" as a recognized column type across documentation, driver/type definitions, entities, and functional tests to enable persistence and retrieval of jsonpath values. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant T as Test Suite
participant ORM as ORM Layer
participant D as PostgresDriver
participant DB as PostgreSQL
rect rgb(245,250,255)
note over T,DB: Persist jsonpath value
T->>ORM: save(entity with `jsonpath` field)
ORM->>D: map columns (includes "jsonpath")
D->>DB: INSERT ... (jsonpath column)
DB-->>D: OK
D-->>ORM: result
ORM-->>T: persisted
end
rect rgb(245,255,245)
note over T,DB: Retrieve and validate
T->>ORM: findOneBy(id)
ORM->>D: build SELECT
D->>DB: SELECT ... (jsonpath)
DB-->>D: row with jsonpath
D-->>ORM: map to string field
ORM-->>T: entity returned (assert checks)
end
note right of D: Driver & types now recognize `jsonpath`
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧬 Code graph analysis (1)test/functional/database-schema/column-types/postgres/jsonpath-postgres.ts (1)
⏰ 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). (20)
🔇 Additional comments (1)
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
🧹 Nitpick comments (1)
docs/docs/drivers/postgres.md (1)
63-63: Docs: fix multirange type name (“datemultirange” not “multidaterange”)Aligns with supported type names elsewhere in TypeORM and Postgres.
Apply this diff:
-`int`, `int2`, `int4`, `int8`, `smallint`, `integer`, `bigint`, `decimal`, `numeric`, `real`, `float`, `float4`, `float8`, `double precision`, `money`, `character varying`, `varchar`, `character`, `char`, `text`, `citext`, `hstore`, `bytea`, `bit`, `varbit`, `bit varying`, `timetz`, `timestamptz`, `timestamp`, `timestamp without time zone`, `timestamp with time zone`, `date`, `time`, `time without time zone`, `time with time zone`, `interval`, `bool`, `boolean`, `enum`, `point`, `line`, `lseg`, `box`, `path`, `polygon`, `circle`, `cidr`, `inet`, `macaddr`, `macaddr8`, `tsvector`, `tsquery`, `uuid`, `xml`, `json`, `jsonb`, `jsonpath`, `int4range`, `int8range`, `numrange`, `tsrange`, `tstzrange`, `daterange`, `int4multirange`, `int8multirange`, `nummultirange`, `tsmultirange`, `tstzmultirange`, `multidaterange`, `geometry`, `geography`, `cube`, `ltree` +`int`, `int2`, `int4`, `int8`, `smallint`, `integer`, `bigint`, `decimal`, `numeric`, `real`, `float`, `float4`, `float8`, `double precision`, `money`, `character varying`, `varchar`, `character`, `char`, `text`, `citext`, `hstore`, `bytea`, `bit`, `varbit`, `bit varying`, `timetz`, `timestamptz`, `timestamp`, `timestamp without time zone`, `timestamp with time zone`, `date`, `time`, `time without time zone`, `time with time zone`, `interval`, `bool`, `boolean`, `enum`, `point`, `line`, `lseg`, `box`, `path`, `polygon`, `circle`, `cidr`, `inet`, `macaddr`, `macaddr8`, `tsvector`, `tsquery`, `uuid`, `xml`, `json`, `jsonb`, `jsonpath`, `int4range`, `int8range`, `numrange`, `tsrange`, `tstzrange`, `daterange`, `int4multirange`, `int8multirange`, `nummultirange`, `tsmultirange`, `tstzmultirange`, `datemultirange`, `geometry`, `geography`, `cube`, `ltree`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
docs/docs/drivers/postgres.md(1 hunks)src/driver/postgres/PostgresDriver.ts(1 hunks)src/driver/types/ColumnTypes.ts(1 hunks)test/functional/database-schema/column-types/postgres/column-types-postgres.ts(3 hunks)test/functional/database-schema/column-types/postgres/entity/JsonPathExample.ts(1 hunks)test/functional/database-schema/column-types/postgres/entity/Post.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-27T20:50:10.364Z
Learnt from: alumni
PR: typeorm/typeorm#11581
File: docs/docs/drivers/postgres.md:23-23
Timestamp: 2025-07-27T20:50:10.364Z
Learning: The correct data source type for Aurora PostgreSQL in TypeORM is `aurora-postgres`, not `aurora-data-api-pg`. The `aurora-data-api-pg` driver was renamed to `aurora-postgres` according to the CHANGELOG.md. This is defined in the DatabaseType union type and AuroraPostgresConnectionOptions interface.
Applied to files:
src/driver/postgres/PostgresDriver.ts
🧬 Code graph analysis (3)
test/functional/database-schema/column-types/postgres/entity/Post.ts (1)
src/decorator/columns/Column.ts (1)
Column(134-220)
test/functional/database-schema/column-types/postgres/column-types-postgres.ts (1)
test/utils/test-utils.ts (2)
reloadTestingDatabases(504-509)closeTestingConnections(487-499)
test/functional/database-schema/column-types/postgres/entity/JsonPathExample.ts (3)
test/functional/database-schema/column-types/postgres/entity/Post.ts (1)
Entity(5-284)src/decorator/columns/PrimaryGeneratedColumn.ts (1)
PrimaryGeneratedColumn(55-119)src/decorator/columns/Column.ts (1)
Column(134-220)
🪛 GitHub Actions: Commit Validation
test/functional/database-schema/column-types/postgres/column-types-postgres.ts
[error] 656-656: AssertionError: expected '$"foo"' to equal '$foo' in jsonpath test: should insert and retrieve jsonpath values as strings for: $foo.
🔇 Additional comments (6)
src/driver/types/ColumnTypes.ts (1)
206-206: LGTM: added "jsonpath" to SimpleColumnTypeConsistent with Postgres support and the rest of the PR. No issues.
src/driver/postgres/PostgresDriver.ts (1)
178-178: LGTM: include "jsonpath" in supportedDataTypesMatches new SimpleColumnType. Serialization paths won’t JSON.stringify jsonpath (good).
If we later support default values on jsonpath columns, Postgres may canonicalize them. Watch for schema diff churn in
defaultEqual; we may need a jsonpath-aware comparison similar to json/jsonb.test/functional/database-schema/column-types/postgres/entity/Post.ts (1)
218-220: LGTM: add jsonpath column to PostType and decorator look correct.
test/functional/database-schema/column-types/postgres/entity/JsonPathExample.ts (1)
1-12: LGTM: jsonpath example entityMinimal and clear for round-trip tests.
test/functional/database-schema/column-types/postgres/column-types-postgres.ts (2)
2-2: LGTM: import JsonPathExampleRequired for the new jsonpath tests.
633-639: Test set looks goodCovers literals, accessors, filters, and arithmetic.
test/functional/database-schema/column-types/postgres/column-types-postgres.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
test/functional/database-schema/column-types/postgres/column-types-postgres.ts (1)
615-640: Consider additional edge cases for jsonpath testing.The test cases provide good coverage of common jsonpath expressions. Consider adding a few more edge cases to ensure robust handling:
- Escaped special characters in field names (e.g.,
$."field.with.dots")- Unicode characters in string literals
- Large numeric literals that might trigger precision issues
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
test/functional/database-schema/column-types/postgres/column-types-postgres.ts(3 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
test/functional/database-schema/column-types/postgres/column-types-postgres.ts (1)
test/utils/test-utils.ts (2)
reloadTestingDatabases(504-509)closeTestingConnections(487-499)
⏰ 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). (20)
- GitHub Check: tests-linux (20) / oracle
- GitHub Check: tests-linux (20) / mssql
- GitHub Check: tests-linux (20) / mongodb
- GitHub Check: tests-linux (20) / mysql_mariadb_latest
- GitHub Check: tests-linux (20) / mysql_mariadb
- GitHub Check: tests-linux (18) / mysql_mariadb
- GitHub Check: tests-linux (18) / mysql_mariadb_latest
- GitHub Check: tests-linux (18) / mongodb
- GitHub Check: tests-linux (18) / oracle
- GitHub Check: tests-linux (18) / sqljs
- GitHub Check: tests-linux (18) / sqlite
- GitHub Check: tests-linux (18) / mssql
- GitHub Check: tests-linux (18) / sap
- GitHub Check: tests-linux (18) / postgres (17)
- GitHub Check: tests-linux (18) / postgres (14)
- GitHub Check: tests-linux (18) / better-sqlite3
- GitHub Check: tests-windows / sqljs
- GitHub Check: tests-windows / sqlite
- GitHub Check: tests-windows / better-sqlite3
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
test/functional/database-schema/column-types/postgres/column-types-postgres.ts (5)
2-2: LGTM! Clean import addition for jsonpath testing.The import is properly positioned with other entity imports.
33-435: Well-structured refactor of the main test.Good restructuring - wrapping the main test in a
describeblock improves test organization and readability. The jsonpath integration (lines 99, 196, 382-383) is cleanly incorporated into the existing test flow.
436-564: LGTM! Clean transition to column options test.The test block properly validates column options with appropriate assertions.
566-609: LGTM! Type inference test properly structured.The test appropriately validates TypeORM's type inference when explicit types are not specified.
642-669: Fix inconsistency between canonical form check and actual canonical value.The test compares
loadedEntity.pathagainst the canonical form on line 658, but then on line 666 comparesloadedCanonicalEntity.pathagainst the originalpath. This appears backwards - the loaded entity from the database should match Postgres's canonical form, while the explicit cast should preserve that canonical form.Additionally, the past review comment correctly identified that Postgres normalizes jsonpath on storage (e.g.,
$foobecomes$"foo"), so the direct string comparison on line 658 is brittle.Apply this diff to fix the comparison logic:
- const loadedEntity = await repository.findOneByOrFail({ - id: createdEntity.id, - }) - - loadedEntity.path.should.be.equal(canonical ?? path) - - const loadedCanonicalEntity = await repository - .createQueryBuilder() - .select("path::text as path") - .where({ id: createdEntity.id }) - .getOneOrFail() - - loadedCanonicalEntity.path.should.be.equal(path) + const loadedEntity = await repository.findOneByOrFail({ + id: createdEntity.id, + }) + + // Get the canonical form from Postgres to account for normalization + const [{ canonical: actualCanonical }] = await connection.query( + "SELECT $1::jsonpath::text as canonical", + [path], + ) + + // The loaded entity should match Postgres's canonical form + loadedEntity.path.should.be.equal(actualCanonical) + + // Verify the expected canonical form matches if provided + if (canonical) { + actualCanonical.should.be.equal(canonical) + }
test/functional/database-schema/column-types/postgres/column-types-postgres.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
test/functional/database-schema/column-types/postgres/column-types-postgres.ts(3 hunks)test/functional/database-schema/column-types/postgres/jsonpath-postgres.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- test/functional/database-schema/column-types/postgres/column-types-postgres.ts
🧰 Additional context used
🧬 Code graph analysis (1)
test/functional/database-schema/column-types/postgres/jsonpath-postgres.ts (1)
test/utils/test-utils.ts (3)
createTestingConnections(388-482)reloadTestingDatabases(504-509)closeTestingConnections(487-499)
⏰ 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). (20)
- GitHub Check: tests-linux (20) / oracle
- GitHub Check: tests-linux (18) / oracle
- GitHub Check: tests-linux (20) / mssql
- GitHub Check: tests-linux (20) / cockroachdb
- GitHub Check: tests-linux (20) / mysql_mariadb
- GitHub Check: tests-linux (20) / mongodb
- GitHub Check: tests-linux (18) / mssql
- GitHub Check: tests-linux (20) / mysql_mariadb_latest
- GitHub Check: tests-linux (18) / sqljs
- GitHub Check: tests-linux (18) / better-sqlite3
- GitHub Check: tests-linux (18) / postgres (17)
- GitHub Check: tests-linux (18) / mongodb
- GitHub Check: tests-linux (18) / postgres (14)
- GitHub Check: tests-linux (18) / mysql_mariadb_latest
- GitHub Check: tests-linux (18) / mysql_mariadb
- GitHub Check: tests-linux (18) / sqlite
- GitHub Check: tests-windows / sqljs
- GitHub Check: tests-windows / sqlite
- GitHub Check: tests-windows / better-sqlite3
- GitHub Check: Analyze (javascript-typescript)
test/functional/database-schema/column-types/postgres/jsonpath-postgres.ts
Outdated
Show resolved
Hide resolved
|
@alumni any suggestion why my tests fail? I followed the |
|
@pkuczynski probably the query is not built correctly - you can try to run them locally with Later edit: nevermind, I see it just by looking at the code - |
Was a good hint, but I dropped this part at the end, as it wasn't different to the step before. Now build passes and it's ready for review :) |
gioboa
left a comment
There was a problem hiding this comment.
Thanks for your help @pkuczynski
Description of change
Re-creates #10457 based of current master due lack of the activity in the original PR.
Fixes #10457
Fixes #10456
Pull-Request Checklist
masterbranchFixes #00000Summary by CodeRabbit
New Features
Documentation
Tests