Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20021
thanks, merging to master!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20021
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20021
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85262/
Test PASSed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20021
**[Test build #85262 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85262/testReport)**
for PR 20021 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20021
**[Test build #85262 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85262/testReport)**
for PR 20021 at commit
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20021
retest this please
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20021
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85245/
Test FAILed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20021
Merged build finished. Test FAILed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20021
**[Test build #85245 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85245/testReport)**
for PR 20021 at commit
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/20021
LGTM too, thanks!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/20021
LGTM
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20021
**[Test build #85245 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85245/testReport)**
for PR 20021 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20021
Merged build finished. Test FAILed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/20021
retest this please.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20021
**[Test build #85240 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85240/testReport)**
for PR 20021 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20021
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85240/
Test FAILed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20021
**[Test build #85240 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85240/testReport)**
for PR 20021 at commit
Github user viirya commented on the issue:
https://github.com/apache/spark/pull/20021
Ok. I'm fine with only checking it in tests.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20021
Well, I proposed to check it only for tests at the beginning, but I don't
have a strong preference now as the new approach I took can guarantee that no
place would violate it, by looking at all
Github user mgaido91 commented on the issue:
https://github.com/apache/spark/pull/20021
Honestly, I liked very much doing the test only for testing and not
throwing an exception in production. IMHO it is an overkill to throw an
exception in production and in the remote case that we
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/20021
Oh, you are right. I misunderstood. After our optimizations, output is also
a part of `arguments`. Let me check others again.
---
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20021
> I checked some call sites. Here is one example that `extraArguments` has
`ev.value` instead of local variable.
Hey, `ev.value` is not from children, it's the output of the current
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20021
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20021
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85164/
Test PASSed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20021
**[Test build #85164 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85164/testReport)**
for PR 20021 at commit
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20021
**[Test build #85164 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85164/testReport)**
for PR 20021 at commit
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/20021
Jenkins, retest this please
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail:
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/20021
Here is [another
example](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/hash.scala#L414).
This is complicated.
`result` is
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20021
Merged build finished. Test FAILed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20021
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85150/
Test FAILed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20021
**[Test build #85150 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85150/testReport)**
for PR 20021 at commit
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/20021
I checked some call site. Here is [one
example](https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/hash.scala#L285)
that `arguments` has
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20021
**[Test build #85150 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85150/testReport)**
for PR 20021 at commit
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20021
> Why can we ensure any global variables are not passed to
ctx.splitExpressions?
You can also do this check. Look at the `arguments` parameter, all the
caller sides are using local
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20021
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85121/
Test PASSed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20021
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20021
**[Test build #85121 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85121/testReport)**
for PR 20021 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20021
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85120/
Test PASSed.
---
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20021
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20021
**[Test build #85120 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85120/testReport)**
for PR 20021 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20021
Merged build finished. Test PASSed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20021
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85112/
Test PASSed.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20021
**[Test build #85112 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85112/testReport)**
for PR 20021 at commit
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/20021
Here is [a fix](https://github.com/apache/spark/pull/19937) that avoids to
pass a global variable to `ctx.splitExpressions`. Before this PR, [this
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20021
**[Test build #85121 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85121/testReport)**
for PR 20021 at commit
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20021
> My question is what is a rule to decide whether a mutable state should be
localized or not
There is no rule as we don't need to localize global variables anymore.
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20021
**[Test build #85120 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85120/testReport)**
for PR 20021 at commit
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/20021
I see. I agree that to add assert helps all of developers.
My question is what is a rule to decide whether a mutable state should be
localized or not? I think that we have to ensure caller of
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20021
Merged build finished. Test FAILed.
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20021
**[Test build #85114 has
finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85114/testReport)**
for PR 20021 at commit
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/20021
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/85114/
Test FAILed.
---
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20021
Yea, I actually manually checked all the caller side of
`ctx.splitExpressions` and didn't find any issue, adding an assert helps me
prove it.
I also reverted some changes that tried to
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20021
**[Test build #85114 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85114/testReport)**
for PR 20021 at commit
Github user kiszk commented on the issue:
https://github.com/apache/spark/pull/20021
Does this PR just check whether the condition of approach 2 occurs or not?
If approach 2 does not replace with a temporary variable, assertion may occur.
Am I wrong?
---
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/20021
**[Test build #85112 has
started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/85112/testReport)**
for PR 20021 at commit
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/20021
cc @kiszk @viirya @mgaido91 @maropu @rednaxelafx @gatorsmile
---
-
To unsubscribe, e-mail:
56 matches
Mail list logo
