[SPARK-32999][SQL] Use Utils.getSimpleName to avoid hitting Malformed class name in TreeNode #29875

rednaxelafx · 2020-09-25T21:30:44Z

What changes were proposed in this pull request?

Use Utils.getSimpleName to avoid hitting Malformed class name error in TreeNode.

Why are the changes needed?

On older JDK versions (e.g. JDK8u), nested Scala classes may trigger java.lang.Class.getSimpleName to throw an java.lang.InternalError: Malformed class name error.

Similar to #29050, we should use Spark's Utils.getSimpleName utility function in place of Class.getSimpleName to avoid hitting the issue.

Does this PR introduce any user-facing change?

Fixes a bug that throws an error when invoking TreeNode.nodeName, otherwise no changes.

How was this patch tested?

Added new unit test case in TreeNodeSuite. Note that the test case assumes the test code can trigger the expected error, otherwise it'll skip the test safely, for compatibility with newer JDKs.

Manually tested on JDK8u and JDK11u and observed expected behavior:

JDK8u: the test case triggers the "Malformed class name" issue and the fix works;
JDK11u: the test case does not trigger the "Malformed class name" issue, and the test case is safely skipped.

… name in TreeNode

rednaxelafx · 2020-09-25T21:31:58Z

cc @Ngone51 @cloud-fan @dongjoon-hyun @viirya @HyukjinKwon @maropu

dongjoon-hyun · 2020-09-25T21:32:54Z

Thanks for pinging me.

SparkQA · 2020-09-25T22:11:48Z

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

SparkQA · 2020-09-25T22:33:33Z

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

SparkQA · 2020-09-26T02:11:22Z

Test build #129124 has finished for PR 29875 at commit 3f14f68.

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

SparkQA · 2020-09-26T03:23:05Z

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

SparkQA · 2020-09-26T03:39:40Z

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

SparkQA · 2020-09-26T07:05:02Z

