will / crystal-pg

a postgres driver for crystal
BSD 3-Clause "New" or "Revised" License
462 stars 77 forks source link

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

Open jwoertink opened 2 years ago

jwoertink commented 2 years ago

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 commented 2 years ago

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 commented 2 years ago

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

straight-shoota commented 2 years ago

Or Time.utc.at_the_end_of_the_day.at_beginning_of_microsecond 🤷

russ commented 2 years ago

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 commented 2 years ago

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.