BrighterCommand / BrighterCommand/Brighter

Change MSSQL Inbox and Outbox Timestamp Columns from `datetime` to `datetimeoffset(7)` in v11

Open
#4,220 3 comments 0 reactions 0 assignees View on GitHub
.NET 0 - Backlog Breaking Change 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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.