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

support rails 7.1.0 #382

Merged
merged 4 commits into from
Oct 6, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ jobs:
pg: [11-3.0, 12-master, 13-master, 14-master, 15-master]
steps:
- name: Set Up Actions
uses: actions/checkout@v2
uses: actions/checkout@v4
- name: Install GEOS
run: sudo apt-get install libgeos-dev
- name: Set Up Ruby
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,4 @@ Gemfile.lock
.idea
debug.log
/test/db/*
.env
BuonOmo marked this conversation as resolved.
Show resolved Hide resolved
2 changes: 0 additions & 2 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -33,10 +33,8 @@ end

# Need to install for tests
gem "rails", github: "rails/rails", tag: "v#{activerecord_version}"

group :development do
# Gems used by the ActiveRecord test suite
gem "bcrypt"
gem "mocha"
gem "sqlite3"
end
4 changes: 2 additions & 2 deletions activerecord-postgis-adapter.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,13 @@ Gem::Specification.new do |spec|

spec.required_ruby_version = ">= 2.7.0"

spec.add_dependency "activerecord", "~> 7.0.0"
spec.add_dependency "activerecord", "~> 7.1.beta1"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
spec.add_dependency "activerecord", "~> 7.1.beta1"
spec.add_dependency "activerecord", ">= 7.0", "< 7.2"

we still do support AR 7.0.8 don't we? @keithdoggett I don't know how we handled that in the past but it seems to me that we are quite strict, aren't we?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We bumped major version at every AR major version. It on the readme.

Also API are not compatible fully.

Copy link
Member

@BuonOmo BuonOmo Oct 1, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Two things though:

  • this is not a major version
  • shouldn't we care about compatibility for our users ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In rails, that a major version .
7.0 to 7.1 is major since it has breaking changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The user api in this adapter is unchanged.

Users can updated it freely and the others small changes were just throwing warnings in ruby 3.2

spec.add_dependency "rgeo-activerecord", "~> 7.0.0"

spec.add_development_dependency "rake", "~> 13.0"
spec.add_development_dependency "minitest", "~> 5.4"
spec.add_development_dependency "mocha", "~> 1.1"
spec.add_development_dependency "benchmark-ips", "~> 2.9.1"
spec.add_development_dependency "benchmark-ips", "~> 2.12"
spec.add_development_dependency "rubocop", "~> 1.50"

spec.metadata = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ module ActiveRecord
module ConnectionAdapters
module PostGIS
module ColumnMethods

def spatial(name, options = {})
raise "You must set a type. For example: 't.spatial type: :st_point'" unless options[:type]
column(name, options[:type], **options)
Expand Down Expand Up @@ -44,6 +45,11 @@ def st_point(name, options = {})
def st_polygon(name, options = {})
column(name, :st_polygon, **options)
end

private
def valid_column_definition_options
super + [:srid, :has_z, :has_m, :geographic, :spatial_type]
end
end
end

Expand Down
5 changes: 4 additions & 1 deletion lib/active_record/connection_adapters/postgis/oid/spatial.rb
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,10 @@ def initialize(geo_type: "geometry", srid: 0, has_z: false, has_m: false, geogra
# has_z: false
# has_m: false
def self.parse_sql_type(sql_type)
geo_type, srid, has_z, has_m = nil, 0, false, false
geo_type = nil
seuros marked this conversation as resolved.
Show resolved Hide resolved
srid = 0
has_z = false
has_m = false

if sql_type =~ /(geography|geometry)\((.*)\)$/i
# geometry(Point)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ module SchemaStatements
# override
# https://github.com/rails/rails/blob/7-0-stable/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb#L662
# Create a SpatialColumn instead of a PostgreSQL::Column
def new_column_from_field(table_name, field)
def new_column_from_field(table_name, field, _definitions)
seuros marked this conversation as resolved.
Show resolved Hide resolved
column_name, type, default, notnull, oid, fmod, collation, comment, attgenerated = field
type_metadata = fetch_type_metadata(column_name, type, oid.to_i, fmod.to_i)
default_value = extract_value_from_default(default)
default_function = extract_default_function(default_value, default)

if match = default_function&.match(/\Anextval\('"?(?<sequence_name>.+_(?<suffix>seq\d*))"?'::regclass\)\z/)
if (match = default_function&.match(/\Anextval\('"?(?<sequence_name>.+_(?<suffix>seq\d*))"?'::regclass\)\z/))
seuros marked this conversation as resolved.
Show resolved Hide resolved
serial = sequence_name_from_parts(table_name, column_name, match[:suffix]) == match[:sequence_name]
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,11 +43,11 @@ def geo_type(type = "GEOMETRY")
end

def limit_from_options(type, options = {})
spatial_type = geo_type(type)
spatial_type << "Z" if options[:has_z]
spatial_type << "M" if options[:has_m]
spatial_type << ",#{options[:srid] || default_srid(options)}"
spatial_type
has_z = options[:has_z] ? 'Z' : ''
has_m = options[:has_m] ? 'M' : ''
srid = options[:srid] || default_srid(options)
field_type = [geo_type(type), has_z, has_m].compact.join
"#{field_type},#{srid}"
BuonOmo marked this conversation as resolved.
Show resolved Hide resolved
end

def default_srid(options)
Expand Down
2 changes: 1 addition & 1 deletion lib/active_record/connection_adapters/postgis/version.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
module ActiveRecord
module ConnectionAdapters
module PostGIS
VERSION = "8.0.2"
VERSION = "9.0.0"
seuros marked this conversation as resolved.
Show resolved Hide resolved
end
end
end
24 changes: 12 additions & 12 deletions lib/active_record/connection_adapters/postgis_adapter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,18 +9,18 @@

require "active_record/connection_adapters"
require "active_record/connection_adapters/postgresql_adapter"
require "active_record/connection_adapters/postgis/version"
require "active_record/connection_adapters/postgis/column_methods"
require "active_record/connection_adapters/postgis/schema_statements"
require "active_record/connection_adapters/postgis/database_statements"
require "active_record/connection_adapters/postgis/spatial_column_info"
require "active_record/connection_adapters/postgis/spatial_table_definition"
require "active_record/connection_adapters/postgis/spatial_column"
require "active_record/connection_adapters/postgis/arel_tosql"
require "active_record/connection_adapters/postgis/oid/spatial"
require "active_record/connection_adapters/postgis/oid/date_time"
require "active_record/connection_adapters/postgis/type" # has to be after oid/*
require "active_record/connection_adapters/postgis/create_connection"
require_relative "postgis/version"
require_relative "postgis/column_methods"
require_relative "postgis/schema_statements"
require_relative "postgis/database_statements"
require_relative "postgis/spatial_column_info"
require_relative "postgis/spatial_table_definition"
require_relative "postgis/spatial_column"
require_relative "postgis/arel_tosql"
require_relative "postgis/oid/spatial"
require_relative "postgis/oid/date_time"
require_relative "postgis/type" # has to be after oid/*
require_relative "postgis/create_connection"
# :startdoc:

module ActiveRecord
Expand Down
2 changes: 1 addition & 1 deletion lib/activerecord-postgis-adapter.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
# frozen_string_literal: true

require "active_record/connection_adapters/postgis_adapter"
require_relative "active_record/connection_adapters/postgis_adapter"
4 changes: 2 additions & 2 deletions test/cases/setup_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@
module PostGIS
class SpatialQueriesTest < ActiveSupport::TestCase
def test_ignore_tables
expect_to_ignore = %w(
expect_to_ignore = %w[
geography_columns
geometry_columns
layer
raster_columns
raster_overviews
spatial_ref_sys
topology
)
]
assert_equal expect_to_ignore, ::ActiveRecord::SchemaDumper.ignore_tables
end
end
Expand Down
1 change: 0 additions & 1 deletion test/test_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

require "bundler/setup"
Bundler.require :development

require "minitest/autorun"
require "minitest/pride"
require "mocha/minitest"
Expand Down