Proposed PR: configurable time encoding
kalafut opened this issue · 5 comments
Hi, and thank you for an excellent library that I've used for years!
This driver stores time.Time
values as strings:
Line 219 in 8543684
int
version of whatever time I'm using.
I'd like to propose a new driver configuration parameter, _time_format
, that would let one opt into different storage formats. This lets you use time.Time
directly but get the encoding you want. I've built a version that allows unix
and unix_ms
formats, and it is a small (~dozen LOC) change: kalafut/go-sqlite3.
Other formats could be added in the future too, which might be useful (e.g. I noticed #951 (comment)). Note that this is completely opt-in and only affects the way values are written. Reading/parsing is unchanged.
Is this of any interest? If so I can prepare a proper PR with better, consolidated tests, and some docs.
Thanks for taking a look.
Reading/parsing is unchanged.
I would think that you would have to change reading/parsing to also consider the custom encoding, or else it will be unable to read what it just wrote, unless it happens to be the same as one of the entries in SQLiteTimestampFormats
.
I also wonder if this sort of thing would be better in a driver.Connector
implementation (which this library currently lacks) instead of continuing to cram things into the DSN.
I would think that you would have to change reading/parsing
For truly custom formats, that's right. I was envisioning that these would be more of a enum type that would relate to selecting one of the SQLiteTimestampFormats
. But even that is a bit more than what I'm most interested in, which is integral timestamp support. These can already be parsed by the driver:
Lines 2126 to 2140 in 8543684
Today if I have a unix timestamp stored in e.g. a TIMESTAMP
column, it is converted just fine. There is just no way to write them without adding another int
to the structure, and is what the proposal addresses.
As for the location of the configuration... I'm not partial to it and actually don't know that much about the options available. I only picked the DSN since I was familiar with it and saw other config there. I can take a look at a Connector implementation if you think that is now worth investing in.
The other thing to keep in mind is that with what you are proposing, it would affect the entire connection. You might be better served making something that implements driver.Valuer
and sql.Scanner
so you can explicitly control the specific field rather than relying on the DSN being correct. To that end, maybe this library could expose some typedefs for that.
I started this with experiments using custom types. They work for the database's use, but those same time values are then used lots of other places and it gets tedious bumping into MyTime
type not being time.Time
, and the conversions then ensue. The friction also extends into external tooling (e.g. https://sqlc.dev).
I don't know if there is any other way to effect per-field configuration.
Well, I'll let you think it over and no biggee if you decide the design doesn't fit and you close this. The fork is pretty easy to keep up and serves my needs.
I’ve also run into this issue. I tend to write time in a string format in the highest resolution SQLite natively supports (milliseconds), without truncation of 0 suffixes, for sortability: strftime('%Y-%m-%dT%H:%M:%fZ')
I’ve resorted to a custom type with (de)serialization which embeds a time.Time
, but it feels a bit awkward. So a driver solution would be preferable for me as well.
I’ve played with solutions myself. One I liked was having a driver initializer constructor function taking a struct, instead of relying on an init()
function and DSN parsing. But I know that’s a breaking change…