You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by HyukjinKwon <gi...@git.apache.org> on 2018/01/26 15:06:03 UTC
[GitHub] spark pull request #20409: Reset the cache in asNondeterministic to set dete...
GitHub user HyukjinKwon opened a pull request:
https://github.com/apache/spark/pull/20409
Reset the cache in asNondeterministic to set deterministic properly
## What changes were proposed in this pull request?
Reproducer:
```python
from pyspark.sql.functions import udf
f = udf(lambda x: x)
spark.range(1).select(f("id")) # cache JVM UDF instance.
f = f.asNondeterministic()
spark.range(1).select(f("id"))._jdf.logicalPlan().projectList().head().deterministic()
```
It should return `False` but the current master returns `True`. Seems it's because we cache the JVM UDF instance and then we reuse it even after setting `deterministic` enabled once it's called.
For an easy reproducer, with the diff below:
```diff
diff --git a/python/pyspark/sql/udf.py b/python/pyspark/sql/udf.py
index de96846c5c7..026a78bf547 100644
--- a/python/pyspark/sql/udf.py
+++ b/python/pyspark/sql/udf.py
@@ -180,6 +180,7 @@ class UserDefinedFunction(object):
wrapper.deterministic = self.deterministic
wrapper.asNondeterministic = functools.wraps(
self.asNondeterministic)(lambda: self.asNondeterministic()._wrapped())
+ wrapper._unwrapped = lambda: self
return wrapper
def asNondeterministic(self):
```
**Before**
```python
>>> from pyspark.sql.functions import udf
>>> f = udf(lambda x: x)
>>> spark.range(1).select(f("id"))
DataFrame[<lambda>(id): string]
>>> f._unwrapped()._judf_placeholder.udfDeterministic()
True
>>> ndf = f.asNondeterministic()
>>> ndf.deterministic
False
>>> spark.range(1).select(ndf("id"))
DataFrame[<lambda>(id): string]
>>> ndf._unwrapped()._judf_placeholder.udfDeterministic()
True
```
**After**
```python
>>> from pyspark.sql.functions import udf
>>> f = udf(lambda x: x)
>>> spark.range(1).select(f("id"))
DataFrame[<lambda>(id): string]
>>> f._unwrapped()._judf_placeholder.udfDeterministic()
True
>>> ndf = f.asNondeterministic()
>>> ndf.deterministic
False
>>> spark.range(1).select(ndf("id"))
DataFrame[<lambda>(id): string]
>>> ndf._unwrapped()._judf_placeholder.udfDeterministic()
False
```
## How was this patch tested?
Manually tested. I am not sure if I should add the test with a lot of JVM accesses.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/HyukjinKwon/spark SPARK-23233
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/20409.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #20409
----
commit 317c4fd54ecb707b92088c62aebd551805ecae8f
Author: hyukjinkwon <gu...@...>
Date: 2018-01-26T15:01:22Z
Reset the cache in asNondeterministic to set deterministic properly
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20409
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/20409
About test case, can't we just use the reproducer in the PR description to check it we change the deterministic status of udf?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20409: [SPARK-23233][PYTHON] Reset the cache in asNondet...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/20409
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20409
**[Test build #86729 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86729/testReport)** for PR 20409 at commit [`b23ff02`](https://github.com/apache/spark/commit/b23ff02f543ecc92db574b808ea00f9ff7d236f8).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20409
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20409
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/305/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20409
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86729/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20409
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86707/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20409
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20409
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/287/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20409
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20409
**[Test build #86706 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86706/testReport)** for PR 20409 at commit [`317c4fd`](https://github.com/apache/spark/commit/317c4fd54ecb707b92088c62aebd551805ecae8f).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20409
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/86706/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20409
Thanks! Merged to master/2.3
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20409
test this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20409: [SPARK-23233][PYTHON] Reset the cache in asNondet...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/20409#discussion_r164178516
--- Diff: python/pyspark/sql/udf.py ---
@@ -188,6 +188,9 @@ def asNondeterministic(self):
.. versionadded:: 2.3
"""
+ # Here, we explicitly clean the cache to create a JVM UDF instance
+ # with 'deterministic' updated. See SPARK-23233.
+ self._judf_placeholder = None
--- End diff --
Add a test case?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20409
**[Test build #86729 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86729/testReport)** for PR 20409 at commit [`b23ff02`](https://github.com/apache/spark/commit/b23ff02f543ecc92db574b808ea00f9ff7d236f8).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: Reset the cache in asNondeterministic to set determinist...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20409
cc @ueshin and @viirya, could you take a look please when you have some time?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20409
**[Test build #86707 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86707/testReport)** for PR 20409 at commit [`317c4fd`](https://github.com/apache/spark/commit/317c4fd54ecb707b92088c62aebd551805ecae8f).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20409
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20409
Thank you @gatorsmile and @viirya.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20409
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20409
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20409
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #20409: [SPARK-23233][PYTHON] Reset the cache in asNondet...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on a diff in the pull request:
https://github.com/apache/spark/pull/20409#discussion_r164242518
--- Diff: python/pyspark/sql/udf.py ---
@@ -188,6 +188,9 @@ def asNondeterministic(self):
.. versionadded:: 2.3
"""
+ # Here, we explicitly clean the cache to create a JVM UDF instance
+ # with 'deterministic' updated. See SPARK-23233.
+ self._judf_placeholder = None
--- End diff --
Do you mean the reproducer above?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/20409
LGTM
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20409
**[Test build #86706 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86706/testReport)** for PR 20409 at commit [`317c4fd`](https://github.com/apache/spark/commit/317c4fd54ecb707b92088c62aebd551805ecae8f).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20409
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution/286/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20409
FYI @ptkool.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20409
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by viirya <gi...@git.apache.org>.
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/20409
retest this please.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by HyukjinKwon <gi...@git.apache.org>.
Github user HyukjinKwon commented on the issue:
https://github.com/apache/spark/pull/20409
I was hesitant because it accesses to a lot of internal instances via JVM as I wrote in the PR description. Let me just use it as a test.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #20409: [SPARK-23233][PYTHON] Reset the cache in asNondeterminis...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20409
**[Test build #86707 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/86707/testReport)** for PR 20409 at commit [`317c4fd`](https://github.com/apache/spark/commit/317c4fd54ecb707b92088c62aebd551805ecae8f).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org