BrighterCommand / BrighterCommand/Brighter
Change MSSQL Inbox and Outbox Timestamp Columns from `datetime` to `datetimeoffset(7)` in v11
- Dominant language
- C#
- Stars
- 2.5k
- Forks
- 296
- Avg merge
- 1d 11h
- Merged PRs (30d)
- 21
Description
### Problem Statement
The MSSQL Inbox and Outbox schemas currently use `DATETIME` for all timestamp columns. This loses timezone offset information, which causes issues when:
- Services run across multiple time zones.
- The outbox `Timestamp` and `Dispatched` columns are compared or analyzed externally without knowing the original offset.
- `DateTimeOffset` values passed into Brighter are silently converted to UTC `DateTime` via `MsSqlOutbox.CreateSqlParameter`, discarding the offset.
### Current Schema
**Outbox (`SqlOutboxBuilder`):**
```sql
[Timestamp] DATETIME NULL,
[Dispatched] DATETIME NULL,
```
**Inbox (`SqlInboxBuilder`):**
```sql
[Timestamp] [DATETIME] NULL,
```
### Current Code Behavior
`MsSqlOutbox.CreateSqlParameter` already accepts `DateTimeOffset` but immediately strips the offset:
```csharp
if (value is DateTimeOffset dateTimeOffset)
{
return new SqlParameter { ParameterName = parameterName, Value = dateTimeOffset.ToUniversalTime().DateTime };
}
```
This means the database never stores the original timezone — only the UTC `DateTime`.
### Proposed Change for v11
Update the MSSQL Inbox and Outbox schemas to use `DATETIMEOFFSET(7)` and stop converting `DateTimeOffset` to `DateTime`.
#### 1. Update Outbox DDL (`SqlOutboxBuilder`)
```sql
[Timestamp] DATETIMEOFFSET(7) NULL,
[Dispatched] DATETIMEOFFSET(7) NULL,
```
#### 2. Update Inbox DDL (`SqlInboxBuilder`)
```sql
[Timestamp] DATETIMEOFFSET(7) NULL,
```
#### 3. Update `MsSqlOutbox.CreateSqlParameter`
Remove the `DateTimeOffset` → `DateTime` conversion. Pass `DateTimeOffset` directly to `SqlParameter.Value`. `SqlParameter` already supports `DateTimeOffset` natively, and SQL Server will store it correctly in a `DATETIMEOFFSET(7)` column.
```csharp
// Remove this block entirely
if (value is DateTimeOffset dateTimeOffset)
{
return new SqlParameter { ParameterName = parameterName, Value = dateTimeOffset.ToUniversalTime().DateTime };
}
```
#### 4. Verify Queries
The existing queries (`MsSqlQueries`) use `ORDER BY [Timestamp]` and compare `@DispatchedSince` / `@DispatchedAt`. `DATETIMEOFFSET` supports ordering and comparison natively, so no SQL changes are required.
### Benefits
1. **Preserves timezone information** — the original offset is retained in the database.
2. **Better cross-timezone correctness** — services in different regions store timestamps with their local context.
3. **Aligns with .NET best practices** — `DateTimeOffset` is the preferred type for timestamps in modern .NET.
4. **No silent data loss** — stops the current behavior of discarding offset info at the parameter level.
### Migration Path for v11
Since this is a **schema-level breaking change**, it targets v11:
1. **v11.0.0**:
- Update DDL in `SqlOutboxBuilder` and `SqlInboxBuilder`.
- Remove `DateTimeOffset` → `DateTime` conversion in `MsSqlOutbox.CreateSqlParameter`.
2. **Migration guide**:
- Existing v10 tables using `DATETIME` will need an `ALTER TABLE` migration to convert to `DATETIMEOFFSET(7)`:
```sql
ALTER TABLE [Outbox] ALTER COLUMN [Timestamp] DATETIMEOFFSET(7) NULL;
ALTER TABLE [Outbox] ALTER COLUMN [Dispatched] DATETIMEOFFSET(7) NULL;
ALTER TABLE [Inbox] ALTER COLUMN [Timestamp] DATETIMEOFFSET(7) NULL;
```
- SQL Server handles `DATETIME` → `DATETIMEOFFSET` conversion automatically (treats existing values as local time offset +00:00).
Contributor guide
Research direction
Start with SqlOutboxBuilder, SqlInboxBuilder, and MsSqlOutbox.CreateSqlParameter to trace the timestamp schema and parameter handling. Review MsSqlQueries to confirm its ordering and comparisons remain valid, then update the v11 migration guidance; done means DATETIMEOFFSET(7) is used consistently and DateTimeOffset values retain their offset.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp, sql
- Domain
- backend, databases
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Active
- Clarity
- Clearly specified
- Newbie friendliness
- 70/100