Skip to content

[SPARK-26294][CORE]Delete Unnecessary If statement #23247

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

Closed
wants to merge 1 commit into from

Conversation

wangjiaochun
Copy link
Contributor

@wangjiaochun wangjiaochun commented Dec 6, 2018

What changes were proposed in this pull request?

Delete unnecessary If statement, because it Impossible execution when
records less than or equal to zero.it is only execution when records begin zero.
...................
if (inMemSorter == null || inMemSorter.numRecords() <= 0) {
return 0L;
}
....................
if (inMemSorter.numRecords() > 0) {
.....................
}

How was this patch tested?

Existing tests

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

Please review http://spark.apache.org/contributing.html before opening a pull request.

@HyukjinKwon
Copy link
Member

@wangjiaochun, I think you better stop fixing trivial stuff in each PR. Those stuff can be fixed when the codes around here is fixed, or let other people fix it later.

@srowen
Copy link
Member

srowen commented Dec 6, 2018

These aren't worth the time it takes us to review them and merge them, honestly. Little cleanup can be OK if it makes an appreciable difference in speed or readability, and if you can find many instances of the same issue.

@SparkQA
Copy link

SparkQA commented Dec 7, 2018

Test build #4456 has finished for PR 23247 at commit d5aea03.

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

@wangjiaochun
Copy link
Contributor Author

wangjiaochun commented Dec 7, 2018

ok! thanks, I will focus on commit bug later.originally intended to put these trivial stuff in a PR,but worrying about unclear descriptions, so using multiple PR, It will not happen later @HyukjinKwon @srowen

@srowen
Copy link
Member

srowen commented Dec 7, 2018

Merged to master

@asfgit asfgit closed this in 9b7679a Dec 7, 2018
jackylee-ch pushed a commit to jackylee-ch/spark that referenced this pull request Feb 18, 2019
## What changes were proposed in this pull request?
Delete unnecessary If statement, because it Impossible execution when
records less than or equal to zero.it is only execution when records begin zero.
...................
if (inMemSorter == null || inMemSorter.numRecords() <= 0) {
       return 0L;
 }
....................
if (inMemSorter.numRecords() > 0) {
.....................
}
## How was this patch tested?
Existing tests

(Please explain how this patch was tested. E.g. unit tests, integration tests, manual tests)
(If this patch involves UI changes, please attach a screenshot; otherwise, remove this)

Please review http://spark.apache.org/contributing.html before opening a pull request.

Closes apache#23247 from wangjiaochun/inMemSorter.

Authored-by: 10087686 <wang.jiaochun@zte.com.cn>
Signed-off-by: Sean Owen <sean.owen@databricks.com>
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