fix(adapter-ppg): handle null values in type parsers for nullable columns#29192
Conversation
The PPG client passes null values through to type parsers for nullable columns, unlike the standard pg client. This causes TypeError when calling .replace() or .slice() on null for nullable DateTime, TimeTZ, Money, and Bytea columns. Fixes prisma#29190
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
WalkthroughNormalization utilities and parser mappings in 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)
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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/adapter-ppg/src/conversion.ts (1)
313-315:⚠️ Potential issue | 🟠 Major
normalize_arrayandnormalizeByteaArraylack null guards that other normalizers have.If a nullable array column contains SQL
NULL, the parsers will receivenulland throw. Other scalar normalizers guard against this (e.g.,normalize_bool,normalize_timestamp,convertBytes). Bothnormalize_arrayandnormalizeByteaArrayshould returnnullearly if the input is null, consistent with the established pattern.
The null guards were correct but the parameter types still declared string, making the null checks appear as dead code under strictNullChecks. This widens the types to string | null so the contract is honest and the compiler can enforce the guards. Also fixes normalize_bool for consistency.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/adapter-ppg/src/conversion.ts (1)
313-315:⚠️ Potential issue | 🟡 Minor
normalize_array's element normalizer type is incompatible with nullable normalizers understrictFunctionTypes.
normalize_arrayexpects(x: string) => string, butnormalize_timestamp,normalize_timestamptz,normalize_money, andnormalize_boolreturnstring | null. Withstrict: trueenabled in the TypeScript configuration, this violates return type covariance and causes a type error.Widen the signature to accept and return nullable values:
Proposed fix
-function normalize_array(element_normalizer: (x: string) => string): (str: string) => string[] { - return (str) => parseArray(str, element_normalizer) +function normalize_array(element_normalizer: (x: string) => string | null): (str: string) => (string | null)[] { + return (str) => parseArray(str, element_normalizer) }
…le array columns Addresses CodeRabbit review feedback: widen normalize_array element_normalizer signature to accept string | null returns, add null input handling to both normalize_array and normalizeByteaArray to prevent TypeError on nullable array columns with SQL NULL values. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
@CLAassistant check |
|
Thank you for the contribution! |
Merging this PR will not alter performance
Comparing Footnotes
|
Add unit tests covering null handling in type parsers (timestamp, timestamptz, timez, money, bool, bytes) and normal value parsing. Include adapter-ppg in the driver-adapter-unit-tests CI job. Also add explicit return type annotation to normalizeByteaArray.
|
Hi Prisma team, just a gentle bump on this PR. All CodeRabbit suggestions have been addressed — null guards added for all affected parser functions including arrays, type signatures widened, and return type annotations added. The fix prevents TypeError crashes for nullable DateTime, TimeTZ, Money, and Bytea columns in the PPG adapter. Would appreciate a human review when someone has a chance! |
jacek-prisma
left a comment
There was a problem hiding this comment.
There's one valid CodeRabbit comment and a type error: src/conversion.test.ts(112,14): error TS18047: 'result' is possibly 'null'
If these get addressed, I would be happy to merge this.
…ble-datetime-adapter-ppg
…umns (#29192) Fixes #29190 The PPG client passes null values through to type parsers for nullable columns, unlike the standard pg client. This causes a `TypeError: Cannot read properties of null (reading 'replace')` when querying models with nullable `DateTime?`, `TimeTZ`, `Money`, or `Bytea` columns that have null values in the database. Added null guards to the following parser functions in `packages/adapter-ppg/src/conversion.ts`: - `normalize_timestamp` - `normalize_timestamptz` - `normalize_timez` - `normalize_money` - `convertBytes` The other parsers (`normalize_bool`, `normalize_date`, `normalize_time`, `normalize_numeric`, `toJson`, `normalize_xml`, `normalizeBit`) either already handle null or simply pass through the value without calling methods on it, so they don't crash. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit * **Bug Fixes** * Improved null-tolerant parsing across booleans, arrays, timestamps, timestamptz, time-with-zone, money, and binary values so empty or missing inputs are preserved instead of erroring. * Public parsers now consistently propagate null-safe behavior. * **Tests** * Added comprehensive tests covering null handling and normalization for timestamps, timestamptz, timetz, money, bool, and binary parsing. * **Chores** * Expanded CI to run unit tests for the adapter package. <!-- end of auto-generated comment: release notes by coderabbit.ai --> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Jacek Malec <malec@prisma.io>
Fixes #29190
The PPG client passes null values through to type parsers for nullable columns, unlike the standard pg client. This causes a
TypeError: Cannot read properties of null (reading 'replace')when querying models with nullableDateTime?,TimeTZ,Money, orByteacolumns that have null values in the database.Added null guards to the following parser functions in
packages/adapter-ppg/src/conversion.ts:normalize_timestampnormalize_timestamptznormalize_timeznormalize_moneyconvertBytesThe other parsers (
normalize_bool,normalize_date,normalize_time,normalize_numeric,toJson,normalize_xml,normalizeBit) either already handle null or simply pass through the value without calling methods on it, so they don't crash.Summary by CodeRabbit
Bug Fixes
Tests
Chores