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-37268][SQL] Remove unused method call in FileScanRDD #34545

Closed
wants to merge 1 commit into from

Conversation

zuston
Copy link
Member

@zuston zuston commented Nov 10, 2021

What changes were proposed in this pull request?

Remove unused method call in FileScanRDD

Why are the changes needed?

In PR of SPARK-25674, it directly uses inputMetrics.recordsRead instead of using preNumRecordsRead. So it's necessary to remove unused method call.

Does this PR introduce any user-facing change?

No

How was this patch tested?

No.

@github-actions github-actions bot added the SQL label Nov 10, 2021
@HyukjinKwon HyukjinKwon changed the title [SPARK-37268] Remove unused method call in FileScanRDD [SPARK-37268][SQL] Remove unused method call in FileScanRDD Nov 11, 2021
@@ -107,7 +107,6 @@ class FileScanRDD(
val nextElement = currentIterator.next()
// TODO: we should have a better separation of row based and batch based scan, so that we
// don't need to run this `if` for every record.
val preNumRecordsRead = inputMetrics.recordsRead
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks fine but it would have been better which PR removed the last usage of this in order to make any mistake.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed from SPARK-25674.

This PR directly uses inputMetrics.recordsRead instead of using preNumRecordsRead

@HyukjinKwon
Copy link
Member

@zuston please fill the PR description properly.

@zuston
Copy link
Member Author

zuston commented Nov 11, 2021

@zuston please fill the PR description properly.

Done. @HyukjinKwon

@HyukjinKwon
Copy link
Member

ok to test

@SparkQA
Copy link

SparkQA commented Nov 11, 2021

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49576/

@SparkQA
Copy link

SparkQA commented Nov 11, 2021

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/49576/

@SparkQA
Copy link

SparkQA commented Nov 11, 2021

Test build #145107 has finished for PR 34545 at commit e8e20c6.

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

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM. Thank you, @zuston and @HyukjinKwon . I also confirmed that SPARK-25647 remove it. Merged to master for Apache Spark 3.3.

@dongjoon-hyun
Copy link
Member

Welcome to the Apache Spark community, @zuston .
I added you to the Apache Spark contributor group and assigned SPARK-37268 to you.

@zuston
Copy link
Member Author

zuston commented Nov 12, 2021

Welcome to the Apache Spark community, @zuston . I added you to the Apache Spark contributor group and assigned SPARK-37268 to you.

Glad to join Spark community. Thanks @dongjoon-hyun @HyukjinKwon.

@zuston zuston deleted the patch-1 branch November 12, 2021 02:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants