[SEDONA-157] Add coordinate accessors to the Flink API #677
Conversation
| @@ -70,6 +70,27 @@ public static double length(Geometry geometry) { | |||
| return geometry.getLength(); | |||
| } | |||
|
|
|||
| public static Double x(Geometry geometry) { | |||
There was a problem hiding this comment.
Does this need to be the boxed type instead of the native double like the other functions return?
There was a problem hiding this comment.
ST_X, ST_Y and ST_Z should return null if the first argument is not a Point, so we have to check the input type somewhere. I chose to do that in the common functions, because if they can direcly return null by using a boxed type, we can simply wrap it with InferredUnaryExpression in the Spark SQL functions. But as you pointed out, returning boxed type looks somewhat strange. Should I change the common functions to return a primitive value? In that case, we have to use UnaryGeometryExpression instead of InferredUnaryExpression so the code will become a bit verbose. Either is fine with me.
There was a problem hiding this comment.
Right that makes sense then, nvm
| @@ -265,6 +265,27 @@ public void testIsEmpty() { | |||
| assertEquals(false, result); | |||
| } | |||
|
|
|||
| @Test | |||
| public void testX() { | |||
There was a problem hiding this comment.
Can you use the same test in testZ for testX and testY so testX and testY don't need to use point_real?
There was a problem hiding this comment.
Updated the PR. Thanks for the review!
jiayuasu
changed the title
Did you read the Contributor Guide?
Is this PR related to a JIRA ticket?
[SEDONA-XXX] my subject.What changes were proposed in this PR?
This PR adds ST_X, ST_Y and ST_Z to the Flink API.
How was this patch tested?
mvn clean installlocallymkdocs serveand checked the generate documentDid this PR include necessary documentation updates?
vX.Y.Zformat.