Repository navigation
[GH-3486] Keep the Z or M dimension of WKT tagged Z, M or ZM even when its values are all NaN - #3487
Merged
Merged
Conversation
…en when its values are all NaN ST_GeomFromWKT and the other WKT constructors parsed with JTS's WKTReader, which builds the same coordinate sequences for POINT (1 2) and POINT Z (1 2 NaN), so the Z dimension was dropped when stored. Use jts-io-patch 1.21.0-datasyslab-3's WKTReader.forDeclaredDimensions(), which marks sequences with the layout their Z, M or ZM tag declares, as the WKB reader already does. Untagged WKT, including the old JTS syntax POINT (1 2 3), is read as before. The same release makes ST_GeomFromWKB read POINT (NaN 1) as a point instead of POINT EMPTY.
…oSeries.geom_equals_identical The test recorded the old behavior, in which POINT Z EMPTY and an all-NaN LINESTRING Z read from WKT were identical to their 2D counterparts.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Did you read the Contributor Guide?
Is this PR related to a ticket?
[GH-XXX] my subject. Closes ST_GeomFromWKT drops an all-NaN Z dimension, and ST_GeomFromWKB reads POINT (NaN 1) as empty #3486What changes were proposed in this PR?
org.datasyslab:jts-io-patchto1.21.0-datasyslab-3. It addsWKTReader.forDeclaredDimensions(), the WKT counterpart of theWKBReader.forDeclaredDimensions()that Sedona already uses, and itsWKBReaderreads a point as empty only when X and Y are both NaN.Constructors.geomFromWKT,geomFromEWKT,mPointFromText,mLineFromText,mPolyFromTextandgeomCollFromText, and the WKT reader inFormatUtils, useWKTReader.forDeclaredDimensions().Effect:
Z,MorZMkeeps that layout when its values are all NaN or the geometry is empty:POINT Z (1 2 NaN)andPOINT Z EMPTYare stored as XYZ, andPOINT ZM (1 2 NaN NaN)as XYZM. SoST_EqualsIdentical(ST_GeomFromWKT('POINT (1 2)'), ST_GeomFromWKT('POINT Z (1 2 NaN)'))is false, as in GEOS, and a WKT geometry is identical to the same geometry read from WKB.POINT (1 2 3)is still XYZ andPOINT (1 2)still XY.ST_GeomFromWKBreadsPOINT (NaN 1)as a point with a NaN X instead ofPOINT EMPTY.POINT (NaN NaN)is still empty.This changes the output of the WKT constructors for tagged all-NaN input, which used to be stored as 2D.
How was this patch tested?
ConstructorsTestcases: the stored layout of tagged XYZ, XYM and XYZM points, lines, polygons, multipoints and empties, before and after a serializer round trip; untagged and old-syntax input unchanged; EWKT with an SRID; andPOINT (NaN 1)versusPOINT (NaN NaN)from WKB. NewPredicatesTestcase forST_EqualsIdenticalon tagged all-NaN WKT. These fail onmasterwith1.21.0-datasyslab-2.common: 1,446 tests pass. Before rebasing:constructorTestScala,predicateTestScala,functionTestScala,dataFrameAPITestScalaandGeoParquetDimensionsSuite(601 passed; the one failure is a MySQL test that needs Docker) and FlinkConstructorTest,PredicateTest,FunctionTest(274 passed), with the published1.21.0-datasyslab-3.Did this PR include necessary documentation updates?