Test build #129129 has finished for PR 29875 at commit d7aeded.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds the following public classes (experimental):
test(\"SPARK-32999: TreeNode.nodeName should not throw malformed class name error\")

viirya · 2020-09-26T19:27:43Z

retest this please

SparkQA · 2020-09-26T20:19:38Z

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

SparkQA · 2020-09-26T20:40:09Z

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

dongjoon-hyun

+1, LGTM. Thank you, @rednaxelafx and all.
I verified this in JDK8 and JDK11 and confirmed that it's JDK8 only issue as mentioned in the PR description.

Merged to master/3.0.

… class name in TreeNode ### What changes were proposed in this pull request? Use `Utils.getSimpleName` to avoid hitting `Malformed class name` error in `TreeNode`. ### Why are the changes needed? On older JDK versions (e.g. JDK8u), nested Scala classes may trigger `java.lang.Class.getSimpleName` to throw an `java.lang.InternalError: Malformed class name` error. Similar to #29050, we should use Spark's `Utils.getSimpleName` utility function in place of `Class.getSimpleName` to avoid hitting the issue. ### Does this PR introduce _any_ user-facing change? Fixes a bug that throws an error when invoking `TreeNode.nodeName`, otherwise no changes. ### How was this patch tested? Added new unit test case in `TreeNodeSuite`. Note that the test case assumes the test code can trigger the expected error, otherwise it'll skip the test safely, for compatibility with newer JDKs. Manually tested on JDK8u and JDK11u and observed expected behavior: - JDK8u: the test case triggers the "Malformed class name" issue and the fix works; - JDK11u: the test case does not trigger the "Malformed class name" issue, and the test case is safely skipped. Closes #29875 from rednaxelafx/spark-32999-getsimplename. Authored-by: Kris Mok <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit 9a155d4) Signed-off-by: Dongjoon Hyun <[email protected]>

dongjoon-hyun · 2020-09-26T23:10:48Z

Hi, @rednaxelafx . Could you make a backport to branch-2.4?

SparkQA · 2020-09-27T00:03:16Z

Test build #129135 has finished for PR 29875 at commit d7aeded.

This patch passes all tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
test(\"SPARK-32999: TreeNode.nodeName should not throw malformed class name error\")

maropu · 2020-09-27T13:06:31Z

Thanks for the fix, Kris!

… class name in TreeNode Use `Utils.getSimpleName` to avoid hitting `Malformed class name` error in `TreeNode`. On older JDK versions (e.g. JDK8u), nested Scala classes may trigger `java.lang.Class.getSimpleName` to throw an `java.lang.InternalError: Malformed class name` error. Similar to apache#29050, we should use Spark's `Utils.getSimpleName` utility function in place of `Class.getSimpleName` to avoid hitting the issue. Fixes a bug that throws an error when invoking `TreeNode.nodeName`, otherwise no changes. Added new unit test case in `TreeNodeSuite`. Note that the test case assumes the test code can trigger the expected error, otherwise it'll skip the test safely, for compatibility with newer JDKs. Manually tested on JDK8u and JDK11u and observed expected behavior: - JDK8u: the test case triggers the "Malformed class name" issue and the fix works; - JDK11u: the test case does not trigger the "Malformed class name" issue, and the test case is safely skipped. Closes apache#29875 from rednaxelafx/spark-32999-getsimplename. Authored-by: Kris Mok <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit 9a155d4) Signed-off-by: Kris Mok <[email protected]>

rednaxelafx · 2020-09-28T22:52:13Z

Hi @dongjoon-hyun , thanks for merging this PR. I've created #29896 for the 2.4 backport.

dongjoon-hyun · 2020-09-29T00:00:46Z

Thank you!

… class name in TreeNode ### What changes were proposed in this pull request? Use `Utils.getSimpleName` to avoid hitting `Malformed class name` error in `TreeNode`. ### Why are the changes needed? On older JDK versions (e.g. JDK8u), nested Scala classes may trigger `java.lang.Class.getSimpleName` to throw an `java.lang.InternalError: Malformed class name` error. Similar to apache#29050, we should use Spark's `Utils.getSimpleName` utility function in place of `Class.getSimpleName` to avoid hitting the issue. ### Does this PR introduce _any_ user-facing change? Fixes a bug that throws an error when invoking `TreeNode.nodeName`, otherwise no changes. ### How was this patch tested? Added new unit test case in `TreeNodeSuite`. Note that the test case assumes the test code can trigger the expected error, otherwise it'll skip the test safely, for compatibility with newer JDKs. Manually tested on JDK8u and JDK11u and observed expected behavior: - JDK8u: the test case triggers the "Malformed class name" issue and the fix works; - JDK11u: the test case does not trigger the "Malformed class name" issue, and the test case is safely skipped. Closes apache#29875 from rednaxelafx/spark-32999-getsimplename. Authored-by: Kris Mok <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]> (cherry picked from commit 9a155d4) Signed-off-by: Dongjoon Hyun <[email protected]>

### What changes were proposed in this pull request? In theory, we don't need #29875 anymore because we dropped JDK 8 (according to the PR description in #29875) but `Utils.getSimpleClass` handles malformed class names in any event, so should be safe to keep them. ### Why are the changes needed? To remove test that does not run. We dropped JDK 11/8 at SPARK-44112 ### Does this PR introduce _any_ user-facing change? No, test-only. ### How was this patch tested? CI in this PR should test them out. ### Was this patch authored or co-authored using generative AI tooling? No. Closes #43092 from HyukjinKwon/SPARK-45305. Authored-by: Hyukjin Kwon <[email protected]> Signed-off-by: Dongjoon Hyun <[email protected]>

SPARK-32999: Use Utils.getSimpleName to avoid hitting Malformed class…

3f14f68

… name in TreeNode

probot-autolabeler bot added the SQL label Sep 25, 2020

HyukjinKwon approved these changes Sep 26, 2020

View reviewed changes

Fixed JIRA ticket prefix in test case description

d7aeded

viirya approved these changes Sep 26, 2020

View reviewed changes

dongjoon-hyun approved these changes Sep 26, 2020

View reviewed changes

dongjoon-hyun closed this in 9a155d4 Sep 26, 2020

rednaxelafx mentioned this pull request Sep 28, 2020

[SPARK-32999][SQL][2.4] Use Utils.getSimpleName to avoid hitting Malformed class name in TreeNode #29896

Closed

HyukjinKwon mentioned this pull request Sep 25, 2023

[SPARK-45305][SQL][TESTS] Remove JDK 8 workaround added TreeNodeSuite #43092

Closed

[SPARK-32999][SQL] Use Utils.getSimpleName to avoid hitting Malformed class name in TreeNode #29875

[SPARK-32999][SQL] Use Utils.getSimpleName to avoid hitting Malformed class name in TreeNode #29875

Uh oh!

Conversation

rednaxelafx commented Sep 25, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

rednaxelafx commented Sep 25, 2020

Uh oh!

dongjoon-hyun commented Sep 25, 2020

Uh oh!

SparkQA commented Sep 25, 2020

Uh oh!

SparkQA commented Sep 25, 2020

Uh oh!

SparkQA commented Sep 26, 2020

Uh oh!

SparkQA commented Sep 26, 2020

Uh oh!

SparkQA commented Sep 26, 2020

Uh oh!

SparkQA commented Sep 26, 2020

Uh oh!

viirya commented Sep 26, 2020

Uh oh!

SparkQA commented Sep 26, 2020

Uh oh!

SparkQA commented Sep 26, 2020

Uh oh!

dongjoon-hyun left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dongjoon-hyun commented Sep 26, 2020

Uh oh!

SparkQA commented Sep 27, 2020

Uh oh!

maropu commented Sep 27, 2020

Uh oh!

rednaxelafx commented Sep 28, 2020

Uh oh!

dongjoon-hyun commented Sep 29, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

rednaxelafx commented Sep 25, 2020 •

edited

Loading

dongjoon-hyun left a comment •

edited

Loading