Skip to content

feat(sqlserver): datetimeoffset is a timestamptz - #3138

Merged
mtoy-googly-moogly merged 3 commits into
malloydata:mainfrom
housejester:jde/sqlserver-datetimeoffset
Oct 14, 2026
Merged

mtoy-googly-moogly merged 3 commits into
malloydata:mainfrom
housejester:jde/sqlserver-datetimeoffset

Conversation

@housejester

@housejester housejester commented Oct 9, 2026 •

Copy link
Copy Markdown
Contributor

A datetimeoffset column is a Malloy timestamptz.

Dialect-only: the SQL Server dialect file, its README, the dialect spec, and one expectation in the connection spec. No other dialect's SQL changes.

The dialect supplies the two civil-time conversions for a datetimeoffset and nothing else; truncation, offsets, extraction and casts reach it through the shared code in dialect.ts, so they only ever operate on a datetime2.

  • sqlConvertToCivilTime reads a datetimeoffset's clock with AT TIME ZONE, at UTC or in the query zone, never at the offset the value was written in. A bare CAST would read that offset's clock.
  • sqlConvertFromCivilTime, when the destination is a timestamptz, casts a UTC clock to DATETIMEOFFSET (at +00:00), or labels a query-zone clock with AT TIME ZONE.

What the server does

Measured on 2017 and 2022. Three operations read a datetimeoffset's clock at its own offset: a cast to a date gives the local date, DATEPART gives the local hour, and a cast to datetime2 drops the offset and keeps the local clock. Comparison, DATEDIFF between two values, MAX and DATEADD already work on the instant.

AT TIME ZONE 'UTC' labels a datetime2 and moves a datetimeoffset, so one expression reads the UTC clock of either type. The same operator on a datetime2 with a named zone labels it, which is how a timestamptz literal is written.

The T-SQL

Malloy T-SQL
the column passes through; the driver returns the instant
hour(t), t.day, t + 1 month, no query zone on CAST((t AT TIME ZONE 'UTC') AS DATETIME2); a truncation or offset is cast back to DATETIMEOFFSET at +00:00
the same with timezone: 'America/Mexico_City' on CAST((t AT TIME ZONE 'UTC') AT TIME ZONE N'Central Standard Time (Mexico)' AS DATETIME2); a truncation or offset comes back with AT TIME ZONE in that zone
t::timestamp the UTC clock, CAST((t AT TIME ZONE 'UTC') AS DATETIME2), with or without a query zone: the cast keeps the instant
t::date the UTC or query-zone clock, cast to DATE
d::timestamptz midnight as a datetime2, cast to DATETIMEOFFSET, or labelled in the query zone
@2024-03-20 12:34:56[America/Chicago]::timestamptz (CAST('2024-03-20 12:34:56' AS DATETIME2) AT TIME ZONE N'Central Standard Time')
hours(a to b) DATEDIFF_BIG, as before
the SQL type for a timestamptz DATETIMEOFFSET

Testing

Against SQL Server 2022 in Docker (test/mssql/mssql_start.sh):

  • test/src/databases/sqlserver/sqlserver.spec.ts and packages/malloy-db-sqlserver pass, including five datetimeoffset tests: the instant, the clock at UTC without a query zone, comparison, a calendar offset, and measuring between two offsets.
  • test/src/databases/all/time.spec.ts, which SQL Server CI does not run: every timestamptz and query-zone test passes, 17 of them newly enabled by hasTimestamptz. Twenty other tests in the file fail, the same twenty as on main: the range-edge tests put a comparison in a CASE WHEN, which needs a boolean value SQL Server does not have, and one runs SELECT 1 with an unnamed column.

James Estes and others added 2 commits October 9, 2026 09:54
A datetimeoffset is an instant, so it is a Malloy timestamptz rather than
a native type. Its clock is read at UTC (SWITCHOFFSET), or in the query's
time zone (AT TIME ZONE), never at the offset the value was written in,
which is what CAST, DATEPART and the DATEDIFF anchor would otherwise read.
Truncation and offsets work on the UTC clock and return a datetimeoffset;
a timestamptz literal is written with AT TIME ZONE. Comparison, DATEDIFF
and MAX already compare instants.

The shared suite's timestamptz tests now run on SQL Server and pass on
2017 and 2022.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T2RKuL1PuzicjnDfKHz7uh
Signed-off-by: James Estes <james.estes@credibledata.com>
AT TIME ZONE 'UTC' labels a datetime2 and moves a datetimeoffset, so the
civil-time conversion is one expression for both types and the UTC case
is the only one that reads the type. The dialect spec keeps the facts the
shared suite cannot state.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01T2RKuL1PuzicjnDfKHz7uh
Signed-off-by: James Estes <james.estes@credibledata.com>
@mtoy-googly-moogly

mtoy-googly-moogly commented Oct 14, 2026 •

Copy link
Copy Markdown
Collaborator

Reading this revealed some weakness in how we provide time computation support in Dialect, so Claude and I have now re-done that.

PR #3145 changed how every dialect does time, and this PR should rebase onto it. It has landed, so please merge main.

Expect conflicts in sqlserver.ts. Time now goes through one path: convert the instant to civil time (a datetime2 for SQL Server), do the work there, then convert back. The comment headed Time in dialect.ts describes it. For this PR, that means:

  • Your datetimeoffset handling moves into the two conversions. The utcClock read goes in sqlConvertToCivilTime for a timestamptz, and the asInstant / AT TIME ZONE step goes in sqlConvertFromCivilTime when the destination is a timestamptz.
  • The timestamptz branches you added to sqlTruncate, sqlOffsetTime, sqlTimeExtractExpr and sqlCast go away. The shared code calls the conversions, and truncate, offset and extract only ever see a datetime2.
  • With hasTimestamptz = true, the new timestamptz tests in test/src/databases/all/time.spec.ts run on SQL Server. That file isn't part of SQL Server CI, so please run it locally. The timestamptz tests should pass.

…imeoffset

Signed-off-by: James Estes <james.estes@credibledata.com>

# Conflicts:
#	packages/malloy/src/dialect/sqlserver/sqlserver.ts
@housejester

Copy link
Copy Markdown
Contributor Author

Merged main. The datetimeoffset handling is now just the two conversions: sqlConvertToCivilTime reads the clock with AT TIME ZONE 'UTC' for a timestamptz, and sqlConvertFromCivilTime casts to DATETIMEOFFSET, or applies AT TIME ZONE in the query zone, when the destination is a timestamptz. The branches in sqlTruncate, sqlOffsetTime, sqlTimeExtractExpr and sqlCast are gone.

Against SQL Server 2017 and 2022 locally, every timestamptz and query-zone test in time.spec.ts passes. Across the full ci-sqlserver run nothing new fails, and two db-all tests now pass, because the test loader writes flights.dep_time and the alltypes timestamp as datetimeoffset.

The 20 other time.spec.ts failures aren't time: 19 are the harness comparing a boolean as a value, which SQL Server can't do, and one selects an unnamed column. I'd like to take that up separately; the boolean one is the same root cause as about 36 db-all failures, so I'd want your take on the shape before touching the harness. The description is updated to match.

@housejester
housejester marked this pull request as ready for review October 14, 2026 17:57
@mtoy-googly-moogly
mtoy-googly-moogly merged commit 07baf02 into malloydata:main Oct 14, 2026
34 of 35 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants