-
Notifications
You must be signed in to change notification settings - Fork 234
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
Error TypeError: can't cast RGeo::Geographic::SphericalPolygonImpl
on Rails 7.2 and HEAD activerecord-postgis-adapter
#402
Comments
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. |
I'm getting a slightly different The query it's running when it's hitting this error is:
If I adjust that query to this unsafe version, it does work:
|
I have not this problem on rails 7.2.1 |
@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:
Errors:
Throws: |
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 |
@JamesChevalier do you have a full one file example tested against |
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 |
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. |
@BuonOmo If helpful, here's another failing one file example. This is throwing 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 |
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 @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 |
Thanks @BuonOmo I may not understand what you are asking, but this was not failing with # 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. |
@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 I think the culprit commit in rails is rails/rails@8e6a5de. Would you (or anyone) have time to investigate further? |
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)] 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:
Whereas the new version of this code produces output like:
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 |
Your The
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 One thing I don't know yet is whether This is the pull request that goes with that commit: rails/rails#51139 UPDATE: I notice that |
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 ofactiverecord-postgis-adapter
Exception:
Thanks for all the work on the postgis adapter for Rails!
The text was updated successfully, but these errors were encountered: