Error TypeError: can't cast RGeo::Geographic::SphericalPolygonImpl on Rails 7.2 and HEAD activerecord-postgis-adapter

[stevegeek]

The following reproduction script shows the error.

It works on Rails 7.0/7.1 with the latest release of activerecord-postgis-adapter but fails when run with Rails 7.2 and master branch of activerecord-postgis-adapter

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"

  gem "rails", "~> 7.2"
  # If you want to test against edge Rails replace the previous line with this:
  # gem "rails", github: "rails/rails", branch: "main"

  gem "pg"
  gem "activerecord-postgis-adapter", github: "rgeo/activerecord-postgis-adapter", branch: "master"
  gem "rgeo-activerecord", "~> 7"
end

require "active_record"
require "minitest/autorun"
require "logger"


# `createdb postgis_test_db`

# This connection will do for database-independent bug reports.
ActiveRecord::Base.establish_connection(
  "postgis://localhost/postgis_test_db"
)
ActiveRecord::Base.logger = Logger.new(STDOUT)

ActiveRecord::Schema.define do
  enable_extension 'postgis'

  create_table :posts, force: true do |t|
    t.st_point :geo
  end
end

class Post < ActiveRecord::Base
end

class BugTest < Minitest::Test
  def test_geo_where
    post = Post.create!(geo: RGeo::Geos.factory(srid: 4326).point(-122, 47))
    point = post.geo
    geo_factory = RGeo::Geographic.spherical_factory(srid: 4326, buffer_resolution: 4)
    area_center = geo_factory.point(*point.coordinates)
    circle = area_center.buffer(10_000)
    polygon = geo_factory.multi_polygon([circle])
    assert_equal 1, Post.where("ST_DWithin(?, posts.geo, ?)", polygon, 1).count
  end
end

Exception:

