Uh oh!
There was an error while loading. Please reload this page.
Fix writing XYM geometries as WKB - #1092
Conversation
| @@ -0,0 +1,79 @@ | |||
| /* | |||
| * Copyright (c) 2016 Vivid Solutions. | |||
There was a problem hiding this comment.
this is incorrect for a new file
There was a problem hiding this comment.
All source code in this file is taken from WKTWriter.java, which is an old file. Should we update the copyright header or preserve the copyright header of the file where it originates from?
There was a problem hiding this comment.
I do not think it needs to hold things up much, the options are:
/*
* Copyright (c) 2016 Vivid Solutions, and others
*
Or explicitly:
/*
* Copyright (c) 2025 your name
* Copyright (c) 2016 Vivid Solutions
*
Issue #733 is closed but the problem still exists in the latest master. |
jiayuasu
commented
Feb 24, 2025
@jodygarnett@dr-jts Would you please merge this PR? We are working on adding Geo support into Apache Iceberg and Parquet Java reader and writer. JTS is a fundamental lib in this process. WKB is the encoding mechanism used in all these PRs. |
jiayuasu
commented
Feb 24, 2025
@jnh5y CC James too 🙏🏻 |
Kontinuation
commented
Feb 25, 2025
I'd like to provide more context for the problem we are solving. Given the following Stringwkt = "MULTILINESTRING M((1 1 1, 2 2 2))";
WKTReaderwktReader = newWKTReader();
GeometrygeometryBefore = wktReader.read(wkt);Writing it using WKBWriter and read it back using WKBReader will result in a WKBWriterwkbWriter = newWKBWriter(3);
byte[] write = wkbWriter.write(geometryBefore);
WKBReaderwkbReader = newWKBReader();
GeometrygeometryAfter = wkbReader.read(write);
System.out.println(Arrays.asList(geometryAfter.getCoordinates())); // [(1.0, 1.0, 1.0), (2.0, 2.0, 2.0)]If we use WKTWriter and WKTReader for serializing and deserializing the WKTWriterwktWriter = newWKTWriter(3);
Stringwrite = wktWriter.write(geometryBefore);
GeometrygeometryAfter = wktReader.read(write);
System.out.println(Arrays.asList(geometryAfter.getCoordinates())); // [(1.0, 1.0, NaN), (2.0, 2.0, NaN)]We found that JTS will only output M ordinate when WKTWriterwktWriter = newWKTWriter(4);
Stringwrite = wktWriter.write(geometryBefore);
System.out.println(write); // MULTILINESTRING M((1 1 1, 2 2 2))GeometrygeometryAfter = wktReader.read(write);
System.out.println(Arrays.asList(geometryAfter.getCoordinates())); // [(1.0, 1.0 m=1.0), (2.0, 2.0 m=2.0)]I believe that WKBWriter should behave the same as WKTWriter. However, specifying WKBWriterwkbWriter = newWKBWriter(4);
byte[] write = wkbWriter.write(geometryBefore); // <- exception raised here// java.lang.IllegalArgumentException: Invalid ordinate index: 3//// at org.locationtech.jts.geom.CoordinateXYM.getOrdinate(CoordinateXYM.java:140)// at org.locationtech.jts.geom.impl.CoordinateArraySequence.getOrdinate(CoordinateArraySequence.java:260)// at org.locationtech.jts.io.WKBWriter.writeCoordinate(WKBWriter.java:524)// at org.locationtech.jts.io.WKBWriter.writeCoordinateSequence(WKBWriter.java:502)// at org.locationtech.jts.io.WKBWriter.writeLineString(WKBWriter.java:425)// at org.locationtech.jts.io.WKBWriter.write(WKBWriter.java:388)// at org.locationtech.jts.io.WKBWriter.writeGeometryCollection(WKBWriter.java:454)// at org.locationtech.jts.io.WKBWriter.write(WKBWriter.java:395)// at org.locationtech.jts.io.WKBWriter.write(WKBWriter.java:367)This PR resolves the above issue by making the behavior of WKBWriter consistent with WKTWriter. No Z or M ordinates will be written with WKBWriterwkbWriter = newWKBWriter(3);
byte[] write = wkbWriter.write(geometryBefore);
WKBReaderwkbReader = newWKBReader();
GeometrygeometryAfter = wkbReader.read(write);
System.out.println(Arrays.asList(geometryAfter.getCoordinates())); // [(1.0, 1.0), (2.0, 2.0)]The M ordinate will be written as expected with WKBWriterwkbWriter = newWKBWriter(4);
byte[] write = wkbWriter.write(geometryBefore);
WKBReaderwkbReader = newWKBReader();
GeometrygeometryAfter = wkbReader.read(write);
System.out.println(Arrays.asList(geometryAfter.getCoordinates())); // [(1.0, 1.0 m=1.0), (2.0, 2.0 m=2.0)] |
jiayuasu
commented
May 1, 2025
Hi Martin, Jody, and James, This is Jia from Wherobots, Apache Sedona. Iceberg and Parquet have adopted WKB as the default encoding for geometry, and we’re currently working on adding JTS as a dependency of Iceberg and Parquet geometry. This change will have a broad impact across the ecosystem. Could you please review and consider merging this PR? Or let us know what’s needed to get it across the finish line? Thanks! |
Fixes#733
Please note that we must specify
outputDimension = 4when writing geometries with M dimension, even when the geometry is XYM instead of XYZM. This behavior is the same as WKTWriter.