fix(adonet): align PostgreSQL persistence script with its migration - #11247
Merged
ReubenBond merged 1 commit intoSep 15, 2026
Merged
ReubenBond merged 1 commit into
ReubenBond merged 1 commit into
Conversation
The PostgreSQL setup script should produce the same database as a deployment created on an older version that then had every migration applied, so that a fresh install never requires a second script (see dotnet#8896 and dotnet#8811). For PostgreSQL that already held everywhere except OrleansStorage: PostgreSQL-Persistence.sql declared `modifiedon timestamp without time zone`, while the 3.6.0 migration converts it to `timestamptz`, so fresh and migrated databases ended up with different column types. The migrations were added in f39c0b0 to mirror a9bf72a ("Support Npgsql 6.0"), but that change only touched the clustering and reminders scripts - persistence was never updated to match. The mismatch also left the migrated function writing shifted values: with `modifiedon` converted to `timestamptz`, the UPDATE branch of writetostorage still wrote `now() at time zone 'utc'`, a bare timestamp that PostgreSQL re-interprets in the session time zone, while the INSERT branch used a plain `now()`. On any non-UTC server the two branches disagreed. Move the setup script forward to `timestamptz` and use `now()` in both branches, and apply the same `now()` fix to the migration. The two writetostorage bodies are now identical apart from CREATE OR REPLACE, matching the clustering and reminders scripts. `OrleansStorage.modifiedon` is never read back by Orleans, so this affects operator-visible data only. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ReubenBond
force-pushed
the
fix/postgres-persistence-modifiedon-timestamptz
branch
from
September 13, 2026 14:58
e50f035 to
d7cde5b
Compare
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
A new migration is needed to repair databases that already applied the released 3.6.0 migration.
Get a fresh assessment by requesting another Copilot review.
Pull request overview
Aligns fresh PostgreSQL persistence installations with the migration schema and corrects timezone-shifted modifiedon writes.
Changes:
- Changes
modifiedontotimestamptz. - Uses
now()consistently for inserts and updates. - Updates the existing 3.6.0 migration function.
File summaries
| File | Description |
|---|---|
PostgreSQL-Persistence.sql |
Aligns fresh-install schema and timestamp writes. |
Migrations/PostgreSQL-Persistence-3.6.0.sql |
Corrects the migrated write function. |
Review details
- Files reviewed: 2/2 changed files
- Comments generated: 1
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
ReubenBond
approved these changes
Sep 15, 2026
This was referenced Oct 3, 2026
Closed
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.
Problem
A provider's ADO.NET setup script should produce the same database as a deployment that was created on an older version and then had every migration applied — the principle behind #8896 and #8811, where
Migrations/is an upgrade path and never a required second step for a fresh install.For PostgreSQL that holds everywhere except
OrleansStorage:PostgreSQL-Persistence.sqldeclaresmodifiedon timestamp without time zoneMigrations/PostgreSQL-Persistence-3.6.0.sqlconverts it totimestamptzSo a freshly created database and a migrated one end up with different column types.
How it happened: the PostgreSQL migrations were added in f39c0b0 (#7490) to mirror a9bf72a (#7402, "Support Npgsql 6.0"). #7402 only touched the clustering and reminders scripts — persistence was never updated to match, so the persistence migration made a change with no counterpart on the fresh-install path.
The mismatch also leaves migrated databases writing shifted timestamps. After migrating,
modifiedonistimestamptz, but the UPDATE branch ofwritetostoragestill writesnow() at time zone 'utc'— a baretimestampthat PostgreSQL re-interprets in the session time zone — while the INSERT branch uses a plainnow(). On any server whose session time zone isn't UTC, the two branches disagree by the UTC offset. The fresh-install script is internally consistent, so this affects migrated deployments only.Solution
Move the setup script forward to
timestamptzand usenow()in both branches, and apply the samenow()fix to the migration.Verification
Tested against PostgreSQL 16 in Docker with the session time zone set to
Europe/Rome(UTC+2), building four databases: fresh and migrated, before and after this change.Schema equality — today's setup script + the fixed migration vs. the new setup script:
information_schema.columnsfororleansstorage: identicalpg_get_functiondefforwritetostorage: identicalorleansqueryrows (key +md5(querytext)): identicalTimestamp drift, measured as stored instant minus true instant across an insert and then an update:
modifiedontimestamptimestamptztimestamptztimestamptz−7200s is exactly one CEST offset, reproducing the bug and confirming the fix.
Notes
OrleansStorage.modifiedonis never read back by Orleans — no query projects it and no C# accessor reads it (theModifiedOninRelationalOrleansQueriesbelongs to the streaming table). Impact is limited to operator-visible data; this is not a behavioural change for running silos.timestamptz, which is the shape a migrated database already has.(now() at time zone 'utc')inside theReadFromStorageKeyquery text is deliberately left alone — it is a constant "current UTC time" projected into the SELECT list, not themodifiedoncolumn.payloadxml/payloadjsoncolumns, which that PR removed from the setup script without a corresponding migration. Since those columns are nullable and unused, leaving them in place is harmless and dropping them would be destructive — but it does mean "setup script == old + migrations" is not yet exactly true for pre-Migrate AdoGrainStorage to the new grain serializer #8081 databases.🤖 Generated with Claude Code
Microsoft Reviewers: Open in CodeFlow