sql(postgres): decode 'infinity'::date/timestamp to the Number ±Infinity - #35121
Conversation
Postgres represents unbounded dates/timestamps as the special values
'infinity' / '-infinity'. On every decode path (scalar text, scalar
binary, array text) these came back as `new Date(NaN)`:
- scalar text went through JS `Date.parse('infinity')` → NaN
- scalar binary put `i64::MAX / 1000 + epoch` (~9.224e15 ms) through
DateInstance::create, whose timeClip maps anything past ±8.64e15 to NaN
- array text already built `SQLDataCell::date(f64::INFINITY)`, but
DateInstance::create's timeClip flattened that to NaN too
An Invalid Date cannot distinguish +infinity from -infinity from a
parse failure, so the value is unrepresentable in user code.
node-postgres (pg-types via postgres-date) returns the Number
±Infinity here; match that.
- types/date.rs from_binary: recognise DT_NOEND/DT_NOBEGIN (i64::MAX/MIN)
and return ±f64::INFINITY
- types/date.rs parse_infinity: shared helper for the text spellings
- DataCell.rs: check parse_infinity before Date.parse on the scalar
text path and the unquoted-date[] array path
- SQLClient.cpp toJS: when the Date cell carries ±Infinity, return
jsDoubleNumber(±Infinity) instead of a timeClip'd DateInstance
|
Updated 7:35 AM PT - Jul 22nd, 2026
❌ @autofix-ci[bot], your commit 0d47616 has 3 failures in
🧪 To try this PR locally: bunx bun-pr 35121That installs a local version of the PR into your bun-35121 --bun |
|
Warning Review limit reachedYou’ve reached a temporary PR review limit under our Fair Usage Limits Policy. Next review available in: 54 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (6)
Comment |
…nfinity-date tests from_js is the sibling encoder of from_binary: now that the decoder produces ±Infinity, map it back to i64::MAX/MIN so a ParameterDescription that reports timestamp/timestamptz does not push it through the (ms - epoch) * 1000 arithmetic (which overflows i64). Also updates test/regression/issue/21311.test.ts, which inserted 'infinity'::date and asserted the old Invalid Date decode.
There was a problem hiding this comment.
The earlier from_js overflow is fixed and I found nothing new, but this is a user-visible behaviour change (infinity dates go from Invalid Date to the Number ±Infinity, and the PR picks node-postgres' semantics over postgres.js'), so a maintainer should sign off on the API choice.
What was reviewed:
from_jsnow short-circuits±Infinitytoi64::MAX/MINbefore the(ms - epoch) * 1000arithmetic, and the new bind test asserts the exact wire bytes.- The shared
SQLClient.cpptoJSDate arm is also used by MySQL — checked that MySQL's date cell only ever carries finite values or NaN, sostd::isinfnever fires there. - Traced the
date[]/timestamp[]/timestamptz[]unquoted-element paths inparse_array; each reaches aSQLDataCell::date(±INFINITY)for both signs.
Extended reasoning...
Overview
The PR changes how Bun's Postgres client decodes 'infinity' / '-infinity' for the date, timestamp, and timestamptz types (and their array forms). Previously all three decode paths — scalar text, scalar binary, and array text — collapsed to Invalid Date (a Date with NaN time), losing the sign. Now they return the JS Number ±Infinity. It touches src/sql_jsc/postgres/types/date.rs (from_binary, new parse_infinity, and from_js), two call sites in src/sql_jsc/postgres/DataCell.rs, one branch in src/jsc/bindings/SQLClient.cpp toJS, a new hermetic wire-protocol test file, and updates three existing tests that pinned the old Invalid Date behaviour.
My previous review flagged that the sibling encoder from_js would overflow i64 when round-tripping the newly-produced ±Infinity; f1f2afb addressed that by mapping ±Infinity → i64::MAX/MIN before the saturating cast, and added a bind-path test asserting the exact PG_INT64_MAX/MIN bytes in the Bind body. That thread is resolved.
Security risks
None identified. Input is server-provided Postgres wire data; the new checks are exact-match comparisons against i64::MAX/MIN and case-insensitive string equality against "infinity"/"-infinity", added before the existing parsing (no new allocation or arithmetic on untrusted lengths). The C++ change only branches on std::isinf of an already-decoded double.
Level of scrutiny
Medium-high. The code changes themselves are small, well-localised, and thoroughly tested with a scripted v3 backend covering every decode path plus the encode round-trip. However, this is a deliberate user-facing behaviour change: the result type for these two values changes from Date to number. The PR chooses node-postgres' semantics (which round-trip) over postgres.js' (which loses the sign), and while that reasoning is sound, picking which reference implementation to match is exactly the kind of API decision REVIEW.md's "API design" section says needs maintainer agreement. SQLClient.cpp's toJS is also shared with MySQL; I checked that MySQL never produces an infinite date cell (its zero-DATETIME path yields NaN, for which std::isinf is false), so no cross-driver leakage, but that's another reason a human should confirm.
Other factors
- The bug-hunting system found nothing on this revision.
- I traced the three array-text entry points in
parse_array:date_arraytakes the text-array arm (now guarded byparse_infinity), while unquotedinfinity/-infinityintimestamp_array/timestamptz_arrayreach the pre-existingb'I'|b'i'andb'-'number-parse arms that already emitSQLDataCell::date(±INFINITY)— the C++std::isinfbranch is what makes those finally observable. - The updated
sql.test.tsand21311.test.tsassertions correctly track the new behaviour without weakening what they protected (element identity and sign are now asserted more strongly, not less). from_js's new== f64::INFINITY/== f64::NEG_INFINITYcomparisons are IEEE-754-correct and leave NaN on its existing path, as the author noted.
|
CI on 0d47616: the SQL tests this PR touches ( |
Problem
Postgres represents unbounded dates and timestamps as the special values
'infinity'/'-infinity'. On every decode path Bun returned them asInvalid Date(aDatewhosegetTime()isNaN):That is data loss:
+infinity,-infinity, and a genuinely unparseable value all collapse to the sameNaN, so user code cannot tell whether a row means "no upper bound" or "no lower bound".Cause
Three independent paths, all ending at the same NaN:
datealways,timestamp/timestamptzon.simple()): routed through JSDate.parse('infinity')→NaN.timestamp/timestamptzon the extended protocol): Postgres sendsPG_INT64_MAX/PG_INT64_MIN(DT_NOEND/DT_NOBEGIN).from_binarydoesi64::MAX as f64 / 1000 + 946684800000≈ 9.224e15 ms, past JS Date's ±8.64e15 range, andDateInstance::create'stimeClipmaps it toNaN.{infinity,-infinity}::timestamp[]): the parser already producedSQLDataCell::date(f64::INFINITY), butSQLClient.cpphands that toDateInstance::create, whosetimeClipmaps every non-finite input toNaN. So the existing code that tried to preserve ±Infinity never took effect.Fix
Return the JS Number
±Infinityfor these values, matching node-postgres (pg-typesviapostgres-date, for oids 1082/1114/1184).postgres.jsdoes not special-case this and returnsInvalid Date, but that loses information; node-postgres' behaviour is the one that round-trips.types/date.rsfrom_binary: recognisei64::MAX/i64::MINand return±f64::INFINITY.types/date.rsparse_infinity: shared helper for theinfinity/-infinitytext spellings.types/date.rsfrom_js: map±Infinityback toi64::MAX/i64::MINso the encode/decode pair is symmetric and the(ms - epoch) * 1000arithmetic never overflows on the value the decoder now produces.DataCell.rs: checkparse_infinitybeforeDate.parseon the scalar text path and the unquoted-date[]array path (thetimestamp[]/timestamptz[]array path already produceddate(±INFINITY)).SQLClient.cpptoJS: when the Date cell carries ±Infinity, returnjsDoubleNumber(±Infinity)instead of atimeClip'dDateInstance.Finite dates are unchanged and remain
Dateinstances. MySQL shares thetoJSpath but never produces ±Infinity there (zero DATETIME isNaN, whichstd::isinfis false for).This is a behaviour change (the result type for these two values changes from
Datetonumber), but the previous value was unusable:.toISOString()throws on it,.getTime()isNaN, and the sign is gone.Verification
test/js/sql/postgres-infinity-date.test.tsdrives a scripted v3 backend and covers every path:date/timestamp/timestamptztimestamp/timestamptz(PG_INT64_MAX/PG_INT64_MINon the wire)date[]/timestamp[]/timestamptz[]±Infinitybound to atimestamp/timestamptzparameter writesPG_INT64_MAX/PG_INT64_MINin theBindbodyEach case asserts
±Infinityfor the special values and aDateinstance for an adjacent finite value. All ten fail on main; all pass here. The threesql.test.tsarray tests andtest/regression/issue/21311.test.tsthat pinned the oldInvalid Datebehaviour are updated.sql-postgres-datetime-roundtrip.test.ts,postgres-datarow-overrun.test.ts,postgres-binary-float-nan-box.test.ts, andwire-frames.test.tsare unchanged and pass.Follows on from the note at the bottom of #35112.