search

lib / pq

Pure Go Postgres driver for database/sql
https://pkg.go.dev/github.com/lib/pq
MIT License
9.09k stars 911 forks source link

Encode time with Postgres accuracy limit #954

Closed anicoll closed 4 years ago

anicoll commented 4 years ago

Postgres limit for accuracy means we need to truncate the time to be as accurate as possible https://www.postgresql.org/docs/current/datatype-datetime.html

cbandy commented 4 years ago

My opinion is the same as last time, https://github.com/lib/pq/issues/227#issuecomment-88530781. This driver should not silently throw away the nanoseconds given to it by the user.

heedson commented 4 years ago

@cbandy Postgres doesn't seem to mention how it deals with more than microsecond precision, therefore it's undefined in the spec. What are your thoughts on https://github.com/lib/pq/pull/954#discussion_r399305269 and that time isn't necessarily always best compared to a number to be rounded?

Since Postgres will end up throwing the data away, I don't think a driver doing that in a more usable way is a bad thing.

What would your opinion be on @leighmcculloch suggestion of a configurable value to decide how it's handled? We were almost bit hard by not truncating our times (and I believe truncating before insertion to not be common) that were sensitive to being EndOfDay() but also staying on the day. Postgres was rounding to the next day... I think that's less accurate but maybe that's only my opinion on an edge case.

leighmcculloch commented 4 years ago

I think the bigger issue here is that the Go stdlib does not provide a function to round that parallels that used by C and by Postgres. If it did have a function that allowed us to round a number to match Postgres' rounded number in every case, would these problems exist? We could propose adding this to the time package.

heedson commented 4 years ago

Just to clarify - if you execute the commands @anicoll provided directly on a postgres instance, it will round the same way. I think the main concern I have with rounding a time is that you can potentially change other facts about the "wallclock" values of said time by accident (difference of billing/expiring a customer account 1 day late).

Rounding 1 microsecond up means 1 more microsecond. 1 more microsecond could mean a new year, new month or new day. I think it's a bit more complex than saying that we shouldn't throw away user data however I do understand that the best way this could all be solved is if Postgres itself handled it.

cbandy commented 4 years ago

@cbandy Postgres doesn't seem to mention how it deals with more than microsecond precision, therefore it's undefined in the spec.

Agreed. I don't see any mention of how nanoseconds are interpreted in the docs.

What are your thoughts on #954 (comment) and that time isn't necessarily always best compared to a number to be rounded?

The example that we ran into was with https://github.com/jinzhu/now and using the EndOfDay() of a time. It would give the exact moment before the end of a day.

EndOfDay() seems ... ill-founded. It is the largest timestamp that time.Time can represent for the same .Date() but ... there is no "exact moment before the end of a day."

Because there is no "last" moment of any day that can be compared, most software uses "before the next day."

Since Postgres will end up throwing the data away, I don't think a driver doing that in a more usable way is a bad thing.

PostgreSQL doesn't store the nanoseconds but it does interpret them when choosing what to store. It looks like round-to-even is taking place on my machines.

What would your opinion be on @leighmcculloch suggestion of a configurable value to decide how it's handled?

As far as I know, this driver tries to stay close to the behavior provided by libpq. To that end, new settings probably belong on the Connector type rather than in the DSN.

We were almost bit hard by not truncating our times (and I believe truncating before insertion to not be common) that were sensitive to being EndOfDay() but also staying on the day. Postgres was rounding to the next day... I think that's less accurate but maybe that's only my opinion on an edge case.

My experience is that (regardless of PostgreSQL) you probably want to be handling your "today" and "end of day" concepts differently. The way you're using time.Time, nanoseconds are really important. I suspect other parts of your stack will encounter this edge case as well.

anicoll commented 4 years ago

Closing this PR as clearly there is no consensus on the matter.