[SEDONA-199] Added ST_NDims to Apache Sedona #720
Conversation
| @@ -280,6 +280,19 @@ public static int nPoints(Geometry geometry) { | |||
| return geometry.getNumPoints(); | |||
| } | |||
|
|
|||
| public static int nDims(Geometry geometry) { | |||
| String str = geometry.getCoordinate().toString(); | |||
There was a problem hiding this comment.
Why not directly check NaN using getX getY getZ getM from coordinate class?
docs/api/sql/Function.md
Outdated
| @@ -1475,3 +1475,24 @@ SELECT ST_ZMin(ST_GeomFromText('LINESTRING(1 3 4, 5 6 7)')) | |||
| ``` | |||
|
|
|||
| Output: `4.0` | |||
|
|
|||
| ## ST_NDims | |||
|
|
|||
There was a problem hiding this comment.
- Please add more elaboration and examples about dimension when the input has M or Z.
- The output should be
3integer not3.0double. Please correct me if I am wrong. - The doc should be sorted by the alphabetical order
| @@ -459,6 +459,11 @@ class functionTestScala extends TestBaseScala with Matchers with GeometrySample | |||
|
|
|||
| } | |||
|
|
|||
| it("Passed ST_NDims") { | |||
There was a problem hiding this comment.
Please add more test cases for 2, 3, 4D (e.g., when Z and M present, or both present)
|
Hi @jiayuasu, The requested changes are fixed now. Please let us know if any more changes are required. |
| } | ||
|
|
||
| it("Passed ST_NDims with both Z and M coordinates") { | ||
| val test = sparkSession.sql("SELECT ST_NDims(ST_GeomFromWKT('POINTZM(1 1 1 0.5)'))") |
There was a problem hiding this comment.
According to PostGIS doc (https://postgis.net/docs/ST_NDims.html), XYZM should return 4, but your case returns 3. If you cannot support M, I am ok with this. But please clearly state it in the API doc.
There was a problem hiding this comment.
Our Java function does support POINTZM values. And the test cases of Java with POINZM values give the expected result of 4 for POINTZM(1 1 1 0.5). But somehow, the same test case for Scala gives the value of 3.
For, POINTM(1 1 0.5), In the java test case, we get the output as 3, but in scala, we get the output as 2.
There was a problem hiding this comment.
Currently the geometry serializer in sedona-sql does not support M dimension, 4D geometries with ZM coordinates will have their M coordinates dropped and became 3D geometries. We're working on a new geometry serializer to resolve this issue.
There was a problem hiding this comment.
Thanks, @Kontinuation for clarifying my doubt.
There was a problem hiding this comment.
@jiayuasu, I made the updates with the test cases and documentation.
Did you read the Contributor Guide?
Is this PR related to a JIRA ticket?
[SEDONA-199] my subject.What changes were proposed in this PR?
Added ST_NDims functions to SQL
How was this patch tested?
Through unit tests written in Scala
Did this PR include necessary documentation updates?
Yes, I have updated the documentation.