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

ActiveRecord 4.2 #145

Merged
merged 111 commits into from
Feb 10, 2015
Merged

ActiveRecord 4.2 #145

merged 111 commits into from
Feb 10, 2015

Conversation

teeparham
Copy link
Member

This branch is released as 3.0.0.beta1. Test with:

gem "activerecord-postgis-adapter", "3.0.0.beta1"

There are significant changes to the PostgreSQL adapter in ActiveRecord 4.2, and it is extremely cumbersome to support prior versions in a single code base. Therefore, instead of having conditional support for prior versions of ActiveRecord, this adapter will only support ActiveRecord 4.2 and later in the master branch and version 3.0.

activerecord-postgis-adapter version 2.x will continue to support ActiveRecord 4.0 and 4.1.

activerecord-postgis-adapter version 3.x will support ActiveRecord 4.2 and later.

PostgreSQLAdapter module was renamed to PostgreSQL:
see rails/rails@cea481699
* Register types in adapter by overriding #initialize_type_map
* Add #new_column override
* Rename oid_type to cast_type
Fix deprecation warning: “Calling `cached_attributes` is no longer
necessary. All attributes are cached.”
* Fix ActiveSupport::TestCase warnings
This was referenced Nov 3, 2014
@virtualfunction
Copy link

Certainly a good step in the right direction, things seem to sort of be working for me.

I'm having issues assigning to spatial collections, so assuming Location has a spatial point column called center Location.columns_hash['center'].spatial? is returning false. This seems to be because the type method is returning :string which in turn I assume is due to OID::Spatial extending from Type::String from your change 0283c64

Given I'm not familiar with the fine details of how this gem works or have a much of a GIS background, I've hacked my way round the above issues by using the following for OID::Spatial

def type
  :spatial
end

def type_cast(value, *args)
  return if value.blank?
  return value if value.is_a? RGeo::Feature::Geometry
  ::RGeo::WKRep::WKBParser.new(@factory_generator, support_ewkb: true).parse(value) rescue nil
end

I'll let someone manually review that. As you can tell in my case thing where failing because I was assigning an RGeo object directly rather than WKB. I assume we need to cater for WKT as well?

@virtualfunction
Copy link

Not sure if this is on your radar - One other thing in migrations I noticed table.column :boundary, :polygon, geographic: true was creating a point column instead. I'm guessing most geometry was defaulting to this. Haven't had a chance to find out why (I was lazy and hacked ALTER statement in my code as a short term workaround)

@teeparham
Copy link
Member Author

@virtualfunction thanks - that helped

Much of the functionality in the SpatialColumn class needs to be moved down to the OID::Spatial class. This will also likely require some changes in rgeo-activerecord. Changes coming ...

* Simplify comments
* Change base class from Type::String to Type::Value
* Move methods into OID::Spatial
* Add OID::Spatial#spatial?
* Fix SpatialColumn#spatial?
* Update #column signature to match parent
* Remove unnecessary module scope
* Remove unnecessary module scope
* Bump development version dependencies for rake, mocha
* The only types that will ever be passed to #type_to_sql are :geometry
and :geography
* Improve comments
* rgeo-activerecord 2.1 removes the spatial index schema hack
* Fix tests
AR-JDBC does not yet support AR 4.2
@teeparham
Copy link
Member Author

Tests are passing locally for me with ruby 2.1.5 (most of the time).

This is really close...

@teeparham teeparham force-pushed the activerecord42 branch 2 times, most recently from 4dc7c14 to 6ecacc4 Compare February 8, 2015 01:24
* Postgres 9.3 is latest, pre-installed with PostGIS 2.1
* Move separate config scripts into .travis.yml
* Remove dependence on RGeo::ActiveRecord::AdapterTestHelper
* Remove dependency on RGeo::ActiveRecord::AdapterTestHelper
* Remove dependency on RGeo::ActiveRecord::AdapterTestHelper
* Use SpatialModel
* Move model creation to private method #create_model
Not sure what the point of that method is, or if it’s this adapter’s
reponsibiliy. The tests fail on Travis & work locally.
@teeparham
Copy link
Member Author

