You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/06/11 03:20:25 UTC

[GitHub] [spark] HyukjinKwon opened a new pull request #28795: [SPARK-31965][TESTS][PYTHON] Move doctests related to Java function registration to test conditionally

HyukjinKwon opened a new pull request #28795:
URL: https://github.com/apache/spark/pull/28795


   ### What changes were proposed in this pull request?
   
   This PR proposes to move the doctests in `registerJavaUDAF` and `registerJavaFunction` to the proper unittests that run conditionally when the test classes are present.
   
   Both tests are dependent on the test classes in JVM side, `test.org.apache.spark.sql.JavaStringLength` and `test.org.apache.spark.sql.MyDoubleAvg`. So if you run the tests against the plain `sbt package`, it fails as below:
   
   ```
   **********************************************************************
   File "/.../spark/python/pyspark/sql/udf.py", line 366, in pyspark.sql.udf.UDFRegistration.registerJavaFunction
   Failed example:
       spark.udf.registerJavaFunction(
           "javaStringLength", "test.org.apache.spark.sql.JavaStringLength", IntegerType())
   Exception raised:
       Traceback (most recent call last):
      ...
   test.org.apache.spark.sql.JavaStringLength, please make sure it is on the classpath;
   ...
      6 of   7 in pyspark.sql.udf.UDFRegistration.registerJavaFunction
      2 of   4 in pyspark.sql.udf.UDFRegistration.registerJavaUDAF
   ***Test Failed*** 8 failures.
   ```
   
   ### Why are the changes needed?
   
   In order to support to run the tests against the plain SBT build. See also https://spark.apache.org/developer-tools.html
   
   ### Does this PR introduce _any_ user-facing change?
   
   No, it's test-only.
   
   ### How was this patch tested?
   
   Manually tested as below:
   
   ```bash
   ./build/sbt -DskipTests -Phive-thriftserver clean package
   cd python
   ./run-tests --python-executable=python3 --testname="pyspark.sql.udf UserDefinedFunction"
   ./run-tests --python-executable=python3 --testname="pyspark.sql.tests.test_udf UDFTests"
   ```
   
   ```bash
   ./build/sbt -DskipTests -Phive-thriftserver clean test:package
   cd python
   ./run-tests --python-executable=python3 --testname="pyspark.sql.udf UserDefinedFunction"
   ./run-tests --python-executable=python3 --testname="pyspark.sql.tests.test_udf UDFTests"
   ```


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on pull request #28795: [SPARK-31965][TESTS][PYTHON] Move doctests related to Java function registration to test conditionally

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #28795:
URL: https://github.com/apache/spark/pull/28795#issuecomment-642381925


   Also cc @dongjoon-hyun FYI


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28795: [SPARK-31965][TESTS][PYTHON] Move doctests related to Java function registration to test conditionally

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28795:
URL: https://github.com/apache/spark/pull/28795#issuecomment-642383044






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28795: [SPARK-31965][TESTS][PYTHON] Move doctests related to Java function registration to test conditionally

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28795:
URL: https://github.com/apache/spark/pull/28795#issuecomment-642382801


   **[Test build #123811 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/123811/testReport)** for PR 28795 at commit [`e7a9974`](https://github.com/apache/spark/commit/e7a99746deea89e98cd800c5cdee9a8cedbf6088).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28795: [SPARK-31965][TESTS][PYTHON] Move doctests related to Java function registration to test conditionally

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28795:
URL: https://github.com/apache/spark/pull/28795#issuecomment-642395035






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun closed pull request #28795: [SPARK-31965][TESTS][PYTHON] Move doctests related to Java function registration to test conditionally

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun closed pull request #28795:
URL: https://github.com/apache/spark/pull/28795


   


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on pull request #28795: [SPARK-31965][TESTS][PYTHON] Move doctests related to Java function registration to test conditionally

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #28795:
URL: https://github.com/apache/spark/pull/28795#issuecomment-642381671


   Also cc @bersprockets I remember you took a look for this one as well.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28795: [SPARK-31965][TESTS][PYTHON] Move doctests related to Java function registration to test conditionally

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28795:
URL: https://github.com/apache/spark/pull/28795#discussion_r438522155



##########
File path: python/pyspark/sql/tests/test_udf.py
##########
@@ -357,6 +359,32 @@ def test_udf_registration_returns_udf(self):
             df.select(add_four("id").alias("plus_four")).collect()
         )
 
