Merge pull request #20448 from sgrif/sg-postgresql-point-type
Return a `Point` object from the PG Point type
This commit is contained in:
commit
ee372bdceb
|
@ -1,3 +1,8 @@
|
||||||
|
* Deprecate the PG `:point` type in favor of a new one which will return
|
||||||
|
`Point` objects instead of an `Array`
|
||||||
|
|
||||||
|
*Sean Griffin*
|
||||||
|
|
||||||
* Ensure symbols passed to `ActiveRecord::Relation#select` are always treated
|
* Ensure symbols passed to `ActiveRecord::Relation#select` are always treated
|
||||||
as columns.
|
as columns.
|
||||||
|
|
||||||
|
|
|
@ -12,6 +12,7 @@ require 'active_record/connection_adapters/postgresql/oid/json'
|
||||||
require 'active_record/connection_adapters/postgresql/oid/jsonb'
|
require 'active_record/connection_adapters/postgresql/oid/jsonb'
|
||||||
require 'active_record/connection_adapters/postgresql/oid/money'
|
require 'active_record/connection_adapters/postgresql/oid/money'
|
||||||
require 'active_record/connection_adapters/postgresql/oid/point'
|
require 'active_record/connection_adapters/postgresql/oid/point'
|
||||||
|
require 'active_record/connection_adapters/postgresql/oid/rails_5_1_point'
|
||||||
require 'active_record/connection_adapters/postgresql/oid/range'
|
require 'active_record/connection_adapters/postgresql/oid/range'
|
||||||
require 'active_record/connection_adapters/postgresql/oid/specialized_string'
|
require 'active_record/connection_adapters/postgresql/oid/specialized_string'
|
||||||
require 'active_record/connection_adapters/postgresql/oid/uuid'
|
require 'active_record/connection_adapters/postgresql/oid/uuid'
|
||||||
|
|
|
@ -0,0 +1,50 @@
|
||||||
|
module ActiveRecord
|
||||||
|
Point = Struct.new(:x, :y)
|
||||||
|
|
||||||
|
module ConnectionAdapters
|
||||||
|
module PostgreSQL
|
||||||
|
module OID # :nodoc:
|
||||||
|
class Rails51Point < Type::Value # :nodoc:
|
||||||
|
include Type::Helpers::Mutable
|
||||||
|
|
||||||
|
def type
|
||||||
|
:point
|
||||||
|
end
|
||||||
|
|
||||||
|
def cast(value)
|
||||||
|
case value
|
||||||
|
when ::String
|
||||||
|
if value[0] == '(' && value[-1] == ')'
|
||||||
|
value = value[1...-1]
|
||||||
|
end
|
||||||
|
x, y = value.split(",")
|
||||||
|
build_point(x, y)
|
||||||
|
when ::Array
|
||||||
|
build_point(*value)
|
||||||
|
else
|
||||||
|
value
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
def serialize(value)
|
||||||
|
if value.is_a?(ActiveRecord::Point)
|
||||||
|
"(#{number_for_point(value.x)},#{number_for_point(value.y)})"
|
||||||
|
else
|
||||||
|
super
|
||||||
|
end
|
||||||
|
end
|
||||||
|
|
||||||
|
private
|
||||||
|
|
||||||
|
def number_for_point(number)
|
||||||
|
number.to_s.gsub(/\.0$/, '')
|
||||||
|
end
|
||||||
|
|
||||||
|
def build_point(x, y)
|
||||||
|
ActiveRecord::Point.new(Float(x), Float(y))
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
||||||
|
end
|
|
@ -838,6 +838,8 @@ module ActiveRecord
|
||||||
ActiveRecord::Type.register(:jsonb, OID::Jsonb, adapter: :postgresql)
|
ActiveRecord::Type.register(:jsonb, OID::Jsonb, adapter: :postgresql)
|
||||||
ActiveRecord::Type.register(:money, OID::Money, adapter: :postgresql)
|
ActiveRecord::Type.register(:money, OID::Money, adapter: :postgresql)
|
||||||
ActiveRecord::Type.register(:point, OID::Point, adapter: :postgresql)
|
ActiveRecord::Type.register(:point, OID::Point, adapter: :postgresql)
|
||||||
|
ActiveRecord::Type.register(:legacy_point, OID::Point, adapter: :postgresql)
|
||||||
|
ActiveRecord::Type.register(:rails_5_1_point, OID::Rails51Point, adapter: :postgresql)
|
||||||
ActiveRecord::Type.register(:uuid, OID::Uuid, adapter: :postgresql)
|
ActiveRecord::Type.register(:uuid, OID::Uuid, adapter: :postgresql)
|
||||||
ActiveRecord::Type.register(:vector, OID::Vector, adapter: :postgresql)
|
ActiveRecord::Type.register(:vector, OID::Vector, adapter: :postgresql)
|
||||||
ActiveRecord::Type.register(:xml, OID::Xml, adapter: :postgresql)
|
ActiveRecord::Type.register(:xml, OID::Xml, adapter: :postgresql)
|
||||||
|
|
|
@ -310,6 +310,7 @@ module ActiveRecord
|
||||||
def load_schema!
|
def load_schema!
|
||||||
@columns_hash = connection.schema_cache.columns_hash(table_name)
|
@columns_hash = connection.schema_cache.columns_hash(table_name)
|
||||||
@columns_hash.each do |name, column|
|
@columns_hash.each do |name, column|
|
||||||
|
warn_if_deprecated_type(column)
|
||||||
define_attribute(
|
define_attribute(
|
||||||
name,
|
name,
|
||||||
connection.lookup_cast_type_from_column(column),
|
connection.lookup_cast_type_from_column(column),
|
||||||
|
@ -356,6 +357,28 @@ module ActiveRecord
|
||||||
base.table_name
|
base.table_name
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
|
def warn_if_deprecated_type(column)
|
||||||
|
return if attributes_to_define_after_schema_loads.key?(column.name)
|
||||||
|
if column.respond_to?(:oid) && column.sql_type.start_with?("point")
|
||||||
|
if column.array?
|
||||||
|
array_arguments = ", array: true"
|
||||||
|
else
|
||||||
|
array_arguments = ""
|
||||||
|
end
|
||||||
|
ActiveSupport::Deprecation.warn(<<-WARNING.strip_heredoc)
|
||||||
|
The behavior of the `:point` type will be changing in Rails 5.1 to
|
||||||
|
return a `Point` object, rather than an `Array`. If you'd like to
|
||||||
|
keep the old behavior, you can add this line to #{self.name}:
|
||||||
|
|
||||||
|
attribute :#{column.name}, :legacy_point#{array_arguments}
|
||||||
|
|
||||||
|
If you'd like the new behavior today, you can add this line:
|
||||||
|
|
||||||
|
attribute :#{column.name}, :rails_5_1_point#{array_arguments}
|
||||||
|
WARNING
|
||||||
|
end
|
||||||
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
|
@ -6,7 +6,15 @@ class PostgresqlPointTest < ActiveRecord::TestCase
|
||||||
include ConnectionHelper
|
include ConnectionHelper
|
||||||
include SchemaDumpingHelper
|
include SchemaDumpingHelper
|
||||||
|
|
||||||
class PostgresqlPoint < ActiveRecord::Base; end
|
class PostgresqlPoint < ActiveRecord::Base
|
||||||
|
attribute :x, :rails_5_1_point
|
||||||
|
attribute :y, :rails_5_1_point
|
||||||
|
attribute :z, :rails_5_1_point
|
||||||
|
attribute :array_of_points, :rails_5_1_point, array: true
|
||||||
|
attribute :legacy_x, :legacy_point
|
||||||
|
attribute :legacy_y, :legacy_point
|
||||||
|
attribute :legacy_z, :legacy_point
|
||||||
|
end
|
||||||
|
|
||||||
def setup
|
def setup
|
||||||
@connection = ActiveRecord::Base.connection
|
@connection = ActiveRecord::Base.connection
|
||||||
|
@ -14,11 +22,27 @@ class PostgresqlPointTest < ActiveRecord::TestCase
|
||||||
t.point :x
|
t.point :x
|
||||||
t.point :y, default: [12.2, 13.3]
|
t.point :y, default: [12.2, 13.3]
|
||||||
t.point :z, default: "(14.4,15.5)"
|
t.point :z, default: "(14.4,15.5)"
|
||||||
|
t.point :array_of_points, array: true
|
||||||
|
t.point :legacy_x
|
||||||
|
t.point :legacy_y, default: [12.2, 13.3]
|
||||||
|
t.point :legacy_z, default: "(14.4,15.5)"
|
||||||
|
end
|
||||||
|
@connection.create_table('deprecated_points') do |t|
|
||||||
|
t.point :x
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
teardown do
|
teardown do
|
||||||
@connection.drop_table 'postgresql_points', if_exists: true
|
@connection.drop_table 'postgresql_points', if_exists: true
|
||||||
|
@connection.drop_table 'deprecated_points', if_exists: true
|
||||||
|
end
|
||||||
|
|
||||||
|
class DeprecatedPoint < ActiveRecord::Base; end
|
||||||
|
|
||||||
|
def test_deprecated_legacy_type
|
||||||
|
assert_deprecated do
|
||||||
|
DeprecatedPoint.new
|
||||||
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_column
|
def test_column
|
||||||
|
@ -32,11 +56,11 @@ class PostgresqlPointTest < ActiveRecord::TestCase
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_default
|
def test_default
|
||||||
assert_equal [12.2, 13.3], PostgresqlPoint.column_defaults['y']
|
assert_equal ActiveRecord::Point.new(12.2, 13.3), PostgresqlPoint.column_defaults['y']
|
||||||
assert_equal [12.2, 13.3], PostgresqlPoint.new.y
|
assert_equal ActiveRecord::Point.new(12.2, 13.3), PostgresqlPoint.new.y
|
||||||
|
|
||||||
assert_equal [14.4, 15.5], PostgresqlPoint.column_defaults['z']
|
assert_equal ActiveRecord::Point.new(14.4, 15.5), PostgresqlPoint.column_defaults['z']
|
||||||
assert_equal [14.4, 15.5], PostgresqlPoint.new.z
|
assert_equal ActiveRecord::Point.new(14.4, 15.5), PostgresqlPoint.new.z
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_schema_dumping
|
def test_schema_dumping
|
||||||
|
@ -49,22 +73,95 @@ class PostgresqlPointTest < ActiveRecord::TestCase
|
||||||
def test_roundtrip
|
def test_roundtrip
|
||||||
PostgresqlPoint.create! x: [10, 25.2]
|
PostgresqlPoint.create! x: [10, 25.2]
|
||||||
record = PostgresqlPoint.first
|
record = PostgresqlPoint.first
|
||||||
assert_equal [10, 25.2], record.x
|
assert_equal ActiveRecord::Point.new(10, 25.2), record.x
|
||||||
|
|
||||||
record.x = [1.1, 2.2]
|
record.x = ActiveRecord::Point.new(1.1, 2.2)
|
||||||
record.save!
|
record.save!
|
||||||
assert record.reload
|
assert record.reload
|
||||||
assert_equal [1.1, 2.2], record.x
|
assert_equal ActiveRecord::Point.new(1.1, 2.2), record.x
|
||||||
end
|
end
|
||||||
|
|
||||||
def test_mutation
|
def test_mutation
|
||||||
p = PostgresqlPoint.create! x: [10, 20]
|
p = PostgresqlPoint.create! x: ActiveRecord::Point.new(10, 20)
|
||||||
|
|
||||||
p.x[1] = 25
|
p.x.y = 25
|
||||||
p.save!
|
p.save!
|
||||||
p.reload
|
p.reload
|
||||||
|
|
||||||
assert_equal [10.0, 25.0], p.x
|
assert_equal ActiveRecord::Point.new(10.0, 25.0), p.x
|
||||||
|
assert_not p.changed?
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_array_assignment
|
||||||
|
p = PostgresqlPoint.new(x: [1, 2])
|
||||||
|
|
||||||
|
assert_equal ActiveRecord::Point.new(1, 2), p.x
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_string_assignment
|
||||||
|
p = PostgresqlPoint.new(x: "(1, 2)")
|
||||||
|
|
||||||
|
assert_equal ActiveRecord::Point.new(1, 2), p.x
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_array_of_points_round_trip
|
||||||
|
expected_value = [
|
||||||
|
ActiveRecord::Point.new(1, 2),
|
||||||
|
ActiveRecord::Point.new(2, 3),
|
||||||
|
ActiveRecord::Point.new(3, 4),
|
||||||
|
]
|
||||||
|
p = PostgresqlPoint.new(array_of_points: expected_value)
|
||||||
|
|
||||||
|
assert_equal expected_value, p.array_of_points
|
||||||
|
p.save!
|
||||||
|
p.reload
|
||||||
|
assert_equal expected_value, p.array_of_points
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_legacy_column
|
||||||
|
column = PostgresqlPoint.columns_hash["legacy_x"]
|
||||||
|
assert_equal :point, column.type
|
||||||
|
assert_equal "point", column.sql_type
|
||||||
|
assert_not column.array?
|
||||||
|
|
||||||
|
type = PostgresqlPoint.type_for_attribute("legacy_x")
|
||||||
|
assert_not type.binary?
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_legacy_default
|
||||||
|
assert_equal [12.2, 13.3], PostgresqlPoint.column_defaults['legacy_y']
|
||||||
|
assert_equal [12.2, 13.3], PostgresqlPoint.new.legacy_y
|
||||||
|
|
||||||
|
assert_equal [14.4, 15.5], PostgresqlPoint.column_defaults['legacy_z']
|
||||||
|
assert_equal [14.4, 15.5], PostgresqlPoint.new.legacy_z
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_legacy_schema_dumping
|
||||||
|
output = dump_table_schema("postgresql_points")
|
||||||
|
assert_match %r{t\.point\s+"legacy_x"$}, output
|
||||||
|
assert_match %r{t\.point\s+"legacy_y",\s+default: \[12\.2, 13\.3\]$}, output
|
||||||
|
assert_match %r{t\.point\s+"legacy_z",\s+default: \[14\.4, 15\.5\]$}, output
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_legacy_roundtrip
|
||||||
|
PostgresqlPoint.create! legacy_x: [10, 25.2]
|
||||||
|
record = PostgresqlPoint.first
|
||||||
|
assert_equal [10, 25.2], record.legacy_x
|
||||||
|
|
||||||
|
record.legacy_x = [1.1, 2.2]
|
||||||
|
record.save!
|
||||||
|
assert record.reload
|
||||||
|
assert_equal [1.1, 2.2], record.legacy_x
|
||||||
|
end
|
||||||
|
|
||||||
|
def test_legacy_mutation
|
||||||
|
p = PostgresqlPoint.create! legacy_x: [10, 20]
|
||||||
|
|
||||||
|
p.legacy_x[1] = 25
|
||||||
|
p.save!
|
||||||
|
p.reload
|
||||||
|
|
||||||
|
assert_equal [10.0, 25.0], p.legacy_x
|
||||||
assert_not p.changed?
|
assert_not p.changed?
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
Loading…
Reference in New Issue