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-10546 Check partitionId's range in ExternalSorter#spill() #8703

Closed
wants to merge 5 commits into from
Closed

SPARK-10546 Check partitionId's range in ExternalSorter#spill() #8703

wants to merge 5 commits into from

Conversation

tedyu
Copy link
Contributor

@tedyu tedyu commented Sep 10, 2015

See this thread for background:
http://search-hadoop.com/m/q3RTt0rWvIkHAE81

We should check the range of partition Id and provide meaningful message through exception.

Alternatively, we can use abs() and modulo to force the partition Id into legitimate range. However, expectation is that user should correct the logic error in his / her code.

@SparkQA
Copy link

SparkQA commented Sep 10, 2015

Test build #42280 has finished for PR 8703 at commit 63dfe11.

  • This patch fails Scala style tests.
  • This patch merges cleanly.
  • This patch adds no public classes.

@andrewor14
Copy link
Contributor

please file JIRA

@tedyu tedyu changed the title Check partitionId's range in ExternalSorter#spill() SPARK-10546 Check partitionId's range in ExternalSorter#spill() Sep 10, 2015
@SparkQA
Copy link

SparkQA commented Sep 10, 2015

Test build #42287 has finished for PR 8703 at commit 04aa45e.

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

@srowen
Copy link
Member

srowen commented Sep 11, 2015

Can you just use one require statement to make this more compact? and string interpolation

@SparkQA
Copy link

SparkQA commented Sep 11, 2015

Test build #42333 has finished for PR 8703 at commit 0e86ae8.

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

@srowen
Copy link
Member

srowen commented Sep 11, 2015

LGTM

@SparkQA
Copy link

SparkQA commented Sep 11, 2015

Test build #42338 has finished for PR 8703 at commit a293906.

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

@asfgit asfgit closed this in b231ab8 Sep 11, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants