[SEDONA-196] Add ST_Force3D #856

iGN5117 · 2023-06-09T23:02:48Z

Did you read the Contributor Guide?

Yes, I have read Contributor Rules and Contributor Development Guide

Is this PR related to a JIRA ticket?

Yes, the URL of the associated JIRA ticket is https://issues.apache.org/jira/browse/SEDONA-196. The PR name follows the format [SEDONA-XXX] my subject.

What changes were proposed in this PR?

Add ST_Force3D to sedona

How was this patch tested?

Comprehensive correctness test cases in java
Integration test cases

Did this PR include necessary documentation updates?

Yes, I am adding a new API. I am using the 1.4.1 (https://github.com/apache/sedona/blob/master/pom.xml#L29) in since v1.4.1 format.
Yes, I have updated the documentation.

This reverts commit c7f6236.

This reverts commit 85ae113.

into develop_Nilesh_1.4.0

Changed generic Exception to IllegalArgumentException in ST_NumPoints implementation and its corresponding test

# Conflicts: # common/src/main/java/org/apache/sedona/common/Functions.java # common/src/test/java/org/apache/sedona/common/FunctionsTest.java # flink/src/main/java/org/apache/sedona/flink/Catalog.java # flink/src/main/java/org/apache/sedona/flink/expressions/Functions.java # flink/src/test/java/org/apache/sedona/flink/FunctionTest.java # python/sedona/sql/st_functions.py # python/tests/sql/test_function.py # sql/common/src/main/scala/org/apache/sedona/sql/UDF/Catalog.scala # sql/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala # sql/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/st_functions.scala # sql/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala # sql/common/src/test/scala/org/apache/sedona/sql/functionTestScala.scala

Refactored function name Made java tests more comprehensive by checking both nDims and WKT of returned geometry Added more test cases in scala test cases Updated documentation with empty geometry case and more examples

flink/src/test/java/org/apache/sedona/flink/FunctionTest.java

jiayuasu · 2023-06-11T06:54:34Z

sql/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala

@@ -957,5 +957,13 @@ class dataFrameAPITestScala extends TestBaseScala {
      val expectedResult = 3
      assert(actualResult == expectedResult)
    }
+
+    it("Passed ST_Force3D") {
+      val lineDf = sparkSession.sql("SELECT ST_Force3D(ST_GeomFromWKT('LINESTRING (0 1, 1 0, 2 0)'), 2.3) AS geom")


Please add the test case that has no zvalue supplied.

What I mean is, add a test case like ST_Force3D(geom), no z value is supplied. This will test the correctness of the the optional parameter.

In addition, the test cases didn't test the correctness of the DataFrame style API, it mistakenly repeated the SQL test cases. Please read other test cases in this file and learn how to test DataFrame style APIs.

Explanation of Sedona DataFrame style APIs: https://sedona.apache.org/latest-snapshot/api/sql/DataFrameAPI/

sql/common/src/test/scala/org/apache/sedona/sql/functionTestScala.scala

flink/src/main/java/org/apache/sedona/flink/Catalog.java

jiayuasu · 2023-06-11T06:56:11Z

docs/api/sql/Function.md

+
+Input: `LINESTRING(0 1, 1 2, 2 1)`
+
+Output: `LINESTRING(0 1 0, 1 2 0, 2 1 0)`


Isn't the output LINESTRING Z?

Yes you're right, updated the documentation with a note explaining the output

…ut. Added a note explaining the same

jiayuasu · 2023-06-11T22:17:37Z

sql/common/src/main/scala/org/apache/sedona/sql/UDF/Catalog.scala

@@ -148,6 +148,7 @@ object Catalog {
    function[ST_AreaSpheroid](),
    function[ST_LengthSpheroid](),
    function[ST_NumPoints](),
+    function[ST_Force3D](),


Please use ST_MinimumBoundingCirle's implementation as an example: (1) Function definition: https://github.com/apache/sedona/blob/master/sql/common/src/main/scala/org/apache/spark/sql/sedona_sql/expressions/Functions.scala#LL473C12-L473C36
(2) Function catalog registration: https://github.com/apache/sedona/blob/master/sql/common/src/main/scala/org/apache/sedona/sql/UDF/Catalog.scala#L102

The default value of the optional parameters must be supplied in the Function Catalog otherwise this is not an optional parameter.

jiayuasu · 2023-06-11T22:20:02Z

sql/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala

@@ -957,5 +957,13 @@ class dataFrameAPITestScala extends TestBaseScala {
      val expectedResult = 3
      assert(actualResult == expectedResult)
    }
+
+    it("Passed ST_Force3D") {
+      val lineDf = sparkSession.sql("SELECT ST_Force3D(ST_GeomFromWKT('LINESTRING (0 1, 1 0, 2 0)'), 2.3) AS geom")


What I mean is, add a test case like ST_Force3D(geom), no z value is supplied. This will test the correctness of the the optional parameter.

sql/common/src/test/scala/org/apache/sedona/sql/functionTestScala.scala

jiayuasu · 2023-06-11T22:21:56Z

sql/common/src/test/scala/org/apache/sedona/sql/dataFrameAPITestScala.scala

@@ -957,5 +957,13 @@ class dataFrameAPITestScala extends TestBaseScala {
      val expectedResult = 3
      assert(actualResult == expectedResult)
    }
+
+    it("Passed ST_Force3D") {
+      val lineDf = sparkSession.sql("SELECT ST_Force3D(ST_GeomFromWKT('LINESTRING (0 1, 1 0, 2 0)'), 2.3) AS geom")


In addition, the test cases didn't test the correctness of the DataFrame style API, it mistakenly repeated the SQL test cases. Please read other test cases in this file and learn how to test DataFrame style APIs.

Explanation of Sedona DataFrame style APIs: https://sedona.apache.org/latest-snapshot/api/sql/DataFrameAPI/

common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java

iGN5117 added 18 commits June 6, 2023 13:29

Remove prebuild_index config from search plugin

85ae113

Update compile the code documentation for sedona

8e829fe

approximate float comparisons in python test cases

4bcf99c

Revert "Update compile the code documentation for sedona"

15c6255

This reverts commit c7f6236.

Revert "Remove prebuild_index config from search plugin"

471e281

This reverts commit 85ae113.

FIxed incorrect indentation

2a5a6d8

Add ST_NumPoints

cf7e78d

Update available version to 1.4.1 in documentation

fc23fc9

Merge branch 'apache:master' into develop_Nilesh_1.4.0

fea1daa

Fix failing scala test case

816e4ed

Merge branch 'develop_Nilesh_1.4.0' of https://github.com/iGN5117/sedona

472c383

into develop_Nilesh_1.4.0

Updated documentation to include negative flow.

471a87e

Changed generic Exception to IllegalArgumentException in ST_NumPoints implementation and its corresponding test

Removed Spark SQL from flink documentation

b33499d

Add ST_Force3D to sedona

0d68337

Updated force3D logic to handle empty geometries

da0314a

Refactored function name Made java tests more comprehensive by checking both nDims and WKT of returned geometry Added more test cases in scala test cases Updated documentation with empty geometry case and more examples

Updated force3D dataframe test

020e9b9

fix error in test case

86de054

jiayuasu requested changes Jun 11, 2023

View reviewed changes

jiayuasu added this to the sedona-1.4.1 milestone Jun 11, 2023

jiayuasu added sedona-sql affect public APIs improvement labels Jun 11, 2023

iGN5117 added 5 commits June 11, 2023 11:11

Updated documentation for Force3D to include Z format WKT in the outp…

307c060

…ut. Added a note explaining the same

Added default zValue test case in sedona flink

dbab04c

Added default zValue dataframe test case

c0eb8bc

Added default zValue scala test case

479b264

fix dataframe testcase

641d2d9

jiayuasu requested changes Jun 11, 2023

View reviewed changes

common/src/main/java/org/apache/sedona/common/utils/GeomUtils.java Outdated Show resolved Hide resolved

Addressed PR comments

c2e27d8

jiayuasu approved these changes Jun 12, 2023

View reviewed changes

jiayuasu merged commit 800a3f5 into apache:master Jun 12, 2023

Kontinuation pushed a commit to Kontinuation/sedona that referenced this pull request Oct 11, 2024

[SEDONA-196] Add ST_Force3D (apache#856)

e500912

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SEDONA-196] Add ST_Force3D #856

[SEDONA-196] Add ST_Force3D #856

iGN5117 commented Jun 9, 2023

jiayuasu Jun 11, 2023

iGN5117 Jun 11, 2023

jiayuasu Jun 11, 2023

jiayuasu Jun 11, 2023

jiayuasu Jun 11, 2023

iGN5117 Jun 11, 2023

jiayuasu Jun 11, 2023

jiayuasu Jun 11, 2023

jiayuasu Jun 11, 2023


		Input: `LINESTRING(0 1, 1 2, 2 1)`

		Output: `LINESTRING(0 1 0, 1 2 0, 2 1 0)`

[SEDONA-196] Add ST_Force3D #856

[SEDONA-196] Add ST_Force3D #856

Conversation

iGN5117 commented Jun 9, 2023

Did you read the Contributor Guide?

Is this PR related to a JIRA ticket?

What changes were proposed in this PR?

How was this patch tested?

Did this PR include necessary documentation updates?

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment