-
Notifications
You must be signed in to change notification settings - Fork 21.5k
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
Support PG unbounded inclusive date/time range #51920
base: main
Are you sure you want to change the base?
Conversation
@@ -196,14 +196,14 @@ def test_timezone_awareness_endless_tzrange | |||
time_string = Time.current.to_s | |||
time = Time.zone.parse(time_string) | |||
|
|||
record = PostgresqlRange.new(tstz_range: time_string...) | |||
assert_equal time..., record.tstz_range | |||
record = PostgresqlRange.new(tstz_range: time_string..) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This just aligns with the next test, which uses inclusion.
assert_equal_round_trip @first_range, :ts_range, nil..Time.public_send(tz, 2010, 1, 1, 14, 30, 0) | ||
time = Time.public_send(::ActiveRecord.default_timezone, 2010, 1, 1, 14, 30, 0) | ||
|
||
assert_equal_round_trip @first_range, :ts_range, time..nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the key assertion. I'm assuming it was never added because the previous committer kept experiencing it being flipped from ..
to ...
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This patch is very close to the solution I came up with myself while doing my own discovery and investigation of this problem and as far as I can tell this solution is good. I hope it is possible to get this into a release in a near future.
Thanks for fixing this!
Fixes #51745
Motivation / Background
Suppose you have a PG
tsrange
column namedmy_range
. If you create a record withmy_range: (Time.current..)
(inclusive), PG will insert the record with[my_range,]
, but behind the scenes it'll convert it to[my_range,)
, making it exclusive.The problem is when you
reload
or retrieve the record, it'll bemy_range: (Time.current...)
(exclusive).You can "force" inclusivity by using the timestamp value of
infinity
on insertion:[my_range, infinity]
And that's what this change does.
Checklist
Before submitting the PR make sure the following are checked:
[Fix #issue-number]