fix(act-pg): exclude the boundary event from created_after - #1603
Merged
Conversation
Postgres stores `created` at microsecond resolution but hands back a JS Date, which carries milliseconds. A row stamped ...976999 reads back as ...976, so `created > '...976'` compared the row against its own timestamp and returned it. InMemory and SQLite both exclude it, so Postgres was the odd one out, and the documented "scan since last time" pattern re-delivered the boundary event there and nowhere else. With a limit it returned the wrong event rather than one extra. Millisecond is the resolution the contract is expressed in, since that is all a Date carries, so strictly-after means "at least the next millisecond". Advancing the bound rather than truncating the column fixes rows already written at microsecond precision, needs no table rewrite on an events table, and keeps the comparison a plain range scan on (created, id). The store TCK gains the case that would have caught it: both bounds must exclude the event they are pinned to. Its existing window case is built as `ts ± 60_000`, so no bound ever landed on an event's own timestamp. Verified the new case fails on Postgres without this fix and passes on InMemory and SQLite unchanged. Closes #1595 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_014Vfpx1bDy3t57oHBynvNPF
|
🎉 This PR is included in version @rotorsoft/act-pg-v1.19.3 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
github-actions Bot
pushed a commit
that referenced
this pull request
Aug 30, 2026
# [@rotorsoft/act-tck-v1.36.15](https://github.com/Rotorsoft/act-root/compare/@rotorsoft/act-tck-v1.36.14...@rotorsoft/act-tck-v1.36.15) (2026-08-30) ### Bug Fixes * **act-pg:** exclude the boundary event from created_after ([#1603](#1603)) ([e8c6a18](e8c6a18)), closes [#1595](#1595) * **act:** revive dates on read instead of re-validating the payload ([#1601](#1601)) ([97ed5b9](97ed5b9)), closes [#1594](#1594)
|
🎉 This PR is included in version @rotorsoft/act-tck-v1.36.15 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #1595.
created_afteron Postgres returned the very event whose timestamp you passed in. InMemory and SQLite exclude it, so Postgres was the odd one out and the same query gave different answers depending on which adapter was underneath.What went wrong
Postgres stores
createdastimestamptz, which keeps microseconds. Thecreateda caller gets back is a JSDate, which carries milliseconds. So a row stamped...:52.976999reads back as...:52.976, and the query then asked forcreated > '...:52.976'— which that row satisfies, because.976999really is greater than.976.The event matched its own timestamp.
InMemory compares two JS
Dates and SQLite compares fixed-width millisecond ISO strings, so both exclude it. Two adapters to one.This is on a public surface —
Query.created_afterandAsOf.created_afterreach it throughapp.query,app.query_array,app.load({ asOf })andAct.restore. Two things follow:query: { created_after: lastScan }) re-delivers the boundary event on Postgres and nowhere elselimit, the off-by-one returns the wrong event rather than one extra:[1]on InMemory against[0]on PostgresThe fix
Millisecond is the resolution the contract is expressed in, because that is all a
Datecan carry. So strictly-after means "at least the next millisecond", and the bound is advanced by one before the comparison.Two alternatives were available and both are worse here:
Storing at millisecond precision (
timestamptz(3), or a truncating default) is the tidier root fix — stored and returned would then be the same value. But anALTER COLUMN ... TYPErewrites the whole table under an exclusive lock, and this is the events table. Doing that insideseed(), which runs at startup, is not something to trigger silently. A truncating default alone would fix only new rows and leave every existing one wrong.Truncating in the comparison (
date_trunc('milliseconds', created) > $1) fixes old and new rows, but it is a function on the column and would stop the query using the(created, id)index.Advancing the bound fixes rows already written at microsecond precision, needs no migration, and keeps the comparison a plain range scan.
The TCK case that would have caught it
store-tck.tsbuilds its window asts ± 60_000, so no bound ever landed on an event's own timestamp — which is the only place the two directions can disagree. It now asserts that both bounds exclude the event they are pinned to, withcreated_before(already strict everywhere) as the control that isolates a failure to thecreated_afterside.Verified the case is worth having: it fails on Postgres without this fix and passes unchanged on InMemory and SQLite.
Test plan
pnpm test— 3614 passingpnpm typecheck, biome cleanNot changed
created_beforewas already correct on every adapter and is untouched. The column type and its default are untouched, deliberately — see above.This is distinct from the already-recorded false positive about microsecond-versus-millisecond on a snapshot-derived
before, which was scoped totruncate/autoclose where the bound is derived asnow − keepand never comes from an event. Here the bound is an event's own timestamp, and the surface isquery.Stability charter impact
No public surface changed.
Query.created_afterkeeps its type and its documented meaning — this makes Postgres honor it.runStoreTckgains a case, which is additive.rfc-gate: exempt — the stability snapshot grew only because it captures source text, and what grew is the new TCK case plus the comment explaining the comparison. No public surface was added.
Found by
Debug wave 21's TCK blind-spot lens, reproduced independently in the main loop before filing.