Skip to content

Conversation

@viirya
Copy link
Member

@viirya viirya commented Jul 12, 2021

What changes were proposed in this pull request?

A followup of #32980. We should use subExprCode to avoid duplicate call of addNewFunction.

Why are the changes needed?

Avoid duplicate all of addNewFunction.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existing test.

@viirya
Copy link
Member Author

viirya commented Jul 12, 2021

cc @cloud-fan

@SparkQA
Copy link

SparkQA commented Jul 12, 2021

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

@SparkQA
Copy link

SparkQA commented Jul 12, 2021

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

@SparkQA
Copy link

SparkQA commented Jul 12, 2021

Test build #140932 has finished for PR 33305 at commit 57d09e5.

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

@viirya
Copy link
Member Author

viirya commented Jul 13, 2021

cc @dongjoon-hyun @maropu

// at least two nodes) as the cost of doing it is expected to be low.

val subExprCode = s"${addNewFunction(fnName, fn)}($INPUT_ROW);"
subexprFunctions += s"${addNewFunction(fnName, fn)}($INPUT_ROW);"
Copy link
Member

Choose a reason for hiding this comment

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

oh, I see...

Copy link
Member Author

Choose a reason for hiding this comment

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

addNewFunction will overwrite in the method map. So this won't add new function. But it is better to avoid duplicatie call.

@viirya
Copy link
Member Author

viirya commented Jul 13, 2021

Thanks @maropu. Merging to master.

@viirya viirya closed this in 8ab1933 Jul 13, 2021
@viirya viirya deleted the fix-minor branch December 27, 2023 18:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants