fix(orm): format Date as HH:MM:SS for @db.Time / @db.Timetz columns (#2633)#2634
fix(orm): format Date as HH:MM:SS for @db.Time / @db.Timetz columns (#2633)#2634erwan-joly wants to merge 2 commits intozenstackhq:devfrom
Conversation
…enstackhq#2633) PostgresCrudDialect.transformInput converted every DateTime input to `Date.prototype.toISOString()`, which Postgres TIME / TIMETZ columns reject with `22007: invalid input syntax for type time: "2026-04-29T21:00:00.000Z"`. The dialect now inspects the field's `@db.*` attribute and formats time-of-day values as `HH:MM:SS.fff` (or `HH:MM:SS.fff+ZZ:ZZ` for TIMETZ); all other DateTime columns (including TIMESTAMP / TIMESTAMPTZ / DATE) keep the existing ISO behaviour because Postgres accepts ISO input for those types. The fix is the write-side counterpart to zenstackhq#2590, which fixed the read side of `@db.Time` (raw strings being silently returned instead of Date objects). Implementation: - `BaseCrudDialect.transformInput` gains an optional 4th parameter, `fieldDef?: FieldDef`, so dialects can read `@db.*` native-type attributes. The default behaviour is unchanged for callers that don't pass it (filter helpers in base-dialect.ts continue to work on simple type info). - `PostgresCrudDialect.transformInput` reads the field's `@db.*` attribute and routes `@db.Time` / `@db.Timetz` to a new `formatTimeOfDay` helper that emits the PG-acceptable format. - All write call sites in `base.ts` (create, createMany, update, upsert, automatic `updatedAt`) thread the field def through. - Filter call sites that compare against TIME columns also benefit from the same coercion via the array-filter path. Tested against `@db.Time` and `@db.Timetz` with single create and nested createMany. Fixes zenstackhq#2633
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe dialect layer's Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related issues
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Review rate limit: 7/8 reviews remaining, refill in 7 minutes and 30 seconds.Comment |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/regression/test/issue-2633.test.ts (1)
52-57: ⚡ Quick winStrengthen round-trip assertions with exact time checks.
toBeInstanceOf(Date)alone won’t catch time-shift regressions. Please assert the actual UTC time values too.Suggested assertion upgrade
const rows = await client.tradingHour.findMany({ orderBy: { id: 'asc' } }); expect(rows).toHaveLength(2); // The application reads `tw.open` / `tw.close` as Date objects. expect(rows[0].open).toBeInstanceOf(Date); expect(rows[0].close).toBeInstanceOf(Date); + expect(rows[0].open.toISOString()).toBe('1970-01-01T09:00:00.000Z'); + expect(rows[0].close.toISOString()).toBe('1970-01-01T16:00:00.000Z'); + expect(rows[1].open.toISOString()).toBe('1970-01-01T10:30:00.000Z'); + expect(rows[1].close.toISOString()).toBe('1970-01-01T17:30:00.000Z');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@tests/regression/test/issue-2633.test.ts` around lines 52 - 57, The test currently only checks types using toBeInstanceOf(Date) for rows[0].open and rows[0].close; strengthen it by asserting exact UTC times to catch time-shift regressions: after fetching rows with client.tradingHour.findMany, compare rows[0].open and rows[0].close (and any other relevant rows) against the expected UTC values (e.g., via toISOString() or constructing new Date(Date.UTC(...))) so the assertions verify the precise UTC timestamp in addition to being Date instances.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@tests/regression/test/issue-2633.test.ts`:
- Around line 52-57: The test currently only checks types using
toBeInstanceOf(Date) for rows[0].open and rows[0].close; strengthen it by
asserting exact UTC times to catch time-shift regressions: after fetching rows
with client.tradingHour.findMany, compare rows[0].open and rows[0].close (and
any other relevant rows) against the expected UTC values (e.g., via
toISOString() or constructing new Date(Date.UTC(...))) so the assertions verify
the precise UTC timestamp in addition to being Date instances.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: f2a7f3c9-ae3a-4067-b9bf-c85d4a0ad068
📒 Files selected for processing (4)
packages/orm/src/client/crud/dialects/base-dialect.tspackages/orm/src/client/crud/dialects/postgresql.tspackages/orm/src/client/crud/operations/base.tstests/regression/test/issue-2633.test.ts
| * full ISO timestamps). | ||
| */ | ||
| transformInput(value: unknown, _type: BuiltinType, _forArrayField: boolean) { | ||
| transformInput(value: unknown, _type: BuiltinType, _forArrayField: boolean, _fieldDef?: FieldDef) { |
There was a problem hiding this comment.
Very close to what we initially did and reverted on a previous PR — but I don't think there's a better way to solve this here. The shape-detect trick from #2590 worked for the read side because PG emits time-only values (09:30:00) and datetimes (2026-04-29 ...) with distinguishable prefixes. On the write side the input is always a Date object, which carries no shape clue about whether the destination column is time or timestamp, so we genuinely need the field metadata to format correctly. Open to alternatives if you see one.
Addresses CodeRabbit nitpick: toBeInstanceOf(Date) alone does not catch time-shift regressions. Add toISOString() comparisons against the expected anchored UTC values for both rows.
Fixes #2633.
Background
PostgresCrudDialect.transformInputconverted everyDateTimeinput toDate.prototype.toISOString(), which Postgres TIME / TIMETZ columns reject with:This is the write-side counterpart to #2590 (which fixed
@db.Timereads silently returning raw strings).Approach
The dialect now inspects the field's
@db.*attribute and routes time-of-day values to aformatTimeOfDayhelper that emits PG-acceptableHH:MM:SS.fff(orHH:MM:SS.fff+ZZ:ZZfor TIMETZ). All otherDateTimecolumns (TIMESTAMP / TIMESTAMPTZ / DATE) keep the existing ISO behaviour — Postgres accepts ISO input for those types natively.To reach the field's attributes from the dialect,
BaseCrudDialect.transformInputgains an optional 4th parameter,fieldDef?: FieldDef. The default behaviour is unchanged for callers that don't pass it.Files changed
packages/orm/src/client/crud/dialects/base-dialect.ts— adds optionalfieldDefparam totransformInput.packages/orm/src/client/crud/dialects/postgresql.ts— addsformatTimeOfDayhelper;transformInputreads@db.Time/@db.Timetzand routes through it.packages/orm/src/client/crud/operations/base.ts— threadsfieldDefthrough everytransformInputwrite call site (create, createMany, update, upsert, automaticupdatedAt).tests/regression/test/issue-2633.test.ts— regression covering single create and nestedcreateManyagainst both@db.Timeand@db.Timetz, plus a round-trip read.Repro
Notes
WHERE open = ?against TIME, that's the natural extension point.@db.Timeergonomics.Summary by CodeRabbit
New Features
Bug Fixes
Tests