-
-
Notifications
You must be signed in to change notification settings - Fork 252
Description
Description
Currently, the adapter will accept either RGeo objects or WKT strings when writing to a spatial column. If a WKT string is passed in, the column's factory is determined by RGeo::ActiveRecord::SpatialFactoryStore
, but if an RGeo object is passed in, this is bypassed and the object is sent to the WKBGenerator
to be written to the db. When spatial columns are being read, the SpatialFactoryStore
determines the factory to parse the WKB from the db into an RGeo object.
This can cause problems, particularly when geographies are being stored, since RGeo::Geographic
factories will raise errors while parsing invalid geometries and RGeo::Geos::CAPIFactory
will not. This leads to situations where users are able to successfully write geometries but when they try to access certain rows, they see RGeo::Error::InvalidGeometry (LinearRing failed ring test)
or some other issue.
Even more confusing can be when geographic columns are used and valid geometries are created with RGeo::Geos::CAPIFactory
and ST_IsValid
returns true, but an error still occurs when reading. This is due to RGeo::Geographic.spherical_factory
sometimes erroneously raising errors on valid geometries (rgeo/rgeo#212), which is another issue that needs to be looked at.
This seems to be the cause of #202 #235 #286 #291 and possibly #312
I can open a PR with failing test cases demonstrating the above if that's helpful.
Proposal
Add some sort of "type checking" when data is being written into the db and raise an error if the factory of the RGeo object doesn't match the factory from the SpatialFactoryStore
. If that is too strict, perhaps just comparing the "family" of factories could be sufficient (i.e. RGeo::Geos
, RGeo::Geographic
, etc.). Another possibility is to cast all geometries to the SpatialFactoryStore
regardless of the input factory.
I'd like to get feedback from people and hear if this would be helpful and if so, how you'd like the "type checking" work.