Skip to content

Commit

Permalink
Fix SchemaStatements#initialize_type_map
Browse files Browse the repository at this point in the history
Since the Rails 6 upgrade, we started seeing some random PostGIS type
issues on our staging env, while we don't know yet why our production env
doesn't seem to be affected, debugging the issue on staging shows that the
ActiveRecord connection `type_map` isn't initialized properly which leave the
custom `ActiveRecord::ConnectionAdapters::PostGIS::SpatialColumn` without
a defined type, while usually, the type is lazy loaded correctly later on
by  `PostgreSQLAdapter#get_oid_type` it could happen that the
`ModelSchema#load_schema!` method is called first which will define and
cache the attribute type (`@attribute_types`) with the wrong
default value type (`ActiveModel::Type::Value`) `type_cast` using the
`lookup_cast_type_from_column` method that reads straight from the
 `type_map` cache without lazy loading the type. The wrong type is
then always returned for that column which brakes all the casting logic.

This patch fixes the `SchemaStatements#initialize_type_map`
method by ensuring that the specific PostGIS types are registered before
calling `super` which will then call `load_additional_types` and registers
the custom types. That way the custom column will be properly initialized
and be present in the `type_map` cache.

Some code references:
 - The `initialize_type_map` activerecord-postgis-adapter method overwritten https://github.com/rgeo/activerecord-postgis-adapter/blob/cc0e3f53520feed0b43800d8ee86561ee03de413/lib/active_record/connection_adapters/postgis/schema_statements.rb#L87-L105
 - the `load_additional_types` method in Rails: https://github.com/rails/rails/blob/8d57cb39a88787bb6cfb7e1c481556ef6d8ede7a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb#L593
 - the `get_oid_type` method in Rails that lazy load the type: https://github.com/rails/rails/blob/8d57cb39a88787bb6cfb7e1c481556ef6d8ede7a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb#L514-L516
 - the `lookup_cast_type_from_column` method in Rails that does not lazy load the type: https://github.com/rails/rails/blob/15748f6a052d4df68b6acf66456c181b42d9fe25/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb#L78

This patch should solve rgeo#307 and rgeo#308.
  • Loading branch information
thibaudgg committed Feb 24, 2020
1 parent 744bed7 commit aae9fc7
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 3 deletions.
2 changes: 1 addition & 1 deletion CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ run `rake appraisal` or run the tests manually:

```
BUNDLE_GEMFILE=./gemfiles/ar60.gemfile bundle
BUNDLE_GEMFILE=./gemfiles/ar60.gemfile rake
BUNDLE_GEMFILE=./gemfiles/ar60.gemfile bundle exec rake
```

Make your changes and submit a pull request.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,6 @@ def spatial_column_info(table_name)
end

def initialize_type_map(map = type_map)
super

%w(
geography
geometry
Expand All @@ -102,6 +100,8 @@ def initialize_type_map(map = type_map)
OID::Spatial.new(oid, sql_type)
end
end

super
end
end
end
Expand Down
24 changes: 24 additions & 0 deletions test/schema_statements_test.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# frozen_string_literal: true

require "test_helper"

class SchemaStatementsTest < ActiveSupport::TestCase
def test_initialize_type_map
initialized_types = SpatialModel.connection.send(:type_map).keys

# PostGIS types must be initialized first, so
# ActiveRecord::ConnectionAdapters::PostgreSQLAdapter#load_additional_types can use them
# https://github.com/rails/rails/blob/8d57cb39a88787bb6cfb7e1c481556ef6d8ede7a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb#L593
assert_equal initialized_types.first(9), %w[
geography
geometry
geometry_collection
line_string
multi_line_string
multi_point
multi_polygon
st_point
st_polygon
]
end
end

0 comments on commit aae9fc7

Please sign in to comment.