Skip to content

Commit

Permalink
Postgres::OID::Range serializes to a Range, quote in Quoting
Browse files Browse the repository at this point in the history
PostgreSQL 9.1+ introduced range types, and Rails added support for
using this datatype in ActiveRecord. However, the serialization of
`PostgreSQL::OID::Range` was incomplete, because it did not properly
quote the bounds that make up the range. A clear example of this is a
`tsrange`.

Normally, ActiveRecord quotes Date/Time objects to include the
milliseconds. However, the way `PostgreSQL::OID::Range` serialized its
bounds, the milliseconds were dropped. This meant that the value was
incomplete and not equal to the submitted value.

An example of normal timestamps vs. a `tsrange`. Note how the bounds
for the range do not include their milliseconds (they were present in
the ruby Range):

    UPDATE "iterations" SET "updated_at" = $1, "range" = $2 WHERE
    "iterations"."id" = $3
    [["updated_at", "2017-09-23 17:07:01.304864"],
    ["range", "[2017-09-23 00:00:00 UTC,2017-09-23 23:59:59 UTC]"],
    ["id", 1234]]

`PostgreSQL::OID::Range` serialized the range by interpolating a
string for the range, which works for most cases, but does not work
for timestamps:

    def serialize(value)
      if value.is_a?(::Range)
        from = type_cast_single_for_database(value.begin)
        to = type_cast_single_for_database(value.end)
        "[#{from},#{to}#{value.exclude_end? ? ')' : ']'}"
      else
        super
      end
    end

    (byebug) from = type_cast_single_for_database(value.begin)
    2010-01-01 13:30:00 UTC

    (byebug) to = type_cast_single_for_database(value.end)
    2011-02-02 19:30:00 UTC

    (byebug) "[#{from},#{to}#{value.exclude_end? ? ')' : ']'}"
    "[2010-01-01 13:30:00 UTC,2011-02-02 19:30:00 UTC)"

@sgrif (the original implementer for Postgres Range support) provided
some feedback about where the quoting should occur:

  Yeah, quoting at all is definitely wrong here. I'm not sure what I
  was thinking in 02579b5, but what this is doing is definitely in the
  wrong place. It should probably just be returning a range of
  subtype.serialize(value.begin) and subtype.serialize(value.end), and
  letting the adapter handle the rest.

`Postgres::OID::Range` now returns a `Range` object, and
`ActiveRecord::ConnectionAdapters::PostgreSQL::Quoting` can now encode
and quote a `Range`:

    def encode_range(range)
      "[#{type_cast(range.first)},#{type_cast(range.last)}#{range.exclude_end? ? ')' : ']'}"
    end

    ...

    encode_range(range)
    #=> "['2010-01-01 13:30:00.670277','2011-02-02 19:30:00.745125')"

This commit includes tests to make sure the milliseconds are
preserved in `tsrange` and `tstzrange` columns
  • Loading branch information
tcannonfodder committed Sep 27, 2017
1 parent 40a63e5 commit 51b6c34
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 2 deletions.
20 changes: 20 additions & 0 deletions activerecord/CHANGELOG.md
@@ -1,3 +1,23 @@
* PostgreSQL `tsrange` now preserves subsecond precision

PostgreSQL 9.1+ introduced range types, and Rails added support for using
this datatype in ActiveRecord. However, the serialization of
PostgreSQL::OID::Range was incomplete, because it did not properly
cast the bounds that make up the range. This led to subseconds being
dropped in SQL commands:

(byebug) from = type_cast_single_for_database(range.first)
2010-01-01 13:30:00 UTC

(byebug) to = type_cast_single_for_database(range.last)
2011-02-02 19:30:00 UTC

(byebug) "[#{from},#{to}#{value.exclude_end? ? ')' : ']'}"
"[2010-01-01 13:30:00 UTC,2011-02-02 19:30:00 UTC)"

(byebug) "[#{type_cast(from)},#{type_cast(to)}#{value.exclude_end? ? ')' : ']'}"
"['2010-01-01 13:30:00.670277','2011-02-02 19:30:00.745125')"

* Passing a `Set` to `Relation#where` now behaves the same as passing an
array.

