-
Notifications
You must be signed in to change notification settings - Fork 697
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
[SEDONA-201] add ST_MPolyFromText and ST_MLineFromText methods #718
[SEDONA-201] add ST_MPolyFromText and ST_MLineFromText methods #718
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for your contribution. Please make the requested changes.
docs/api/flink/Constructor.md
Outdated
|
||
Format: `ST_MLineFromWKT (Text:string)` | ||
|
||
Since: `1.3.0` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The version should be 1.3.1
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jiayuasu I have made all the requested changes and have successfully run the test cases. Please let me know if any more changes are required.
@@ -28,4 +28,20 @@ public static Geometry geomFromWKT(String wkt, int srid) throws ParseException { | |||
GeometryFactory geometryFactory = new GeometryFactory(new PrecisionModel(), srid); | |||
return new WKTReader(geometryFactory).read(wkt); | |||
} | |||
|
|||
public static Geometry mLineFromWKT(String wkt, int srid) throws ParseException { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The function names should be FromText
, not FromWKT
@@ -204,5 +204,15 @@ class constructorTestScala extends TestBaseScala { | |||
var spatialRDD2 = Adapter.toSpatialRdd(df, "geometry") | |||
Adapter.toDf(spatialRDD2, sparkSession).show(1) | |||
} | |||
|
|||
it("Passed ST_MLineFromWKT") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add tests for the input with SRID
@@ -185,4 +185,19 @@ public Geometry eval(@DataTypeHint("String") String kml) throws ParseException { | |||
return new KMLReader().read(kml); | |||
} | |||
} | |||
|
|||
public static class ST_MPolyFromWKT extends ScalarFunction { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please create overload functions to take care of the inputs with SRID
@@ -73,4 +73,11 @@ object st_constructors extends DataFrameAPI { | |||
|
|||
def ST_PolygonFromText(coords: Column, delimiter: Column): Column = wrapExpression[ST_PolygonFromText](coords, delimiter) | |||
def ST_PolygonFromText(coords: String, delimiter: String): Column = wrapExpression[ST_PolygonFromText](coords, delimiter) | |||
|
|||
def ST_MPolyFromWKT(wkt: Column): Column = wrapExpression[ST_MPolyFromWKT](wkt, 0) | |||
def ST_MPolyFromWKT(wkt: String): Column = wrapExpression[ST_MPolyFromWKT](wkt, 0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please handle the case with optional SRID
@gmaraswa ST_MakePolygon also uses an optional parameter. Please follow the way it handles optional parameters |
Did you read the Contributor Guide?
Is this PR related to a JIRA ticket?
What changes were proposed in this PR?
Implementation of ST_MLineFromText and ST_MPolyFromText
How was this patch tested?
Scala, Python, Java integration tests
Did this PR include necessary documentation updates?