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

[minor] remove dead code in ExpressionEvalHelper #21958

Closed
wants to merge 1 commit into from

Conversation

cloud-fan
Copy link
Contributor

What changes were proposed in this pull request?

This addresses https://github.com/apache/spark/pull/21236/files#r207078480

both #21236 and #21838 add a InternalRow result check to ExpressionEvalHelper and becomes duplicated.

How was this patch tested?

N/A

@holdensmagicalunicorn
Copy link

@cloud-fan, thanks! I am a bot who has found some folks who might be able to help with the review:@ueshin, @rxin and @JoshRosen

@cloud-fan
Copy link
Contributor Author

cc @srowen @gatorsmile

@gatorsmile
Copy link
Member

LGTM

@SparkQA
Copy link

SparkQA commented Aug 2, 2018

Test build #93949 has finished for PR 21958 at commit 19dd614.

  • This patch fails due to an unknown error code, -9.
  • This patch merges cleanly.
  • This patch adds no public classes.

@ueshin
Copy link
Member

ueshin commented Aug 2, 2018

Jenkins, retest this please.

@ueshin
Copy link
Member

ueshin commented Aug 2, 2018

LGTM.

@SparkQA
Copy link

SparkQA commented Aug 2, 2018

Test build #93976 has finished for PR 21958 at commit 19dd614.

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

@cloud-fan
Copy link
Contributor Author

retest this please

@SparkQA
Copy link

SparkQA commented Aug 2, 2018

Test build #93987 has finished for PR 21958 at commit 19dd614.

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

@srowen
Copy link
Member

srowen commented Aug 2, 2018

Thanks, merged to master

@asfgit asfgit closed this in f04cd67 Aug 2, 2018
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.

6 participants