Expand Down
Expand Up @@ -35,7 +35,7 @@ def serialize(value)
if value.is_a?(::Range)
from = type_cast_single_for_database(value.begin)
to = type_cast_single_for_database(value.end)
"[#{from},#{to}#{value.exclude_end? ? ')' : ']'}"
::Range.new(from, to, value.exclude_end?)
else
super
end
Expand Down
Expand Up @@ -101,6 +101,8 @@ def _quote(value)
end
when OID::Array::Data
_quote(encode_array(value))
when Range
_quote(encode_range(value))
else
super
end
Expand All @@ -117,6 +119,8 @@ def _type_cast(value)
value.to_s
when OID::Array::Data
encode_array(value)
when Range
encode_range(value)
else
super
end
Expand All @@ -133,6 +137,10 @@ def encode_array(array_data)
result
end

def encode_range(range)
"[#{type_cast(range.first)},#{type_cast(range.last)}#{range.exclude_end? ? ')' : ']'}"
end

def determine_encoding_of_strings_in_array(value)
case value
when ::Array then determine_encoding_of_strings_in_array(value.first)
Expand Down
51 changes: 51 additions & 0 deletions activerecord/test/cases/adapters/postgresql/range_test.rb
Expand Up @@ -232,6 +232,57 @@ def test_timezone_awareness_tsrange
end
end

def test_create_tstzrange_preserve_usec
tstzrange = Time.parse("2010-01-01 14:30:00.670277 +0100")...Time.parse("2011-02-02 14:30:00.745125 CDT")
round_trip(@new_range, :tstz_range, tstzrange)
assert_equal @new_range.tstz_range, tstzrange
assert_equal @new_range.tstz_range, Time.parse("2010-01-01 13:30:00.670277 UTC")...Time.parse("2011-02-02 19:30:00.745125 UTC")
end

def test_update_tstzrange_preserve_usec
assert_equal_round_trip(@first_range, :tstz_range,
Time.parse("2010-01-01 14:30:00.245124 CDT")...Time.parse("2011-02-02 14:30:00.451274 CET"))
assert_nil_round_trip(@first_range, :tstz_range,
Time.parse("2010-01-01 14:30:00.245124 +0100")...Time.parse("2010-01-01 13:30:00.245124 +0000"))
end

def test_create_tsrange_preseve_usec
tz = ::ActiveRecord::Base.default_timezone
assert_equal_round_trip(@new_range, :ts_range,
Time.send(tz, 2010, 1, 1, 14, 30, 0, 125435)...Time.send(tz, 2011, 2, 2, 14, 30, 0, 225435))
end

def test_update_tsrange_preserve_usec
tz = ::ActiveRecord::Base.default_timezone
assert_equal_round_trip(@first_range, :ts_range,
Time.send(tz, 2010, 1, 1, 14, 30, 0, 142432)...Time.send(tz, 2011, 2, 2, 14, 30, 0, 224242))
assert_nil_round_trip(@first_range, :ts_range,
Time.send(tz, 2010, 1, 1, 14, 30, 0, 142432)...Time.send(tz, 2010, 1, 1, 14, 30, 0, 142432))
end

def test_timezone_awareness_tsrange_preserve_usec
tz = "Pacific Time (US & Canada)"

in_time_zone tz do
PostgresqlRange.reset_column_information
time_string = "2017-09-26 07:30:59.132451 -0700"
time = Time.zone.parse(time_string)
assert time.usec > 0

record = PostgresqlRange.new(ts_range: time_string..time_string)
assert_equal time..time, record.ts_range
assert_equal ActiveSupport::TimeZone[tz], record.ts_range.begin.time_zone
assert_equal time.usec, record.ts_range.begin.usec

record.save!
record.reload

assert_equal time..time, record.ts_range
assert_equal ActiveSupport::TimeZone[tz], record.ts_range.begin.time_zone
assert_equal time.usec, record.ts_range.begin.usec
end
end

def test_create_numrange
assert_equal_round_trip(@new_range, :num_range,
BigDecimal.new("0.5")...BigDecimal.new("1"))
Expand Down
Expand Up @@ -30,6 +30,6 @@ class PostgresqlTypeLookupTest < ActiveRecord::PostgreSQLTestCase
big_range = 0..123456789123456789

assert_raises(ActiveModel::RangeError) { int_range.serialize(big_range) }
assert_equal "[0,123456789123456789]", bigint_range.serialize(big_range)
assert_equal "[0,123456789123456789]", @connection.type_cast(bigint_range.serialize(big_range))
end
end

0 comments on commit 51b6c34

Please sign in to comment.