You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
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.
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.
The text was updated successfully, but these errors were encountered:
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 theWKBGenerator
to be written to the db. When spatial columns are being read, theSpatialFactoryStore
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 andRGeo::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 seeRGeo::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
andST_IsValid
returns true, but an error still occurs when reading. This is due toRGeo::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 theSpatialFactoryStore
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.
The text was updated successfully, but these errors were encountered: