Repository navigation
Fix/sqlite time utc - #32
Merged
Merged
Conversation
modernc.org/sqlite binds a time.Time with t.String() unless the DSN sets _time_format. That appends " m=+86400.02..." whenever the value still carries a monotonic reading, which any time.Now().Add(...) does, and it prints a zone with no name as "+0200 +0200". parseTimeString accepted neither. It fails inside Scan, so one bad row failed the whole query. Range filters broke too: WHERE at < ? compares TEXT, and a non-UTC argument matched the wrong rows. The sqlite driver now converts time.Time, *time.Time and valid sql.NullTime args to UTC in utcArgs, at the three places args reach database/sql: SqliteDB outside a transaction, sqliteTx and sqliteStmt. That covers inserts, bulk inserts, updates, WHERE args and raw queries without touching the builders. The caller's slice is copied and left alone, because RawQuery reuses its args and hooks see qc.RawArgs. parseTimeString strips the monotonic suffix and has a layout for the doubled offset, so rows already written that way scan again. That part lives in scan, so turso gets it as well. sql.NullTime fields couldn't read sqlite at all. NullTime.Scan only takes a time.Time and sqlite hands back a string, so even a clean named-zone value failed. scan now wraps it the same way it wraps time.Time. Old rows keep whatever zone they were written in, and new rows are UTC. A table holding both sorts lexically wrong across the two until the old rows are rewritten. Defaulting _time_format=sqlite has the same problem, which is why this doesn't. drivers/sqlitedriver/time_roundtrip_test.go round-trips a monotonic time, a nameless offset and a named zone through insert, bulk insert, update and a transaction, for time.Time, *time.Time and sql.NullTime. It checks the stored text directly too, since the scan fix alone makes a round trip pass whatever the write side does.
extension was on v1.10.0 and kv/extension on v1.9.8, so the two had drifted apart. Both now require v1.11.2. go mod tidy also moved confy to v1.0.3 and go-utils to v1.3.0, bumped golang.org/x/crypto, net and text in kv/extension, and dropped zap and multierr, which forge no longer pulls in.
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
1 participant
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.
No description provided.