Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Time seems to drop precision when passed in as an arg using at_end_of_day #252

Open
jwoertink opened this issue Jun 13, 2022 · 5 comments
Open

Comments

@jwoertink
Copy link
Contributor

It seems when passing in a Time object set to at_end_of_day as an arg, it somehow loses the precision and rolls in to the beginning of the next day.

Here's an example where I generate a date series. The first SQL has the time set manually, and when ran, it returns 1 result properly. The second SQL pulls the times from Crystal and passes it over as as args. Even though the Crystal times have the same values, running this SQL returns 2 results instead of 1.

class Date
  DB.mapping({date: String})
end

working_sql = <<-SQL
SELECT
  date::text
FROM GENERATE_SERIES(date('2022-05-01T00:00:00.000000000Z'), date('2022-05-01T23:59:59.999999999Z'), '1 DAY') AS date
SQL

borked_sql = <<-SQL
SELECT
  date::text
FROM GENERATE_SERIES(date($1), date($2), '1 DAY') AS date
SQL

url = "postgres://postgres@localhost:5432"

DB.open(url) do |db|
  single_result = db.query_all(working_sql, as: Date)

  pp! single_result

  day_start = Time.utc(2022, 5, 1).at_beginning_of_day
  day_end = Time.utc(2022, 5, 1).at_end_of_day

  double_results = db.query_all(borked_sql, args: [day_start, day_end], as: Date)

  pp! double_results
end
@straight-shoota
Copy link
Contributor

straight-shoota commented Jun 13, 2022

The reason is that timestamp types in posgresql only store the time value with microsecond precision but Crystal's Time type has nanosecond precision.
at_end_of_day results in a time instance with 999_999_999 nanoseconds. When postgres parses that time information, it rounds the excess precision to the next full microsecond, which is the first microsecond of the next day.

You can observe the same behaviour running this SQL query:

SELECT '2022-06-13T23:59:59.999999999999Z' :: timestamp;
--       timestamp
-- ---------------------
--  2022-06-14 00:00:00
-- (1 row)

@jwoertink
Copy link
Contributor Author

Oh lovely. Good catch. I guess we need Time.utc.almost_at_the_end_of_the_day 😂

@straight-shoota
Copy link
Contributor

Or Time.utc.at_the_end_of_the_day.at_beginning_of_microsecond 🤷

@russ
Copy link

russ commented Jun 13, 2022

If that is the case, can this line be updated to 6 to only do the microseconds?
https://github.com/will/crystal-pg/blob/master/src/pq/param.cr#L23

Running a quick test on my end makes the example work.

@straight-shoota
Copy link
Contributor

Yes, that should fix this problem. There are other implications, though. Truncating the fraction digits results in an odd rounding behaviour that might be unexpected at some times. So I'm not sure if the driver should handle this.
Would be interesting to look how other drivers handle excess precision.

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

No branches or pull requests

3 participants