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

Z dimension not saved in db unless X or Y changed #364

Open
pcboy opened this issue May 17, 2022 · 5 comments
Open

Z dimension not saved in db unless X or Y changed #364

pcboy opened this issue May 17, 2022 · 5 comments

Comments

@pcboy
Copy link

pcboy commented May 17, 2022

I have a 3D point set in my database.

:lnglat => #<RGeo::Geographic::SphericalPointImpl:0x13f38 "POINT (99.8575 9.243055556 0.0)">,

If I try to manually change the Z only.

:lnglat => #<RGeo::Geographic::SphericalPointImpl:0x14104 "POINT (99.8575 9.243055556 317.0473937988281)">,

I can see it correctly in memory.
But then if I save! my object, nothing is being saved (I can't see any SQL query being made, and no exception triggered).

But now if I change the Y or the X, everything gets saved:

lnglat = RGeo::Geographic.spherical_factory(srid: 4326, has_z_coordinate: true).point(99.8575, 8.243055556, 42)
2.7.3 :016 > sensor.save
D, [2022-05-17T13:51:12.809218 #467861] DEBUG -- :   TRANSACTION (178.5ms)  BEGIN
D, [2022-05-17T13:51:12.986207 #467861] DEBUG -- :   DailySensor Update (176.3ms)  UPDATE "sensors" SET "lnglat" = $1, "updated_at" = $2 WHERE "sensors"."id" = $3  [["lnglat", "00a0000001000010e64058f6e147ae147b40207c71c720431f4045000000000000"], ["updated_at", "2022-05-17 04:51:12.629805"], ["id", "7cfef318-6913-4fb6-b762-639990480d93"]]
D, [2022-05-17T13:51:13.164166 #467861] DEBUG -- :   TRANSACTION (176.7ms)  COMMIT
 => true 

In my schema dump I have:

 lnglat public.geography(PointZ,4326) NOT NULL,

In my rgeo initializer I have:

RGeo::ActiveRecord::SpatialFactoryStore.instance.tap do |config|
  config.register(
    RGeo::Geographic.spherical_factory(srid: 4326, uses_lenient_assertions: true,
                                       has_z_coordinate: true),
    geo_type: 'point',
    has_z: true
  )

  # By default, use the GEOS implementation for spatial columns.
  config.default = RGeo::Geographic.spherical_factory(srid: 4326, uses_lenient_assertions: true)
end

I'm on:

  • Rails 6.1.3.1
  • activerecord-postgis-adapter 7.1.1

Did I miss something obvious? The Z coordinate seem to never be saved unless X or Y changed.

EDIT: If I use ActiveModel::Dirty and manually set my lnglat attribute as dirty with lnglat_will_change! after setting the elevation, lnglat always gets saved properly. So it seems setting the Z value is failing the dirtiness check for some reason.

@keithdoggett
Copy link
Member

Thanks for the detailed write-up. I'll start looking into this soon.

@keithdoggett
Copy link
Member

@pcboy I looked into this and the reason is that in GEOS and RGeo implementations of Z and M coordinates, they are essentially just labels and not used in any computation (including equality checks). Ex.

require 'rgeo'

fac = RGeo::Geographic.spherical_factory(has_z_coordinate: true)
p fac.point(1,2,3) == fac.point(1,2,4)
# => true

fac = RGeo::Geos.factory(has_z_coordinate: true)
p fac.point(1,2,3) == fac.point(1,2,4)
# => true

For now, I think the best solution is to use ActiveModel::Dirty and check for z changes explicitly like you mentioned.

@BuonOmo what are your thoughts on this? Is it worthwhile to look into including z and m in equality checks? It technically would be out of spec with GEOS then, but it is pretty confusing IMO.

@BuonOmo
Copy link
Member

BuonOmo commented May 23, 2022

I think I'd stick with GEOS spec, to avoid having a heavy CAPI codebase and to keep clarity. This is not a strong opinion though

@keithdoggett
Copy link
Member

I also agree that it's best to stick with the spec for the core gem. I wonder if there might be a solution for this repo or rgeo-activerecord where we provide an option to perform a more advanced equality check on z and m fields if present.

@BuonOmo
Copy link
Member

BuonOmo commented Jun 8, 2022

@keithdoggett this can be sensible indeed if it is needed by AR to trigger a change.

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

No branches or pull requests

3 participants