+    @unittest.skipIf(not test_compiled, test_not_compiled_message)
+    def test_register_java_function(self):
+        self.spark.udf.registerJavaFunction(
+            "javaStringLength", "test.org.apache.spark.sql.JavaStringLength", IntegerType())
+        [value] = self.spark.sql("SELECT javaStringLength('test')").first()
+        self.assertEqual(value, 4)
+
+        self.spark.udf.registerJavaFunction(
+            "javaStringLength2", "test.org.apache.spark.sql.JavaStringLength")
+        [value] = self.spark.sql("SELECT javaStringLength2('test')").first()
+        self.assertEqual(value, 4)
+
+        self.spark.udf.registerJavaFunction(
+            "javaStringLength3", "test.org.apache.spark.sql.JavaStringLength", "integer")
+        [value] = self.spark.sql("SELECT javaStringLength3('test')").first()
+        self.assertEqual(value, 4)
+
+    @unittest.skipIf(not test_compiled, test_not_compiled_message)
+    def test_register_java_udaf(self):
+        self.spark.udf.registerJavaUDAF("javaUDAF", "test.org.apache.spark.sql.MyDoubleAvg")
+        df = self.spark.createDataFrame([(1, "a"), (2, "b"), (3, "a")], ["id", "name"])
+        df.createOrReplaceTempView("df")
+        row = self.spark.sql(
+            "SELECT name, javaUDAF(id) as avg from df group by name order by name desc").first()
+        self.assertEqual(row.asDict(), Row(name='b', avg=102.0).asDict())

Review comment:
       In this case, we don't compare `[Row(name=u'b', avg=102.0), Row(name=u'a', avg=102.0)]`?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA removed a comment on pull request #28795: [SPARK-31965][TESTS][PYTHON] Move doctests related to Java function registration to test conditionally

Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #28795:
URL: https://github.com/apache/spark/pull/28795#issuecomment-642382801


   **[Test build #123811 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/123811/testReport)** for PR 28795 at commit [`e7a9974`](https://github.com/apache/spark/commit/e7a99746deea89e98cd800c5cdee9a8cedbf6088).


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28795: [SPARK-31965][TESTS][PYTHON] Move doctests related to Java function registration to test conditionally

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28795:
URL: https://github.com/apache/spark/pull/28795#discussion_r438522155



##########
File path: python/pyspark/sql/tests/test_udf.py
##########
@@ -357,6 +359,32 @@ def test_udf_registration_returns_udf(self):
             df.select(add_four("id").alias("plus_four")).collect()
         )
 
+    @unittest.skipIf(not test_compiled, test_not_compiled_message)
+    def test_register_java_function(self):
+        self.spark.udf.registerJavaFunction(
+            "javaStringLength", "test.org.apache.spark.sql.JavaStringLength", IntegerType())
+        [value] = self.spark.sql("SELECT javaStringLength('test')").first()
+        self.assertEqual(value, 4)
+
+        self.spark.udf.registerJavaFunction(
+            "javaStringLength2", "test.org.apache.spark.sql.JavaStringLength")
+        [value] = self.spark.sql("SELECT javaStringLength2('test')").first()
+        self.assertEqual(value, 4)
+
+        self.spark.udf.registerJavaFunction(
+            "javaStringLength3", "test.org.apache.spark.sql.JavaStringLength", "integer")
+        [value] = self.spark.sql("SELECT javaStringLength3('test')").first()
+        self.assertEqual(value, 4)
+
+    @unittest.skipIf(not test_compiled, test_not_compiled_message)
+    def test_register_java_udaf(self):
+        self.spark.udf.registerJavaUDAF("javaUDAF", "test.org.apache.spark.sql.MyDoubleAvg")
+        df = self.spark.createDataFrame([(1, "a"), (2, "b"), (3, "a")], ["id", "name"])
+        df.createOrReplaceTempView("df")
+        row = self.spark.sql(
+            "SELECT name, javaUDAF(id) as avg from df group by name order by name desc").first()
+        self.assertEqual(row.asDict(), Row(name='b', avg=102.0).asDict())

Review comment:
       In this case, we only compare `first`?




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on a change in pull request #28795: [SPARK-31965][TESTS][PYTHON] Move doctests related to Java function registration to test conditionally

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on a change in pull request #28795:
URL: https://github.com/apache/spark/pull/28795#discussion_r438529975



##########
File path: python/pyspark/sql/tests/test_udf.py
##########
@@ -357,6 +359,32 @@ def test_udf_registration_returns_udf(self):
             df.select(add_four("id").alias("plus_four")).collect()
         )
 
