Skip to content
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

RGeo::Geographic::SphericalPointImpl serialization not happening #307

Closed
kwent opened this issue Jan 6, 2020 · 15 comments
Closed

RGeo::Geographic::SphericalPointImpl serialization not happening #307

kwent opened this issue Jan 6, 2020 · 15 comments

Comments

@kwent
Copy link

kwent commented Jan 6, 2020

Environment

Platform affected (untested on other platforms)

OSX

Rails

6.0.2.1

Gemfile.lock

activerecord-postgis-adapter (6.0.0)
  activerecord (~> 6.0)
  rgeo-activerecord (~> 6.0)

PostgreSQL

psql (12.1)
 PostgreSQL 12.1 on x86_64-apple-darwin19.0.0, compiled by Apple clang version 11.0.0 (clang-1100.0.33.12), 64-bit

Postgis

psql (12.1)
db=# select PostGIS_Full_Version();
POSTGIS="3.0.0 r17983" [EXTENSION] PGSQL="120" GEOS="3.8.0-CAPI-1.13.1 " PROJ="6.2.1" LIBXML="2.9.10" LIBJSON="0.13.1" LIBPROTOBUF="1.3.2" WAGYU="0.4.3 (Internal)"

Given

class User < ApplicationRecord
  def latitude
    @latitude ||= self.lonlat.lat
  end
  alias_method :lat, :latitude

  def longitude
    @longitude ||= self.lonlat.lon
  end
  alias_method :lon, :longitude
end

Issue

Seeing on some when doing some http requests (it's not a consistent error, sometime behave well, sometime behave badly):

NoMethodError (undefined method `lat' for "0101000020E6100000F349383EA45F5EC0E462B1F2704A4340":String
NoMethodError (undefined method `lat' for "POINT(-122.406417 37.785834)":String

Looks like the adapter is not serializing latlon to a RGeo::Geographic::SphericalPointImpl object.

An idea of what could go wrong ?

Regards

@klaustopher
Copy link

klaustopher commented Jan 6, 2020

I am experiencing the same behavior ... I have multiple geometry columns, and in some models it works fine, in others it does not.

>> MatchingConfiguration.columns.find { |c| c.name == 'area' }
=> #<ActiveRecord::ConnectionAdapters::PostGIS::SpatialColumn:0x00007fe4cb94fe00 @sql_type_metadata=#<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x00007fe4cb944398 @sql_type="geometry(MultiPolygon,4326)", @type=:geometry, @limit=nil, @precision=nil, @scale=nil>, @geographic=false, @name="area", @null=true, @default=nil, @default_function=nil, @collation=nil, @comment=nil, @serial=nil, @srid=4326, @has_z=false, @has_m=false, @geometric_type=RGeo::Feature::MultiPolygon, @limit={:srid=>4326, :type=>"multi_polygon"}>

>> Product.columns.find { |c| c.name == 'area' }
=> #<ActiveRecord::ConnectionAdapters::PostGIS::SpatialColumn:0x00007fe4cbe78350 @sql_type_metadata=#<ActiveRecord::ConnectionAdapters::SqlTypeMetadata:0x00007fe4cbe78508 @sql_type="geometry(MultiPolygon,4326)", @type=:geometry, @limit=nil, @precision=nil, @scale=nil>, @geographic=false, @name="area", @null=true, @default=nil, @default_function=nil, @collation=nil, @comment=nil, @serial=nil, @srid=4326, @has_z=false, @has_m=false, @geometric_type=RGeo::Feature::MultiPolygon, @limit={:srid=>4326, :type=>"multi_polygon"}>

>> MatchingConfiguration.first.area
  MatchingConfiguration Load (1.1ms)  SELECT "matching_configurations".* FROM "matching_configurations" ORDER BY "matching_configurations"."id" ASC LIMIT $1  [["LIMIT", 1]]
=> #<RGeo::Geographic::SphericalMultiPolygonImpl:0x3ff2668bde84 "MULTIPOLYGON (((8.6 50.1, ...)))">

>> Product.first.area
  Product Load (0.5ms)  SELECT "products".* FROM "products" ORDER BY "products"."created_at" ASC LIMIT $1  [["LIMIT", 1]]
=> "0106..."

same code is working fine with adapter version 5.2.2 (otherwise same setup: pg gem 1.2.1)

@kwent
Copy link
Author

kwent commented Jan 6, 2020

Closest related issue i could find is rgeo/rgeo#113.

I think there is an issue around those line :

@klaustopher
Copy link

Hmm, this code wasn't touched in a while

@kwent
Copy link
Author

kwent commented Jan 22, 2020

I also don’t see that issue in a production setup. My guess is that the dev hot reloading in rails is not reloading the adapter postgis adapter and so the model is reading raw values of the columns.

@klaustopher
Copy link

I do not only see it in dev mode. When running my whole spsec suite it works fine, vut when running a single test with a model that uses geo columns, it just reads the raw string

@kwent
Copy link
Author

kwent commented Jan 23, 2020

Still make me think to a load/reload adapter kind of issue.

@keithdoggett
Copy link
Member

Would you mind posting what ruby version, server, and any gems used specifically in development/test?

I think I've seen this issue before and started looking into it, but I haven't had any luck replicating it. I'm running Ubuntu 18, but other than that our setups look the same.

@kwent
Copy link
Author

kwent commented Feb 21, 2020

Sure thing

  • Darwin MacBook-Pro.local 19.3.0 Darwin Kernel Version 19.3.0: Thu Jan 9 20:58:23 PST 2020; root:xnu-6153.81.5~1/RELEASE_X86_64 x86_64
  • ruby 2.6.3p62 (2019-04-16 revision 67580) [universal.x86_64-darwin19]
  • Rails 6.0.1

Gemfile.lock

GEM
  remote: https://rubygems.org/
  specs:
    CFPropertyList (3.0.2)
    activesupport (4.2.11.1)
      i18n (~> 0.7)
      minitest (~> 5.1)
      thread_safe (~> 0.3, >= 0.3.4)
      tzinfo (~> 1.1)
    addressable (2.7.0)
      public_suffix (>= 2.0.2, < 5.0)
    algoliasearch (1.27.1)
      httpclient (~> 2.8, >= 2.8.3)
      json (>= 1.5.1)
    atomos (0.1.3)
    babosa (1.0.3)
    claide (1.0.3)
    cocoapods (1.8.4)
      activesupport (>= 4.0.2, < 5)
      claide (>= 1.0.2, < 2.0)
      cocoapods-core (= 1.8.4)
      cocoapods-deintegrate (>= 1.0.3, < 2.0)
      cocoapods-downloader (>= 1.2.2, < 2.0)
      cocoapods-plugins (>= 1.0.0, < 2.0)
      cocoapods-search (>= 1.0.0, < 2.0)
      cocoapods-stats (>= 1.0.0, < 2.0)
      cocoapods-trunk (>= 1.4.0, < 2.0)
      cocoapods-try (>= 1.1.0, < 2.0)
      colored2 (~> 3.1)
      escape (~> 0.0.4)
      fourflusher (>= 2.3.0, < 3.0)
      gh_inspector (~> 1.0)
      molinillo (~> 0.6.6)
      nap (~> 1.0)
      ruby-macho (~> 1.4)
      xcodeproj (>= 1.11.1, < 2.0)
    cocoapods-core (1.8.4)
      activesupport (>= 4.0.2, < 6)
      algoliasearch (~> 1.0)
      concurrent-ruby (~> 1.1)
      fuzzy_match (~> 2.0.4)
      nap (~> 1.0)
    cocoapods-deintegrate (1.0.4)
    cocoapods-downloader (1.3.0)
    cocoapods-plugins (1.0.0)
      nap
    cocoapods-search (1.0.0)
    cocoapods-stats (1.1.0)
    cocoapods-trunk (1.4.1)
      nap (>= 0.8, < 2.0)
      netrc (~> 0.11)
    cocoapods-try (1.1.0)
    colored (1.2)
    colored2 (3.1.2)
    commander-fastlane (4.4.6)
      highline (~> 1.7.2)
    concurrent-ruby (1.1.5)
    declarative (0.0.10)
    declarative-option (0.1.0)
    digest-crc (0.4.1)
    domain_name (0.5.20190701)
      unf (>= 0.0.5, < 1.0.0)
    dotenv (2.7.5)
    emoji_regex (1.0.1)
    escape (0.0.4)
    excon (0.71.1)
    faraday (0.17.3)
      multipart-post (>= 1.2, < 3)
    faraday-cookie_jar (0.0.6)
      faraday (>= 0.7.4)
      http-cookie (~> 1.0.0)
    faraday_middleware (0.13.1)
      faraday (>= 0.7.4, < 1.0)
    fastimage (2.1.7)
    fastlane (2.140.0)
      CFPropertyList (>= 2.3, < 4.0.0)
      addressable (>= 2.3, < 3.0.0)
      babosa (>= 1.0.2, < 2.0.0)
      bundler (>= 1.12.0, < 3.0.0)
      colored
      commander-fastlane (>= 4.4.6, < 5.0.0)
      dotenv (>= 2.1.1, < 3.0.0)
      emoji_regex (>= 0.1, < 2.0)
      excon (>= 0.71.0, < 1.0.0)
      faraday (~> 0.17)
      faraday-cookie_jar (~> 0.0.6)
      faraday_middleware (~> 0.13.1)
      fastimage (>= 2.1.0, < 3.0.0)
      gh_inspector (>= 1.1.2, < 2.0.0)
      google-api-client (>= 0.29.2, < 0.37.0)
      google-cloud-storage (>= 1.15.0, < 2.0.0)
      highline (>= 1.7.2, < 2.0.0)
      json (< 3.0.0)
      jwt (~> 2.1.0)
      mini_magick (>= 4.9.4, < 5.0.0)
      multi_xml (~> 0.5)
      multipart-post (~> 2.0.0)
      plist (>= 3.1.0, < 4.0.0)
      public_suffix (~> 2.0.0)
      rubyzip (>= 1.3.0, < 2.0.0)
      security (= 0.1.3)
      simctl (~> 1.6.3)
      slack-notifier (>= 2.0.0, < 3.0.0)
      terminal-notifier (>= 2.0.0, < 3.0.0)
      terminal-table (>= 1.4.5, < 2.0.0)
      tty-screen (>= 0.6.3, < 1.0.0)
      tty-spinner (>= 0.8.0, < 1.0.0)
      word_wrap (~> 1.0.0)
      xcodeproj (>= 1.13.0, < 2.0.0)
      xcpretty (~> 0.3.0)
      xcpretty-travis-formatter (>= 0.0.3)
    fastlane-plugin-flutter (0.3.19)
    fourflusher (2.3.1)
    fuzzy_match (2.0.4)
    gh_inspector (1.1.3)
    google-api-client (0.36.4)
      addressable (~> 2.5, >= 2.5.1)
      googleauth (~> 0.9)
      httpclient (>= 2.8.1, < 3.0)
      mini_mime (~> 1.0)
      representable (~> 3.0)
      retriable (>= 2.0, < 4.0)
      signet (~> 0.12)
    google-cloud-core (1.5.0)
      google-cloud-env (~> 1.0)
      google-cloud-errors (~> 1.0)
    google-cloud-env (1.3.0)
      faraday (~> 0.11)
    google-cloud-errors (1.0.0)
    google-cloud-storage (1.25.1)
      addressable (~> 2.5)
      digest-crc (~> 0.4)
      google-api-client (~> 0.33)
      google-cloud-core (~> 1.2)
      googleauth (~> 0.9)
      mini_mime (~> 1.0)
    googleauth (0.10.0)
      faraday (~> 0.12)
      jwt (>= 1.4, < 3.0)
      memoist (~> 0.16)
      multi_json (~> 1.11)
      os (>= 0.9, < 2.0)
      signet (~> 0.12)
    highline (1.7.10)
    http-cookie (1.0.3)
      domain_name (~> 0.5)
    httpclient (2.8.3)
    i18n (0.9.5)
      concurrent-ruby (~> 1.0)
    json (2.3.0)
    jwt (2.1.0)
    memoist (0.16.2)
    mini_magick (4.10.1)
    mini_mime (1.0.2)
    minitest (5.14.0)
    molinillo (0.6.6)
    multi_json (1.14.1)
    multi_xml (0.6.0)
    multipart-post (2.0.0)
    nanaimo (0.2.6)
    nap (1.1.0)
    naturally (2.2.0)
    netrc (0.11.0)
    os (1.0.1)
    plist (3.5.0)
    public_suffix (2.0.5)
    representable (3.0.4)
      declarative (< 0.1.0)
      declarative-option (< 0.2.0)
      uber (< 0.2.0)
    retriable (3.1.2)
    rouge (2.0.7)
    ruby-macho (1.4.0)
    rubyzip (1.3.0)
    security (0.1.3)
    signet (0.12.0)
      addressable (~> 2.3)
      faraday (~> 0.9)
      jwt (>= 1.5, < 3.0)
      multi_json (~> 1.10)
    simctl (1.6.7)
      CFPropertyList
      naturally
    slack-notifier (2.3.2)
    terminal-notifier (2.0.0)
    terminal-table (1.8.0)
      unicode-display_width (~> 1.1, >= 1.1.1)
    thread_safe (0.3.6)
    tty-cursor (0.7.1)
    tty-screen (0.7.0)
    tty-spinner (0.9.2)
      tty-cursor (~> 0.7)
    tzinfo (1.2.6)
      thread_safe (~> 0.1)
    uber (0.1.0)
    unf (0.1.4)
      unf_ext
    unf_ext (0.0.7.6)
    unicode-display_width (1.6.1)
    word_wrap (1.0.0)
    xcodeproj (1.14.0)
      CFPropertyList (>= 2.3.3, < 4.0)
      atomos (~> 0.1.3)
      claide (>= 1.0.2, < 2.0)
      colored2 (~> 3.1)
      nanaimo (~> 0.2.6)
    xcpretty (0.3.0)
      rouge (~> 2.0.7)
    xcpretty-travis-formatter (1.0.0)
      xcpretty (~> 0.2, >= 0.0.7)

PLATFORMS
  ruby

DEPENDENCIES
  cocoapods
  fastlane
  fastlane-plugin-flutter

BUNDLED WITH
   2.1.4

@thibaudgg
Copy link
Contributor

Hi, we are also seeing some random issue around typecasting and so far our debug journey brought us to this patch electric-feel@0dea53f, which allows PostGIS types to be registered before AR loads the additional types, which should ensure that the AR connection type_map cache is properly pre-populated. We saw that in some conditions AR will not lazy load correctly the custom PostGIS type and then set the wrong default type value (ActiveModel::Type::Value), this then broke the serialization.

Please feel free to give a try to this patch to see if it solves your problem, we will certainly create a proper PR with more details around the issue next week. AR will most likely also need a separate patch to fix the lazy loading of the type.

@kwent
Copy link
Author

kwent commented Feb 21, 2020

Ok i will test this branch and report back

@kwent
Copy link
Author

kwent commented Feb 22, 2020

@thibaudgg Looks like your patch works for me ! yay

@klaustopher
Copy link

# Gemfile
gem 'activerecord-postgis-adapter', '~> 6.0.0', github: 'electric-feel/activerecord-postgis-adapter', branch: '0dea53f7763a21da2159710edf4bc011a9f4ccb8'
$ bundle update activerecord-postgis-adapter

(...)
Using activerecord-postgis-adapter 6.0.0 from https://github.com/electric-feel/activerecord-postgis-adapter.git (at 0dea53f7763a21da2159710edf4bc011a9f4ccb8@0dea53f)
(...)

Result is, all RSpec tests are green ✅

Works for us! Great work @thibaudgg. Can we get this in a PR and get it merged here?

@thibaudgg
Copy link
Contributor

@klaustopher @kwent thanks for trying the patch out, I will for sure submit a PR soon.

thibaudgg added a commit to electric-feel/activerecord-postgis-adapter that referenced this issue Feb 24, 2020
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.
thibaudgg added a commit to electric-feel/activerecord-postgis-adapter that referenced this issue Feb 24, 2020
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.
thibaudgg added a commit to electric-feel/activerecord-postgis-adapter that referenced this issue Feb 24, 2020
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.
thibaudgg added a commit to electric-feel/activerecord-postgis-adapter that referenced this issue Feb 24, 2020
Since the Rails 6 upgrade, we started seeing some random PostGIS type
issues , debugging the issue 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.
thibaudgg added a commit to electric-feel/activerecord-postgis-adapter that referenced this issue Feb 24, 2020
Since the Rails 6 upgrade, we started seeing some random PostGIS
serialization/deserialization issues, debugging the issues shows that the
ActiveRecord connection `type_map` isn't initialized properly which leave
the custom `ActiveRecord::ConnectionAdapters::PostGIS::SpatialColumn`
with no mapping for its PostGIS OID.

Usually, Rails would add missing OID mappings when encountered, however,
in that particular code path, it does not happen and because of
the way `super` is done, that happens too late.
Another issue in Rails must also be fixed, but that's another story :)

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 other PostGIS OIDs. 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.
thibaudgg added a commit to electric-feel/activerecord-postgis-adapter that referenced this issue Feb 24, 2020
Since the Rails 6 upgrade, we started seeing some random PostGIS
serialization/deserialization issues, debugging the issues shows that the
ActiveRecord connection `type_map` isn't initialized properly.
It does not contain the PostGIS OID mappings that are retrieved in [`load_additional_types`](https://github.com/rgeo/activerecord-postgis-adapter/blob/cc0e3f53520feed0b43800d8ee86561ee03de413/lib/active_record/connection_adapters/postgis/schema_statements.rb#L87-L105).

Usually, Rails would add missing OID mappings when encountered, however,
there are code paths that go around this (See [`lookup_cast_type_from_column`](https://github.com/rails/rails/blob/15748f6a052d4df68b6acf66456c181b42d9fe25/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb#L78)).
This is most likely a bug in Rails internals.

This patch fixes the [`initialize_type_map`](https://github.com/rgeo/activerecord-postgis-adapter/blob/cc0e3f53520feed0b43800d8ee86561ee03de413/lib/active_record/connection_adapters/postgis/schema_statements.rb#L87-L105)
method by ensuring that the specific PostGIS types are registered before
calling `super` which will then call `load_additional_types` and registers
the other PostGIS OIDs. That way the custom column will be properly initialized
and be present in the `type_map` cache.

This patch should solve rgeo#307 and rgeo#308.
thibaudgg added a commit to electric-feel/activerecord-postgis-adapter that referenced this issue Feb 24, 2020
Since the Rails 6 upgrade, we started seeing some random PostGIS
serialization/deserialization issues, debugging the issues shows that the
ActiveRecord connection `type_map` isn't initialized properly.
It does not contain the PostGIS OID mappings that are retrieved in [`load_additional_types`](https://github.com/rgeo/activerecord-postgis-adapter/blob/cc0e3f53520feed0b43800d8ee86561ee03de413/lib/active_record/connection_adapters/postgis/schema_statements.rb#L87-L105).

Usually, Rails would add missing OID mappings when encountered, however,
there are code paths that go around this (See [`lookup_cast_type_from_column`](https://github.com/rails/rails/blob/15748f6a052d4df68b6acf66456c181b42d9fe25/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb#L78)).
This is most likely a bug in Rails internals.

This patch fixes the [`initialize_type_map`](https://github.com/rgeo/activerecord-postgis-adapter/blob/cc0e3f53520feed0b43800d8ee86561ee03de413/lib/active_record/connection_adapters/postgis/schema_statements.rb#L87-L105)
method by ensuring that the specific PostGIS types are registered before
calling `super` which will then call `load_additional_types` and registers
the other PostGIS OIDs.

This patch should solve rgeo#307 and rgeo#308.
thibaudgg added a commit to electric-feel/activerecord-postgis-adapter that referenced this issue Feb 24, 2020
Since the Rails 6 upgrade, we started seeing some random PostGIS
serialization/deserialization issues, debugging the issues shows that the
ActiveRecord connection `type_map` isn't initialized properly.
It does not contain the PostGIS OID type mappings after `initialize_type_map` has ended.

Usually, Rails would add missing OID mappings when encountered, however,
there are code paths that go around this (See [`lookup_cast_type_from_column`](https://github.com/rails/rails/blob/15748f6a052d4df68b6acf66456c181b42d9fe25/activerecord/lib/active_record/connection_adapters/postgresql/quoting.rb#L78)).
This is most likely a bug in Rails internals.

This patch fixes the [`initialize_type_map`](https://github.com/rgeo/activerecord-postgis-adapter/blob/cc0e3f53520feed0b43800d8ee86561ee03de413/lib/active_record/connection_adapters/postgis/schema_statements.rb#L87-L105) method by ensuring that
the specific PostGIS types are registered before calling `super` which
will then call [`load_additional_types`](https://github.com/rgeo/activerecord-postgis-adapter/blob/cc0e3f53520feed0b43800d8ee86561ee03de413/lib/active_record/connection_adapters/postgis/schema_statements.rb#L87-L105) and registers the other PostGIS OIDs.

This patch should solve rgeo#307 and rgeo#308.
@thibaudgg
Copy link
Contributor

PR #309 submitted!

@mjy
Copy link

mjy commented Aug 17, 2020

@keithdoggett I think we can close this issue now?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants