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

SRID not preserved in Geometry types (Point) since it is encoded as text using the WKTWriter #542

Closed
jurriaan opened this issue Aug 31, 2022 · 7 comments
Assignees
Labels
type: bug A general bug
Milestone

Comments

@jurriaan
Copy link

Bug Report

I'm trying to store JTS Points in a geometry(Point,4326) type column. This fails since the SRID is not encoded in the encoded representation.

Versions

  • Driver: 0.9.1
  • Database: PostgreSQL 13.6 (PostGIS 3.0 USE_GEOS=1 USE_PROJ=1 USE_STATS=1)
  • Java: OpenJDK 64-Bit Server VM (build 18.0.2+0, mixed mode)
  • OS: Linux 5.19.5

Current Behavior

Inserting a Point with SRID into a geometry(Point,4326) column results in the following error.

io.r2dbc.postgresql.ExceptionFactory$PostgresqlBadGrammarException: [22023] Geometry SRID (0) does not match column SRID (4326)

Steps to reproduce

This is how Geometries are currently encoded in PostgisGeometryCodec:

@Override
public EncodedParameter encode(Object value) {
    Assert.requireType(value, Geometry.class, "value must be Geometry type");
    Geometry geometry = (Geometry) value;

    return new EncodedParameter(Format.FORMAT_TEXT, this.oid, Mono.fromSupplier(
        () -> ByteBufUtils.encode(this.byteBufAllocator, geometry.toText())
    ));
}

This drops the SRID from the geometry as shown with this example:

val factory = GeometryFactory(PrecisionModel(), 4326)

val point = factory.createPoint(Coordinate(10.0, 5.0))
println(point.srid) // -> 4326
println(point.toText()) // -> POINT (10 5)

Expected behavior/workaround

Either EWKT or (E)WKB representations should be used when encoding Geometries, these can encode the SRID properly.

For our purposes a workaround that seems to help is encoding the geometries as WKB before inserting (we use Spring Data, so this is easy using Converters):

/**
 * Alternative encoder of geometries that encodes the geometry in WKB (well known binary) format and includes the SRID.
 */
@WritingConverter
object GeometryToWKBConverter : Converter<Geometry, ByteArray> {
    private val encoder = WKBWriter(/* outputDimension = */ 2, /* includeSRID = */ true)

    override fun convert(source: Geometry): ByteArray = encoder.write(source)
}
@jurriaan jurriaan added the status: waiting-for-triage An issue we've not yet triaged label Aug 31, 2022
@mp911de
Copy link
Collaborator

mp911de commented Aug 31, 2022

Thanks for the report. Would you be interested in submitting a pull request (and potentially reviewing the other geometry codecs)? I'm happy to provide further guidance. However, I'm not familiar with geometry specifics and happy to collaborate on the fix.

FWIW, we plan to ship another service release tomorrow, so we have an excellent opportunity to ship a fix for this issue timely.

@mp911de mp911de added type: bug A general bug and removed status: waiting-for-triage An issue we've not yet triaged labels Aug 31, 2022
@mp911de mp911de added this to the 0.9.2.RELEASE milestone Aug 31, 2022
@jurriaan
Copy link
Author

@mp911de Thanks for the quick response!

I want to contribute a solution for this, but I'm not sure what the best way forward is.

The thing is, Postgis supports 2, 3, and 4 dimension geometries (POINT, POINT Z, POINT ZM), and my workaround with WKBWriter needs to have the amount of dimensions specified beforehand (the outputDimension initializer property). I just set it to 2 in the workaround since it works for our usecase. But you probably want a more universal solution in r2dbc-postgresql I think.

@jurriaan
Copy link
Author

The official postgis java package also does not support more than two dimensions at the moment: https://github.com/postgis/postgis-java/blob/a01a8c899e9835afbbd78e8057959ecdba6738c0/postgis-jdbc-jts/src/main/java/net/postgis/jdbc/jts/JtsBinaryWriter.java#L77-L78

@mp911de
Copy link
Collaborator

mp911de commented Aug 31, 2022

We're using JTS objects. I would expect that the Geometry object contains details about how many dimensions there are (CoordinateSequence contains some methods that explain how many coordinates/dimensions/measures there are). I have no idea whether the SRID need always to be present.

@mp911de mp911de self-assigned this Sep 1, 2022
@mp911de mp911de closed this as completed in fe652ff Sep 1, 2022
mp911de added a commit that referenced this issue Sep 1, 2022
We now use WKBWriter with two dimensions preserving the SRID.

Add integration tests for Postgis.

[resolves #542]

Signed-off-by: Mark Paluch <mpaluch@vmware.com>
@mp911de
Copy link
Collaborator

mp911de commented Sep 1, 2022

I fixed the issue by using your suggested workaround and added an integration test.

@jurriaan
Copy link
Author

jurriaan commented Sep 1, 2022

@mp911de Oh wow haha, that works as well!

It is kinda hard to know how many dimensions there are since Geometry can be nested as GeometryCollections / MultiLineString / LineString with the dimensions only known in the nested points so it's kinda hard to decide this automatically. I couldn't directly find a way to do so.

At least this fixes my issue for now. Thanks a lot!

@mp911de
Copy link
Collaborator

mp911de commented Sep 1, 2022

I couldn't directly find a way to do so.

Me neither, either someone more knowledgeable will show up and help us or things are fine as they are now. In any case, thanks for your support.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug A general bug
Projects
None yet
Development

No branches or pull requests

2 participants