Skip to content
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

[SPARK-5038] Add explicit return type for implicit functions. #3860

Closed
wants to merge 1 commit into from

Conversation

rxin
Copy link
Contributor

@rxin rxin commented Dec 31, 2014

As we learned in #3580, not explicitly typing implicit functions can lead to compiler bugs and potentially unexpected runtime behavior.

This is a follow up PR for rest of Spark (outside Spark SQL). The original PR for Spark SQL can be found at #3859

This is a follow up PR for rest of Spark (outside Spark SQL).

The original PR for Spark SQL can be found at apache#3859
@SparkQA
Copy link

SparkQA commented Dec 31, 2014

Test build #24967 has started for PR 3860 at commit 73702f9.

  • This patch merges cleanly.

@SparkQA
Copy link

SparkQA commented Dec 31, 2014

Test build #24967 has finished for PR 3860 at commit 73702f9.

  • This patch fails Spark unit tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24967/
Test FAILed.

@rxin
Copy link
Contributor Author

rxin commented Dec 31, 2014

Jenkins, retest this please.

@SparkQA
Copy link

SparkQA commented Dec 31, 2014

Test build #24974 has started for PR 3860 at commit 73702f9.

  • This patch merges cleanly.

@JoshRosen
Copy link
Contributor

It looks like the org.apache.spark.scheduler.TaskResultGetterSuite.task retried if result missing from block manager test has somehow become non-deterministically flaky...

@SparkQA
Copy link

SparkQA commented Dec 31, 2014

Test build #24974 has finished for PR 3860 at commit 73702f9.

  • This patch passes all tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@AmplabJenkins
Copy link

Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/24974/
Test PASSed.

@JoshRosen
Copy link
Contributor

LGTM. I guess the only way that this could introduce problems would be if you changed the explicit type to be wider than the implicit type that was being inferred before, but it doesn't look like that's the case here.

@rxin
Copy link
Contributor Author

rxin commented Jan 1, 2015

Thanks. Merging in master.

@asfgit asfgit closed this in 7749dd6 Jan 1, 2015
@rxin rxin deleted the implicit branch January 1, 2015 01:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants