Skip to content

Commit e7df02c

Browse files
waratumanclaude
andcommitted
Fix geometry type: perf, nil-limit crash, adapter pollution, and DDL hardening
- Build the RGeo factory, WKB/WKT parsers, and WKB generator once in the Geometry type's initializer instead of rebuilding them on every cast/ serialize. A GEOS factory was previously allocated per value read; it is invariant per column. Eager construction (vs. lazy memoization) is also required because AR freezes Type instances after initialization. - Guard the nil-limit path: a bare `geometry` column (no type/srid modifier) has no limit, so `limit[:srid]` raised NoMethodError on read. Add a `srid` helper defaulting to 0 (PostGIS "unknown SRID"). - type_to_sql: coerce srid with `to_i` so it cannot carry arbitrary text into the geometry() type modifier. - Use `merge` instead of `merge!` for NATIVE_DATABASE_TYPES so the parent PostgreSQLAdapter's type map is not mutated with :geometry. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 5796ece commit e7df02c

3 files changed

Lines changed: 20 additions & 7 deletions

File tree

lib/active_record/connection_adapters/postgis/oid/geometry.rb

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,19 @@ module OID
88
class Geometry < Type::Value
99
include ActiveModel::Type::Helpers::Mutable
1010

11+
# Build the RGeo factory, parsers and generator once, up front. The
12+
# SRID is fixed for the column, so these are invariant for the life of
13+
# the type instance. Doing it here (rather than lazily on each cast)
14+
# avoids rebuilding a GEOS factory on every value read, and is
15+
# required because Type instances are frozen after initialization.
16+
def initialize(...)
17+
super
18+
@rgeo_factory = RGeo::Geos.factory(srid: srid)
19+
@wkb_parser = RGeo::WKRep::WKBParser.new(@rgeo_factory, support_ewkb: true, default_srid: srid)
20+
@wkt_parser = RGeo::WKRep::WKTParser.new(@rgeo_factory, support_ewkt: true, default_srid: srid)
21+
@wkb_generator = RGeo::WKRep::WKBGenerator.new(hex_format: true, type_format: :ewkb, emit_ewkb_srid: true)
22+
end
23+
1124
def type
1225
:geometry
1326
end
@@ -20,9 +33,9 @@ def cast(value)
2033
value
2134
when ::String # HEXEWKB
2235
if value[0,1] == "\x00" || value[0,1] == "\x01" || value[0,4] =~ /[0-9a-fA-F]{4}/
23-
RGeo::WKRep::WKBParser.new(rgeo_factory_generator, support_ewkb: true, default_srid: limit[:srid]).parse(value)
36+
@wkb_parser.parse(value)
2437
else
25-
RGeo::WKRep::WKTParser.new(rgeo_factory_generator, support_ewkt: true, default_srid: limit[:srid]).parse(value)
38+
@wkt_parser.parse(value)
2639
end
2740
else
2841
raise ArgumentError, "Unsupported argument type (#{value.class})"
@@ -32,7 +45,7 @@ def cast(value)
3245
def serialize(value)
3346
case value
3447
when RGeo::Feature::Instance
35-
::RGeo::WKRep::WKBGenerator.new(hex_format: true, type_format: :ewkb, emit_ewkb_srid: true).generate(value)
48+
@wkb_generator.generate(value)
3649
else
3750
value
3851
end
@@ -44,8 +57,8 @@ def changed_in_place?(raw_old_value, new_value)
4457

4558
private
4659

47-
def rgeo_factory_generator
48-
RGeo::Geos.factory(srid: limit[:srid].to_i)
60+
def srid
61+
limit ? limit[:srid].to_i : 0
4962
end
5063

5164
end

lib/active_record/connection_adapters/postgis/schema_statements.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ def type_to_sql(sql_type, limit: nil, precision: nil, scale: nil, array: nil, ty
1414
type ||= 'Geometry'
1515
srid ||= 4326
1616
end
17-
"geometry(#{type},#{srid})"
17+
"geometry(#{type},#{srid.to_i})"
1818
else
1919
super
2020
end

lib/active_record/connection_adapters/postgis_adapter.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ module ConnectionAdapters
1212
class PostGISAdapter < PostgreSQLAdapter
1313
ADAPTER_NAME = 'PostGIS'.freeze
1414

15-
NATIVE_DATABASE_TYPES = PostgreSQLAdapter::NATIVE_DATABASE_TYPES.merge!({
15+
NATIVE_DATABASE_TYPES = PostgreSQLAdapter::NATIVE_DATABASE_TYPES.merge({
1616
geometry: { name: "geometry" },
1717
})
1818

0 commit comments

Comments
 (0)