Skip to content

Make sure to use timeString/timeStringNullable everywhere for SQLite - #1353

Merged
brandur merged 1 commit into
masterfrom
brandur-time-string
Aug 21, 2026
Merged

Make sure to use timeString/timeStringNullable everywhere for SQLite#1353
brandur merged 1 commit into
masterfrom
brandur-time-string

Conversation

@brandur

@brandur brandur commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Make sure to use timeString/timeStringNullable everywhere for SQLite
rather than passing a time.Time or *time.Time. Using the latter
works, but causes the underlying driver to default to whatever
serialization method for the time it wants to use, making it a different
format compared to what we normally store, and leading to requiring
liberal input unmarshaling as times might exist in a variety of formats
(we had a recent River Pro bug where this was the case).

I also checked River Pro for problems of this sort, but we're clean over
there already.

Make sure to use `timeString`/`timeStringNullable` everywhere for SQLite
rather than passing a `time.Time` or `*time.Time`. Using the latter
works, but causes the underlying driver to default to whatever
serialization method for the time it wants to use, making it a different
format compared to what we normally store, and leading to requiring
liberal input unmarshaling as times might exist in a variety of formats
(we had a recent River Pro bug where this was the case).

I also checked River Pro for problems of this sort, but we're clean over
there already.
@brandur
brandur force-pushed the brandur-time-string branch from 6b73c82 to 2c7ccfc Compare August 20, 2026 21:50
@brandur
brandur requested a review from bgentry August 20, 2026 21:50
@brandur

brandur commented Aug 21, 2026

Copy link
Copy Markdown
Contributor Author

thx!

@brandur
brandur merged commit 2158b28 into master Aug 21, 2026
14 of 15 checks passed
@brandur
brandur deleted the brandur-time-string branch August 21, 2026 20:29
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