TypeError: can't cast RGeo::Geographic::SphericalPolygonImpl
    /Users/stephen/.asdf/installs/ruby/3.3.4/lib/ruby/gems/3.3.0/gems/activerecord-7.2.0/lib/active_record/connection_adapters/abstract/quoting.rb:105:in `type_cast'
    /Users/stephen/.asdf/installs/ruby/3.3.4/lib/ruby/gems/3.3.0/gems/activerecord-7.2.0/lib/active_record/connection_adapters/postgresql/quoting.rb:185:in `type_cast'
    /Users/stephen/.asdf/installs/ruby/3.3.4/lib/ruby/gems/3.3.0/gems/activerecord-7.2.0/lib/active_record/connection_adapters/abstract/quoting.rb:229:in `block in type_casted_binds'
    /Users/stephen/.asdf/installs/ruby/3.3.4/lib/ruby/gems/3.3.0/gems/activerecord-7.2.0/lib/active_record/connection_adapters/abstract/quoting.rb:225:in `map'
    /Users/stephen/.asdf/installs/ruby/3.3.4/lib/ruby/gems/3.3.0/gems/activerecord-7.2.0/lib/active_record/connection_adapters/abstract/quoting.rb:225:in `type_casted_binds'
...

Thanks for all the work on the postgis adapter for Rails!

[DanielJackson-Oslo]

Same error here. Downgrading to Rails 7.1 in the meantime!

Thank you guys for all the help, and let me know if I can chip in with any money to help you free up time for this.

[JamesChevalier]

I'm getting a slightly different can't cast RGeo::Geographic::SphericalLineStringImpl error in my test suite when running it against Rails 7.2.1. I looked into it a little bit, and was able to get a little more detail - hopefully it's helpful.

The query it's running when it's hitting this error is:

Node.where("ST_DWITHIN(geog, ?, 25)", geog)

If I adjust that query to this unsafe version, it does work:

Node.where("ST_DWITHIN(geog, '#{geog}', 25)")

[StoneGod]

I have not this problem on rails 7.2.1

test.log

[jnweaver]

@JamesChevalier I have a similar experience in terms of an unsafe version of query working, but throwing an error when using the safe version. Also Rails 7.2.1.

Works:

details.common.where("ST_CONTAINS(way, st_transform(GeomFromEWKT('srid=4326;POINT(#{lng.to_f}  #{lat.to_f})'), 3857))")

Errors:

details.common.where("ST_CONTAINS(way, st_transform(GeomFromEWKT('srid=4326;POINT(:lng  :lat)'), 3857))", {lng: lng.to_f, lat: lat.to_f})

Throws: ActiveRecord::StatementInvalid: PG::IndeterminateDatatype: ERROR: could not determine data type of parameter $1

[JamesChevalier]

Some additional details that 🤷 may or may not be helpful...

My initializer:

RGEO_FACTORY = RGeo::Geographic.spherical_factory(srid: 4326, uses_lenient_assertions: true)

RGeo::ActiveRecord::SpatialFactoryStore.instance.tap do |config|
  config.default = RGEO_FACTORY
end

I tested with simple_mercator_factory and that also produced the same error.
I tested against the bump-7-2 branch from #405 and that also produced the same error.

[BuonOmo]

@JamesChevalier do you have a full one file example tested against bump-7-2? That would be pretty useful! (here's the activerecord template)

[JamesChevalier]

Thanks for the suggestion, @BuonOmo

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"
  git_source(:github) { |repo| "https://github.com/#{repo}.git" }
  gem "rails", "~> 7.2"
  gem "pg"
  gem "activerecord-postgis-adapter", github: "rgeo/activerecord-postgis-adapter", branch: "bump-7-2"
end

require "active_record"
require "minitest/autorun"
require "logger"

ActiveRecord::Base.logger = Logger.new(STDOUT)
ActiveRecord::Base.establish_connection("postgis://localhost/postgis_test_db")
ActiveRecord::Schema.define do
  enable_extension "postgis"

  create_table :posts, force: true do |t|
    t.geometry :geo, geographic: true
  end
end

class Post < ActiveRecord::Base
end

class BugTest < Minitest::Test
  def test_geo_where
    post = Post.create!(geo: RGeo::Geographic.spherical_factory(srid: 4326, uses_lenient_assertions: true).point(-122, 47))
    assert_equal 1, Post.where("ST_DWITHIN(geog, ?, 25)", post.geo).count
  end
end

[BuonOmo]

Thank you! It might actually be related to the failing tests that we currently have and are blocking us from releasing this branch. I'll hopefully have a look within the next two weeks.

[jnweaver]

@BuonOmo If helpful, here's another failing one file example. This is throwing ActiveRecord::StatementInvalid: PG::IndeterminateDatatype: ERROR: could not determine data type of parameter $1

Using an unsafe query instead -- commented out after the failing assertion -- passes.

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"
  git_source(:github) { |repo| "https://github.com/#{repo}.git" }
  gem "rails", "~> 7.2"
  gem "pg"
  gem "activerecord-postgis-adapter", github: "rgeo/activerecord-postgis-adapter", branch: "bump-7-2"
end

require "active_record"
require "minitest/autorun"
require "logger"

ActiveRecord::Base.logger = Logger.new(STDOUT)
ActiveRecord::Base.establish_connection("postgis://localhost/postgis_test_db")
ActiveRecord::Schema.define do
  enable_extension "postgis"

  create_table :posts, force: true do |t|
    t.geometry :geo, limit: {srid: 3857, type: "geometry"}
  end
end

class Post < ActiveRecord::Base
end

class BugTest < Minitest::Test
  def test_geo_where
    post = Post.create!(geo: "POLYGON ((-9953167.702116467 5323537.15351723, -9953167.702116467 5323635.125782488, -9953041.625807118 5323635.125782488, -9953041.625807118 5323537.15351723, -9953167.702116467 5323537.15351723))")
		post_with_center = Post.select("ST_Y(ST_centroid(ST_transform(geo,4326))) as lat, ST_X(ST_centroid(ST_transform(geo,4326))) as lng").first
		lng = post_with_center.lng
		lat = post_with_center.lat
    assert_equal 1, Post.where("ST_CONTAINS(geo, st_transform(GeomFromEWKT('srid=4326;POINT(?  ?)'), 3857))", lng, lat).count
    # assert_equal 1, Post.where("ST_CONTAINS(geo, st_transform(GeomFromEWKT('srid=4326;POINT(#{lng} #{lat})'), 3857))").count
  end
end

[BuonOmo]

So it looks like this was already failing before 7.2. @jnweaver do you have a timing in history where it was not failing ? As of now the failure makes a lot of sense, there is not any overriding of type casting at all, so the code Post.where(query, bindings) never goes through this codebase!

@seuros @keithdoggett any idea of history related to this issue ?

Here's a diff that would fix the issue:

diff --git a/lib/active_record/connection_adapters/postgis/quoting.rb b/lib/active_record/connection_adapters/postgis/quoting.rb
new file mode 100644
index 0000000..1d7c8b9
--- /dev/null
+++ b/lib/active_record/connection_adapters/postgis/quoting.rb
@@ -0,0 +1,18 @@
+# frozen_string_literal: true
+
+module ActiveRecord
+  module ConnectionAdapters
+    module PostGIS
+      module Quoting
+        def type_cast(value)
+          case value
+          when RGeo::Feature::Instance
+            value.to_s
+          else
+            super
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/lib/active_record/connection_adapters/postgis_adapter.rb b/lib/active_record/connection_adapters/postgis_adapter.rb
index 9b890d5..7b80558 100644
--- a/lib/active_record/connection_adapters/postgis_adapter.rb
+++ b/lib/active_record/connection_adapters/postgis_adapter.rb
@@ -16,6 +16,7 @@ require_relative "postgis/spatial_column"
 require_relative "postgis/arel_tosql"
 require_relative "postgis/oid/spatial"
 require_relative "postgis/oid/date_time"
+require_relative "postgis/quoting"
 require_relative "postgis/type" # has to be after oid/*
 # :startdoc:
 
@@ -42,6 +43,7 @@ module ActiveRecord
       # http://postgis.17.x6.nabble.com/Default-SRID-td5001115.html
       DEFAULT_SRID = 0
 
+      include PostGIS::Quoting
       include PostGIS::SchemaStatements
       include PostGIS::DatabaseStatements
 

But I don't see time to test this now, and it does not change anything with the actual PR so I would not prioritize it. In the mean time, you can just #to_s and still be safe :)

[jnweaver]

Thanks @BuonOmo

I may not understand what you are asking, but this was not failing with Rails 7.1 and activerecord-postgis-adapter 9.0.2

# frozen_string_literal: true

require "bundler/inline"

gemfile(true) do
  source "https://rubygems.org"
  git_source(:github) { |repo| "https://github.com/#{repo}.git" }
  gem "rails", "7.1.3.4"
  gem "pg"
  gem "activerecord-postgis-adapter", "9.0.2"
end

require "active_record"
require "minitest/autorun"
require "logger"

ActiveRecord::Base.logger = Logger.new(STDOUT)
ActiveRecord::Base.establish_connection("postgis://localhost/postgis_test_db")
ActiveRecord::Schema.define do
  enable_extension "postgis"

  create_table :posts, force: true do |t|
    t.geometry :geo, limit: {srid: 3857, type: "geometry"}
  end
end

class Post < ActiveRecord::Base
end

class BugTest < Minitest::Test
  def test_geo_where
    post = Post.create!(geo: "POLYGON ((-9953167.702116467 5323537.15351723, -9953167.702116467 5323635.125782488, -9953041.625807118 5323635.125782488, -9953041.625807118 5323537.15351723, -9953167.702116467 5323537.15351723))")
		post_with_center = Post.select("ST_Y(ST_centroid(ST_transform(geo,4326))) as lat, ST_X(ST_centroid(ST_transform(geo,4326))) as lng").first
		lng = post_with_center.lng
		lat = post_with_center.lat
    assert_equal 1, Post.where("ST_CONTAINS(geo, st_transform(GeomFromEWKT('srid=4326;POINT(?  ?)'), 3857))", lng, lat).count
    # assert_equal 1, Post.where("ST_CONTAINS(geo, st_transform(GeomFromEWKT('srid=4326;POINT(#{lng} #{lat})'), 3857))").count
  end
end

At any rate, I can definitely work around this when a version supporting Rails 7.2 is available.

[BuonOmo]

@jnweaver if this induces a bug in 7.2 (which we see it does thanks to your last comment!), then a version supporting rails 7.2 depends on this!

It is taking a lot of my personnal time, when I'll be able to make some money out of my OSS projects I might be able to contribute faster! In the mean time, could you help now ?

I've been working on the difference between the two versions (basically running a debugger and checking the stacktrace after #build_where_clause using the irb command u /build_where_clause/). And it seems that is where things change.

I think the culprit commit in rails is rails/rails@8e6a5de. Would you (or anyone) have time to investigate further?

[JamesChevalier]

I had a few minutes to dig into things - here are some details, in case they're helpful (apologies if I'm sharing information you already know).

activerecord v7.2 introduced parts = [build_bound_sql_literal(opts, rest)]
Previously, the query would use parts = [klass.sanitize_sql(rest.empty? ? opts : [opts, *rest])]

So, for a query in the format of:

YourModel.where("ST_DWITHIN(geog, ?, 25)", geog)

The old version of this code would produce output like:

["ST_DWITHIN(geog, 'STRING_VERSION_OF_GEO_DATA', 25)"]

Whereas the new version of this code produces output like:

[#<Arel::Nodes::BoundSqlLiteral "(ST_DWITHIN(geog, ?, 25))" [#<RGeo::Geographic::SphericalLineStringImpl:0x3d554 "LINESTRING (ALL, THE, COORDINATES)">]>]

I don't think it's super relevant, but when the code flows through the build_bound_sql_literal method, it goes into the else branch where value is passed along without modification because value.respond_to?(:id_for_database) is false.

[JamesChevalier]

Your PostGIS::Quoting diff, above, does resolve the issue. It makes sense, as well, since the thing that's missing in Rails 7.2+ is the sql sanitization that was once done via parts = [klass.sanitize_sql(rest.empty? ? opts : [opts, *rest])]

The sanitize_sql code flows like:

• sanitize_sql calls sanitize_sql_for_conditions
• which in turn calls sanitize_sql_array
• and since the statement includes ? it calls replace_bind_variables
• which then flows through replace_bind_variable and into quote_bound_value (again because of the ?)
• finally calling connection.quote(connection.cast_bound_value(value))

That last step is where we land on the logic of the commit that you linked to. That commit was specifically (if I'm understanding correctly) removing the use of connection in too many places. Your use of to_s falls in line with the spirit of the commit, in that it's also not relying on connection.

One thing I don't know yet is whether when RGeo::Feature::Instance catches all possible cases of to_s being needed. Based on what I've seen so far (if I'm understanding things correctly), I'd answer the question of "In which cases does activerecord-postgis-adapter need to self-sanitize?" with "All of them."

This is the pull request that goes with that commit: rails/rails#51139

UPDATE: I notice that quote is already defined at https://github.com/rgeo/activerecord-postgis-adapter/blob/master/lib/active_record/connection_adapters/postgis_adapter.rb#L121-L129 and since I don't have experience at this low level I'm unsure if that is/will be an issue