spark-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From dongj...@apache.org
Subject [spark] branch branch-3.0 updated: [SPARK-32999][SQL] Use Utils.getSimpleName to avoid hitting Malformed class name in TreeNode
Date Sat, 26 Sep 2020 23:18:59 GMT
This is an automated email from the ASF dual-hosted git repository.

dongjoon pushed a commit to branch branch-3.0
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/branch-3.0 by this push:
     new 4425c3a  [SPARK-32999][SQL] Use Utils.getSimpleName to avoid hitting Malformed class
name in TreeNode
4425c3a is described below

commit 4425c3a9c35d4ab59976d8a409d18c933a5f9180
Author: Kris Mok <kris.mok@databricks.com>
AuthorDate: Sat Sep 26 16:03:59 2020 -0700

    [SPARK-32999][SQL] Use Utils.getSimpleName to avoid hitting Malformed 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 https://github.com/apache/spark/pull/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 <kris.mok@databricks.com>
    Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
    (cherry picked from commit 9a155d42a3202fbafc48f8b722bbc27cce522e11)
    Signed-off-by: Dongjoon Hyun <dhyun@apple.com>
---
 .../apache/spark/sql/catalyst/trees/TreeNode.scala |  9 +++++---
 .../spark/sql/catalyst/trees/TreeNodeSuite.scala   | 26 ++++++++++++++++++++++
 2 files changed, 32 insertions(+), 3 deletions(-)

diff --git a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala
b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala
index 4c74742..4dc3627 100644
--- a/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala
+++ b/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala
@@ -40,6 +40,7 @@ import org.apache.spark.sql.catalyst.util.truncatedString
 import org.apache.spark.sql.internal.SQLConf
 import org.apache.spark.sql.types._
 import org.apache.spark.storage.StorageLevel
+import org.apache.spark.util.Utils
 
 /** Used by [[TreeNode.getNodeNumbered]] when traversing the tree for a given number */
 private class MutableInt(var i: Int)
@@ -516,11 +517,13 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]] extends Product
{
     mapChildren(_.clone(), forceCopy = true)
   }
 
+  private def simpleClassName: String = Utils.getSimpleName(this.getClass)
+
   /**
    * Returns the name of this type of TreeNode.  Defaults to the class name.
    * Note that we remove the "Exec" suffix for physical operators here.
    */
-  def nodeName: String = getClass.getSimpleName.replaceAll("Exec$", "")
+  def nodeName: String = simpleClassName.replaceAll("Exec$", "")
 
   /**
    * The arguments that should be included in the arg string.  Defaults to the `productIterator`.
@@ -739,7 +742,7 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]] extends Product
{
   protected def jsonFields: List[JField] = {
     val fieldNames = getConstructorParameterNames(getClass)
     val fieldValues = productIterator.toSeq ++ otherCopyArgs
-    assert(fieldNames.length == fieldValues.length, s"${getClass.getSimpleName} fields: "
+
+    assert(fieldNames.length == fieldValues.length, s"$simpleClassName fields: " +
       fieldNames.mkString(", ") + s", values: " + fieldValues.mkString(", "))
 
     fieldNames.zip(fieldValues).map {
@@ -793,7 +796,7 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]] extends Product
{
       try {
         val fieldNames = getConstructorParameterNames(p.getClass)
         val fieldValues = p.productIterator.toSeq
-        assert(fieldNames.length == fieldValues.length, s"${getClass.getSimpleName} fields:
" +
+        assert(fieldNames.length == fieldValues.length, s"$simpleClassName fields: " +
           fieldNames.mkString(", ") + s", values: " + fieldValues.mkString(", "))
         ("product-class" -> JString(p.getClass.getName)) :: fieldNames.zip(fieldValues).map
{
           case (name, value) => name -> parseToJson(value)
diff --git a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/trees/TreeNodeSuite.scala
b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/trees/TreeNodeSuite.scala
index f525970..8ee64ed 100644
--- a/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/trees/TreeNodeSuite.scala
+++ b/sql/catalyst/src/test/scala/org/apache/spark/sql/catalyst/trees/TreeNodeSuite.scala
@@ -734,4 +734,30 @@ class TreeNodeSuite extends SparkFunSuite with SQLHelper {
     assertDifferentInstance(leaf, leafCloned)
     assert(leaf.child.eq(leafCloned.asInstanceOf[FakeLeafPlan].child))
   }
+
+  object MalformedClassObject extends Serializable {
+    case class MalformedNameExpression(child: Expression) extends TaggingExpression
+  }
+
+  test("SPARK-32999: TreeNode.nodeName should not throw malformed class name error") {
+    val testTriggersExpectedError = try {
+      classOf[MalformedClassObject.MalformedNameExpression].getSimpleName
+      false
+    } catch {
+      case ex: java.lang.InternalError if ex.getMessage.contains("Malformed class name")
=>
+        true
+      case ex: Throwable => throw ex
+    }
+    // This test case only applies on older JDK versions (e.g. JDK8u), and doesn't trigger
the
+    // issue on newer JDK versions (e.g. JDK11u).
+    assume(testTriggersExpectedError, "the test case didn't trigger malformed class name
error")
+
+    val expr = MalformedClassObject.MalformedNameExpression(Literal(1))
+    try {
+      expr.nodeName
+    } catch {
+      case ex: java.lang.InternalError if ex.getMessage.contains("Malformed class name")
=>
+        fail("TreeNode.nodeName should not throw malformed class name error")
+    }
+  }
 }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@spark.apache.org
For additional commands, e-mail: commits-help@spark.apache.org


Mime
View raw message