go-geom icon indicating copy to clipboard operation
go-geom copied to clipboard

Question about the Valuer implementation of EWKB

Open JanBaryla opened this issue 3 years ago • 3 comments

Hi,

lately I've been trying to get this library to work with sqlboiler, to basically teach it how to talk to PostGIS. Everything went pretty well, but when attempting to INSERT or UPDATE, this error https://github.com/golang/go/blob/master/src/database/sql/convert.go#L194 kept popping up.

Everything looked good, I didn't understand why the Value isn't called. I found out this switch statement https://github.com/golang/go/blob/2ebe77a2fda1ee9ff6fd9a3e08933ad1ebaea039/src/database/sql/driver/types.go#L241 doesn't detect ewkb.GeometryCollection that I was using as a Valuer.

The fix I ended up doing is this: https://github.com/JanBaryla/go-geom/commit/daae2eef34b19e20253170ee1b0a5c0786cd04ac. Not quite sure what the consequences could be, though, so I'm opening an Issue instead of a PR.

JanBaryla avatar May 03 '21 16:05 JanBaryla

It might be necessary to use *ewkb.GeometryCollection (i.e. pointer to struct) not ewkb.GeometryCollection. This allows the handling of nil values (which are represented as NULL in SQL). go-geom currently does not support NOT NULL geometries.

There's a PostGIS example that includes an INSERT statement.

Hope this helps. If the problem persists when using *ewkb.GeometryCollection please could you share code that reproduces the problem - I'm not familiar with sqlboiler (but it looks like a really cool project!).

twpayne avatar May 03 '21 20:05 twpayne

I see. You're right that nils might be tricky.

I'll see what I can do when it comes to that example, there's quite some boilerplate if you wanna use sqlboiler.

But it really just comes down to the fact that even though ewkb.GeometryCollection appears to be implementing Value, it actually doesn't (because of the pointers), and the Golang's standard library for database operations doesn't recognize it as a Valuer because of it (because instead of blindly calling Value, it does the switch on type and tries checking the type)

It seems like in order to correctly implement the interface and keep things clean with nulls, one would have to add types like ewkb.NullGeometryCollection.

JanBaryla avatar May 03 '21 20:05 JanBaryla

It seems like in order to correctly implement the interface and keep things clean with nulls, one would have to add types like ewkb.NullGeometryCollection.

Yes, that makes sense. To be clear, it's only *ewkb.GeometryCollection that implements sql/driver.Valuer. ewkb.GeometryCollection does not implement the interface.

If I remember correctly, when I first implemented this it just seemed simpler and without loss of functionality to implement nullable geometries by default, as I didn't see a strong reason to implement non-nullable geometries. That said, I would be very happy to add ewkb.NullGeometryCollection and friends to go-geom. If we make any non-backwards-compatible changes then we can just bump the Go module's major version.

twpayne avatar May 03 '21 21:05 twpayne

Hopefully this is now resolved. Please re-open if needed.

twpayne avatar Jan 24 '23 22:01 twpayne