+    @unittest.skipIf(not test_compiled, test_not_compiled_message)
+    def test_register_java_function(self):
+        self.spark.udf.registerJavaFunction(
+            "javaStringLength", "test.org.apache.spark.sql.JavaStringLength", IntegerType())
+        [value] = self.spark.sql("SELECT javaStringLength('test')").first()
+        self.assertEqual(value, 4)
+
+        self.spark.udf.registerJavaFunction(
+            "javaStringLength2", "test.org.apache.spark.sql.JavaStringLength")
+        [value] = self.spark.sql("SELECT javaStringLength2('test')").first()
+        self.assertEqual(value, 4)
+
+        self.spark.udf.registerJavaFunction(
+            "javaStringLength3", "test.org.apache.spark.sql.JavaStringLength", "integer")
+        [value] = self.spark.sql("SELECT javaStringLength3('test')").first()
+        self.assertEqual(value, 4)
+
+    @unittest.skipIf(not test_compiled, test_not_compiled_message)
+    def test_register_java_udaf(self):
+        self.spark.udf.registerJavaUDAF("javaUDAF", "test.org.apache.spark.sql.MyDoubleAvg")
+        df = self.spark.createDataFrame([(1, "a"), (2, "b"), (3, "a")], ["id", "name"])
+        df.createOrReplaceTempView("df")
+        row = self.spark.sql(
+            "SELECT name, javaUDAF(id) as avg from df group by name order by name desc").first()
+        self.assertEqual(row.asDict(), Row(name='b', avg=102.0).asDict())

Review comment:
       Got it~ No problem~




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins removed a comment on pull request #28795: [SPARK-31965][TESTS][PYTHON] Move doctests related to Java function registration to test conditionally

Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #28795:
URL: https://github.com/apache/spark/pull/28795#issuecomment-642395035






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] SparkQA commented on pull request #28795: [SPARK-31965][TESTS][PYTHON] Move doctests related to Java function registration to test conditionally

Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #28795:
URL: https://github.com/apache/spark/pull/28795#issuecomment-642394663


   **[Test build #123811 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/123811/testReport)** for PR 28795 at commit [`e7a9974`](https://github.com/apache/spark/commit/e7a99746deea89e98cd800c5cdee9a8cedbf6088).
    * This patch passes all tests.
    * This patch merges cleanly.
    * This patch adds no public classes.


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] AmplabJenkins commented on pull request #28795: [SPARK-31965][TESTS][PYTHON] Move doctests related to Java function registration to test conditionally

Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #28795:
URL: https://github.com/apache/spark/pull/28795#issuecomment-642383044






----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on pull request #28795: [SPARK-31965][TESTS][PYTHON] Move doctests related to Java function registration to test conditionally

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on pull request #28795:
URL: https://github.com/apache/spark/pull/28795#issuecomment-642396796


   Thanks!


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] HyukjinKwon commented on a change in pull request #28795: [SPARK-31965][TESTS][PYTHON] Move doctests related to Java function registration to test conditionally

Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #28795:
URL: https://github.com/apache/spark/pull/28795#discussion_r438527805



##########
File path: python/pyspark/sql/tests/test_udf.py
##########
@@ -357,6 +359,32 @@ def test_udf_registration_returns_udf(self):
             df.select(add_four("id").alias("plus_four")).collect()
         )
 
+    @unittest.skipIf(not test_compiled, test_not_compiled_message)
+    def test_register_java_function(self):
+        self.spark.udf.registerJavaFunction(
+            "javaStringLength", "test.org.apache.spark.sql.JavaStringLength", IntegerType())
+        [value] = self.spark.sql("SELECT javaStringLength('test')").first()
+        self.assertEqual(value, 4)
+
+        self.spark.udf.registerJavaFunction(
+            "javaStringLength2", "test.org.apache.spark.sql.JavaStringLength")
+        [value] = self.spark.sql("SELECT javaStringLength2('test')").first()
+        self.assertEqual(value, 4)
+
+        self.spark.udf.registerJavaFunction(
+            "javaStringLength3", "test.org.apache.spark.sql.JavaStringLength", "integer")
+        [value] = self.spark.sql("SELECT javaStringLength3('test')").first()
+        self.assertEqual(value, 4)
+
+    @unittest.skipIf(not test_compiled, test_not_compiled_message)
+    def test_register_java_udaf(self):
+        self.spark.udf.registerJavaUDAF("javaUDAF", "test.org.apache.spark.sql.MyDoubleAvg")
+        df = self.spark.createDataFrame([(1, "a"), (2, "b"), (3, "a")], ["id", "name"])
+        df.createOrReplaceTempView("df")
+        row = self.spark.sql(
+            "SELECT name, javaUDAF(id) as avg from df group by name order by name desc").first()
+        self.assertEqual(row.asDict(), Row(name='b', avg=102.0).asDict())

Review comment:
       I think we could compare them as are. It's just to prevent an issue such as SPARK-29748 or similar issues in the future.




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org


[GitHub] [spark] dongjoon-hyun commented on pull request #28795: [SPARK-31965][TESTS][PYTHON] Move doctests related to Java function registration to test conditionally

Posted by GitBox <gi...@apache.org>.
dongjoon-hyun commented on pull request #28795:
URL: https://github.com/apache/spark/pull/28795#issuecomment-642382287


   Thank you for pinging me, @HyukjinKwon .


----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org