@thibaudgg
Copy link
Contributor

@teeparham thanks a lot for the hard work, working great in our app! Do you plan a gem release soon? 💟

@teeparham
Copy link
Member Author

This branch is released as 3.0.0.beta1. Test with:

gem "activerecord-postgis-adapter", "3.0.0.beta1"

Unless there are major problems, I will be merging this into master soon and releasing 3.0.0.

@thibaudgg
Copy link
Contributor

3.0.0.beta1 works great for us, thanks!

@andrewhao
Copy link

I'm unclear to the internals of this adapter, but I think something may be off here:

In a migration:

class CreateTrackPoints < ActiveRecord::Migration
  def change
    create_table :track_points do |t|
      t.st_point :coordinate, geographic: true
    end
  end
end

Executing this writes out this into my schema.rb:

create_table "track_points", force: :cascade do |t|
  t.geography "coordinate", limit: {:srid=>4326, :type=>"point", :geographic=>true}
end

That's fine and well, but any time I attempt to run any rake command involving db:schema:load, I get the following error:

/opt/rubies/2.2.0/bin/ruby -I/opt/rubies/2.2.0/lib/ruby/gems/2.2.0/gems/rspec-core-3.1.7/lib:/opt/rubies/2.2.0/lib/ruby/gems/2.2.0/gems/rspec-support-3.1.2/lib /opt/rubies/2.2.0/
lib/ruby/gems/2.2.0/gems/rspec-core-3.1.7/exe/rspec --pattern spec/\*\*\{,/\*/\*\*\}/\*_spec.rb  --exclude-pattern spec/features/\*\*/\*_spec.rb
rake aborted!
ActiveRecord::StatementInvalid: PG::InvalidParameterValue: ERROR:  Invalid geometry type modifier: geography
LINE 1: ...ck_points" ("id" serial primary key, "coordinate" geometry(G...
                                                             ^
: CREATE TABLE "track_points" ("id" serial primary key, "coordinate" geometry(GEOGRAPHY,4326), "elevation" decimal, "heart_rate" integer, "time" timestamp, "track_id" integer)
/Users/andrewhao/workspace/velocitas-core/db/schema.rb:35:in `block in <top (required)>'
/Users/andrewhao/workspace/velocitas-core/db/schema.rb:14:in `<top (required)>'
-e:1:in `<main>'
PG::InvalidParameterValue: ERROR:  Invalid geometry type modifier: geography
LINE 1: ...ck_points" ("id" serial primary key, "coordinate" geometry(G...
                                                             ^
/Users/andrewhao/workspace/velocitas-core/db/schema.rb:35:in `block in <top (required)>'
/Users/andrewhao/workspace/velocitas-core/db/schema.rb:14:in `<top (required)>'
-e:1:in `<main>'
Tasks: TOP => db:test:load_schema
(See full trace by running task with --trace)

Would it make sense to have the adapter write out the following statement instead into schema.rb?

+    t.st_point "coordinate", :geographic=>true
-    t.geography "coordinate", limit: {:srid=>4326, :type=>"point", :geographic=>true}

Doing so seems to fix the problem.

@teeparham
Copy link
Member Author

I'm merging this into master, although it's not quite ready for a 3.0.0 release yet. Please open bugs (like the one above) as individual issues.

teeparham added a commit that referenced this pull request Feb 10, 2015
@teeparham teeparham merged commit 2b36642 into master Feb 10, 2015
@andrewhao
Copy link

Will do.

@erickreutz
Copy link

@averas I'm running into that same issue. Did you figure anything out for it?

Could not log "sql.active_record" event. NoMethodError: private method `binary?' called for #<ActiveRecord::ConnectionAdapters::PostGISAdapter::OID::Spatial:0x007fa5cf54a4f8>

@teeparham teeparham deleted the activerecord42 branch February 28, 2015 20:59
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

Successfully merging this pull request may close these issues.

None yet