You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@spark.apache.org by we...@apache.org on 2022/07/05 08:47:49 UTC

[spark] branch master updated: [SPARK-39564][SQL][FOLLOWUP] Consider the case of serde available in CatalogTable on explain string for LogicalRelation

This is an automated email from the ASF dual-hosted git repository.

wenchen pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/spark.git


The following commit(s) were added to refs/heads/master by this push:
     new b37defef418 [SPARK-39564][SQL][FOLLOWUP] Consider the case of serde available in CatalogTable on explain string for LogicalRelation
b37defef418 is described below

commit b37defef4183c51e64fd9629d26ca6aecd320a1b
Author: Jungtaek Lim <ka...@gmail.com>
AuthorDate: Tue Jul 5 16:47:34 2022 +0800

    [SPARK-39564][SQL][FOLLOWUP] Consider the case of serde available in CatalogTable on explain string for LogicalRelation
    
    ### What changes were proposed in this pull request?
    
    This PR is a follow-up of #36963.
    
    With the change of #36963, LogicalRelation prints out the catalog table differently in explain on logical plan. It only prints out the qualified table name, and optionally also prints the class of serde if the information is available.
    
    #36963 does not account the part which can be optionally written (serde class). While this does not break "current" tests, it would be ideal to address the issue in prior.
    
    This PR proposes to introduce a new internal config to exclude serde on output of CatalogTable in SQL explain, which is intended to use only for test e.g. SQLQueryTestSuite.
    
    ### Why are the changes needed?
    
    It could probably be a possible bug in future, though it would be just a test side.
    
    ### Does this PR introduce _any_ user-facing change?
    
    No.
    
    ### How was this patch tested?
    
    Existing UTs. The internal test failed because of this (it exposed the serde class), and we fixed the test with this patch.
    
    Closes #37042 from HeartSaVioR/SPARK-39564-followup.
    
    Authored-by: Jungtaek Lim <ka...@gmail.com>
    Signed-off-by: Wenchen Fan <we...@databricks.com>
---
 .../main/scala/org/apache/spark/sql/catalyst/trees/TreeNode.scala   | 6 +++++-
 .../src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala     | 3 +++
 2 files changed, 8 insertions(+), 1 deletion(-)

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 8081a3edc81..71d8a0740bc 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
@@ -886,7 +886,11 @@ abstract class TreeNode[BaseType <: TreeNode[BaseType]] extends Product with Tre
 
   private def stringArgsForCatalogTable(table: CatalogTable): Seq[Any] = {
     table.storage.serde match {
-      case Some(serde) => table.identifier :: serde :: Nil
+      case Some(serde)
+        // SPARK-39564: don't print out serde to avoid introducing complicated and error-prone
+        // regex magic.
+        if !SQLConf.get.getConfString("spark.test.noSerdeInExplain", "false").toBoolean =>
+        table.identifier :: serde :: Nil
       case _ => table.identifier :: Nil
     }
   }
diff --git a/sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala b/sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
index 0601dce1d4b..bd48d173039 100644
--- a/sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
+++ b/sql/core/src/test/scala/org/apache/spark/sql/SQLQueryTestSuite.scala
@@ -147,6 +147,9 @@ class SQLQueryTestSuite extends QueryTest with SharedSparkSession with SQLHelper
     .set(SQLConf.SHUFFLE_PARTITIONS, 4)
     // use Java 8 time API to handle negative years properly
     .set(SQLConf.DATETIME_JAVA8API_ENABLED, true)
+    // SPARK-39564: don't print out serde to avoid introducing complicated and error-prone
+    // regex magic.
+    .set("spark.test.noSerdeInExplain", "true")
 
   // SPARK-32106 Since we add SQL test 'transform.sql' will use `cat` command,
   // here we need to ignore it.


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