You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by liancheng <gi...@git.apache.org> on 2016/05/12 15:34:20 UTC

[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

GitHub user liancheng opened a pull request:

    https://github.com/apache/spark/pull/13079

    [SPARK-14346][SQL] Native SHOW CREATE TABLE for Hive tables/views

    ## What changes were proposed in this pull request?
    
    This is a follow-up of #12781. It adds native `SHOW CREATE TABLE` support for Hive tables and views. A new field `fullyMapped` is added to `CatalogTable` to indicate whether all table metadata retrieved from the concrete underlying external catalog (i.e. Hive metastore in this case) can be mapped to fields in `CatalogTable`. This flag is useful when the target Hive table contains structures that can't be handled by Spark SQL, e.g., skewed columns and storage handler, etc..
    
    ## How was this patch tested?
    
    New test cases are added in `ShowCreateTableSuite` to do round-trip tests.

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/liancheng/spark spark-14346-show-create-table-for-hive-tables

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/spark/pull/13079.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #13079
    
----
commit b3cc55fcc7be773cb322042dc16303dc2abcdb53
Author: Cheng Lian <li...@databricks.com>
Date:   2016-05-12T15:23:03Z

    Native SHOW CREATE TABLE for Hive tables/views

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/13079#issuecomment-219754482
  
    **[Test build #58697 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58697/consoleFull)** for PR 13079 at commit [`74fd5d3`](https://github.com/apache/spark/commit/74fd5d342459e844bc41ed86ef9bf04d7d092b3e).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/13079#issuecomment-219784650
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58697/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/13079#issuecomment-219784277
  
    **[Test build #58697 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58697/consoleFull)** for PR 13079 at commit [`74fd5d3`](https://github.com/apache/spark/commit/74fd5d342459e844bc41ed86ef9bf04d7d092b3e).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63394602
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
    @@ -95,7 +101,8 @@ case class CatalogTable(
         properties: Map[String, String] = Map.empty,
         viewOriginalText: Option[String] = None,
         viewText: Option[String] = None,
    -    comment: Option[String] = None) {
    +    comment: Option[String] = None,
    +    fullyMapped: Boolean = true) {
    --- End diff --
    
    +1


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63543727
  
    --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/ShowCreateTableSuite.scala ---
    @@ -116,24 +116,143 @@ class ShowCreateTableSuite extends QueryTest with SQLTestUtils with TestHiveSing
             .bucketBy(2, "c", "d")
             .saveAsTable("ddl_test5")
     
    -      checkCreateTable(TableIdentifier("ddl_test5", Some("default")))
    +      checkCreateTable("ddl_test5")
    +    }
    +  }
    +
    +  test("simple hive table") {
    +    withTable("t1") {
    +      sql(
    +        s"""CREATE TABLE t1 (
    +           |  c1 INT COMMENT 'bla',
    +           |  c2 STRING
    +           |)
    +           |TBLPROPERTIES (
    +           |  'prop1' = 'value1',
    +           |  'prop2' = 'value2'
    +           |)
    +         """.stripMargin
    +      )
    +
    +      checkCreateTable("t1")
    +    }
    +  }
    +
    +  test("simple external hive table") {
    +    withTempDir { dir =>
    +      withTable("t1") {
    +        sql(
    +          s"""CREATE TABLE t1 (
    +             |  c1 INT COMMENT 'bla',
    +             |  c2 STRING
    +             |)
    +             |LOCATION '$dir'
    +             |TBLPROPERTIES (
    +             |  'prop1' = 'value1',
    +             |  'prop2' = 'value2'
    +             |)
    +           """.stripMargin
    +        )
    +
    +        checkCreateTable("t1")
    +      }
    +    }
    +  }
    +
    +  test("partitioned hive table") {
    +    withTable("t1") {
    +      sql(
    +        s"""CREATE TABLE t1 (
    +           |  c1 INT COMMENT 'bla',
    +           |  c2 STRING
    +           |)
    +           |COMMENT 'bla'
    +           |PARTITIONED BY (
    +           |  p1 BIGINT COMMENT 'bla',
    +           |  p2 STRING
    +           |)
    +         """.stripMargin
    +      )
    +
    +      checkCreateTable("t1")
    +    }
    +  }
    +
    +  test("hive table with explicit storage info") {
    +    withTable("t1") {
    +      sql(
    +        s"""CREATE TABLE t1 (
    +           |  c1 INT COMMENT 'bla',
    +           |  c2 STRING
    +           |)
    +           |ROW FORMAT DELIMITED FIELDS TERMINATED BY ','
    +           |COLLECTION ITEMS TERMINATED BY '@'
    +           |MAP KEYS TERMINATED BY '#'
    +           |NULL DEFINED AS 'NaN'
    +           |STORED AS PARQUET
    +         """.stripMargin
    +      )
    +
    +      checkCreateTable("t1")
    +    }
    +  }
    +
    +  test("hive table with serde info") {
    +    withTable("t1") {
    +      sql(
    +        s"""CREATE TABLE t1 (
    +           |  c1 INT COMMENT 'bla',
    +           |  c2 STRING
    +           |)
    +           |ROW FORMAT SERDE 'org.apache.hadoop.hive.ql.io.parquet.serde.ParquetHiveSerDe'
    +           |WITH SERDEPROPERTIES (
    +           |  'mapkey.delim' = ',',
    +           |  'field.delim' = ','
    +           |)
    +           |STORED AS
    +           |  INPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat'
    +           |  OUTPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetOutputFormat'
    +         """.stripMargin
    +      )
    +
    +      checkCreateTable("t1")
    +    }
    +  }
    +
    +  test("hive view") {
    +    withView("v1") {
    +      sql("CREATE VIEW v1 AS SELECT 1 AS a")
    +      checkCreateView("v1")
    +    }
    +  }
    +
    +  test("hive view with output columns") {
    +    withView("v1") {
    +      sql("CREATE VIEW v1 (b) AS SELECT 1 AS a")
    +      checkCreateView("v1")
    --- End diff --
    
    Test added.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63620803
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
    @@ -200,6 +207,7 @@ case class SimpleCatalogRelation(
         }
       }
     
    -  require(metadata.identifier.database == Some(databaseName),
    +  require(
    +    metadata.identifier.database.contains(databaseName),
    --- End diff --
    
    `contains` is not in scala 2.10. Let me fixing the build.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:

    https://github.com/apache/spark/pull/13079#issuecomment-218803458
  
    cc @yhuai @cloud-fan


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by xwu0226 <gi...@git.apache.org>.
Github user xwu0226 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63066039
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
    @@ -626,40 +626,142 @@ case class ShowCreateTableCommand(table: TableIdentifier) extends RunnableComman
         val stmt = if (DDLUtils.isDatasourceTable(tableMetadata)) {
           showCreateDataSourceTable(tableMetadata)
         } else {
    -      throw new UnsupportedOperationException(
    -        "SHOW CREATE TABLE only supports Spark SQL data source tables.")
    +      showCreateHiveTable(tableMetadata)
         }
     
         Seq(Row(stmt))
       }
     
    +  private def showCreateHiveTable(metadata: CatalogTable): String = {
    +    def reportUnsupportedError(): Unit = {
    +      throw new UnsupportedOperationException(
    +        s"Failed to execute SHOW CREATE TABLE against table ${metadata.identifier.quotedString}, " +
    +          "because it contains table structure(s) (e.g. skewed columns) that Spark SQL doesn't " +
    +          "support yet."
    +      )
    +    }
    +
    +    if (!metadata.fullyMapped) {
    +      reportUnsupportedError()
    +    }
    +
    +    val builder = StringBuilder.newBuilder
    +
    +    val tableTypeString = metadata.tableType match {
    +      case EXTERNAL => " EXTERNAL TABLE"
    +      case VIEW => " VIEW"
    +      case MANAGED => " TABLE"
    +      case INDEX => reportUnsupportedError()
    +    }
    +
    +    builder ++= s"CREATE$tableTypeString ${table.quotedString}"
    +
    +    if (metadata.tableType == VIEW) {
    +      if (metadata.schema.nonEmpty) {
    +        builder ++= metadata.schema.map(_.name).mkString("(", ", ", ")")
    +      }
    +      builder ++= metadata.viewOriginalText.mkString(" AS\n", "", "\n")
    +    } else {
    +      showHiveTableDataColumns(metadata, builder)
    +      showHiveTableNonDataColumns(metadata, builder)
    +      showHiveTableStorageInfo(metadata, builder)
    +      showHiveTableProperties(metadata, builder)
    +    }
    +
    +    builder.toString()
    +  }
    +
    +  private def showHiveTableDataColumns(metadata: CatalogTable, builder: StringBuilder): Unit = {
    +    val columns = metadata.schema.filterNot { column =>
    +      metadata.partitionColumnNames.contains(column.name)
    +    }.map(columnToDDLFragment)
    +
    +    if (columns.nonEmpty) {
    +      builder ++= columns.mkString("(", ", ", ")\n")
    +    }
    +  }
    +
    +  private def columnToDDLFragment(column: CatalogColumn): String = {
    +    val comment = column.comment.map(escapeSingleQuotedString).map(" COMMENT '" + _ + "'")
    +    s"${quoteIdentifier(column.name)} ${column.dataType}${comment.getOrElse("")}"
    +  }
    +
    +  private def showHiveTableNonDataColumns(metadata: CatalogTable, builder: StringBuilder): Unit = {
    +    if (metadata.partitionColumns.nonEmpty) {
    +      val partCols = metadata.partitionColumns.map(columnToDDLFragment)
    +      builder ++= partCols.mkString("PARTITIONED BY (", ", ", ")\n")
    +    }
    +
    +    if (metadata.bucketColumnNames.nonEmpty) {
    +      builder ++= metadata.bucketColumnNames.mkString("CLUSTERED BY (", ", ", ")\n")
    +
    +      if (metadata.sortColumnNames.nonEmpty) {
    +        builder ++= metadata.bucketColumnNames.mkString("SORTED BY (", ", ", ")\n")
    +      }
    +
    +      builder ++= s"INTO ${metadata.numBuckets} BUCKETS\n"
    +    }
    +  }
    +
    +  private def showHiveTableStorageInfo(metadata: CatalogTable, builder: StringBuilder): Unit = {
    +    val storage = metadata.storage
    +
    +    storage.serde.foreach { serde =>
    +      builder ++= s"ROW FORMAT SERDE '$serde'\n"
    +
    +      val serdeProps = metadata.storage.serdeProperties.map {
    +        case (key, value) =>
    +          s"'${escapeSingleQuotedString(key)}' = '${escapeSingleQuotedString(value)}'"
    +      }
    +
    +      builder ++= serdeProps.mkString("WITH SERDEPROPERTIES (", ",\n  ", "\n)\n")
    +    }
    +
    +    if (storage.inputFormat.isDefined || storage.outputFormat.isDefined) {
    +      builder ++= "STORED AS\n"
    +
    +      storage.inputFormat.foreach { format =>
    +        builder ++= s"  INPUTFORMAT '${escapeSingleQuotedString(format)}'\n"
    +      }
    +
    +      storage.outputFormat.foreach { format =>
    +        builder ++= s"  OUTPUTFORMAT '${escapeSingleQuotedString(format)}'\n"
    +      }
    +    }
    +
    +    if (metadata.tableType == EXTERNAL) {
    +      storage.locationUri.foreach { uri =>
    +        builder ++= s"LOCATION '$uri'\n"
    +      }
    +    }
    +  }
    +
    +  private def showHiveTableProperties(metadata: CatalogTable, builder: StringBuilder): Unit = {
    +    if (metadata.properties.nonEmpty) {
    +      val props = metadata.properties.map { case (key, value) =>
    +        s"'${escapeSingleQuotedString(key)}' = '${escapeSingleQuotedString(value)}'"
    +      }
    +
    +      builder ++= props.mkString("TBLPROPERTIES (", ",\n  ", ")\n")
    --- End diff --
    
    Will there be duplicate property key in this list? For example, if the table was created as EXTERNAL table, the generated DDL will be `CREATE EXTERNAL TABLE ...`, but the `TBLPROPERTIES` may still contain `EXTERNAL` -> `true`. I see Hive's logic purposely skip printing this key from `TBLPROPERTIES`. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63488983
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
    @@ -95,7 +101,8 @@ case class CatalogTable(
         properties: Map[String, String] = Map.empty,
         viewOriginalText: Option[String] = None,
         viewText: Option[String] = None,
    -    comment: Option[String] = None) {
    +    comment: Option[String] = None,
    +    fullyMapped: Boolean = true) {
    --- End diff --
    
    Thanks, renamed to `hasUnsupportedFeatures`.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63108319
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
    @@ -626,40 +626,142 @@ case class ShowCreateTableCommand(table: TableIdentifier) extends RunnableComman
         val stmt = if (DDLUtils.isDatasourceTable(tableMetadata)) {
           showCreateDataSourceTable(tableMetadata)
         } else {
    -      throw new UnsupportedOperationException(
    -        "SHOW CREATE TABLE only supports Spark SQL data source tables.")
    +      showCreateHiveTable(tableMetadata)
         }
     
         Seq(Row(stmt))
       }
     
    +  private def showCreateHiveTable(metadata: CatalogTable): String = {
    +    def reportUnsupportedError(): Unit = {
    +      throw new UnsupportedOperationException(
    --- End diff --
    
    also, can you add tests for this case?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63710152
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
    @@ -626,40 +626,149 @@ case class ShowCreateTableCommand(table: TableIdentifier) extends RunnableComman
         val stmt = if (DDLUtils.isDatasourceTable(tableMetadata)) {
           showCreateDataSourceTable(tableMetadata)
         } else {
    -      throw new UnsupportedOperationException(
    -        "SHOW CREATE TABLE only supports Spark SQL data source tables.")
    +      showCreateHiveTable(tableMetadata)
         }
     
         Seq(Row(stmt))
       }
     
    +  private def showCreateHiveTable(metadata: CatalogTable): String = {
    +    def reportUnsupportedError(): Unit = {
    +      throw new UnsupportedOperationException(
    +        s"Failed to execute SHOW CREATE TABLE against table ${metadata.identifier.quotedString}, " +
    +          "because it contains table structure(s) (e.g. skewed columns) that Spark SQL doesn't " +
    +          "support yet."
    --- End diff --
    
    Replaced `hasUnsupportedFeatures: Boolean` with `unsupportedFeatures: Seq[String]`, which holds string descriptions of unsupported features, so that we can list them in the exception message.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63105105
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
    @@ -79,6 +79,12 @@ case class CatalogTablePartition(
      *
      * Note that Hive's metastore also tracks skewed columns. We should consider adding that in the
      * future once we have a better understanding of how we want to handle skewed columns.
    + *
    + * Field `fullyMapped` is used to indicate whether all table metadata entries retrieved from the
    --- End diff --
    
    you can use `@param` here


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63108072
  
    --- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
    @@ -338,6 +338,14 @@ private[hive] class HiveClientImpl(
           // partition columns are part of the schema
           val partCols = h.getPartCols.asScala.map(fromHiveColumn)
           val schema = h.getCols.asScala.map(fromHiveColumn) ++ partCols
    +
    +      // Skew spec, storage handler, and bucketing info can't be mapped to CatalogTable (yet)
    +      val fullyMapped = (
    +        h.getSkewedColNames.isEmpty
    +          && h.getStorageHandler == null
    +          && h.getBucketCols.isEmpty
    +      )
    --- End diff --
    
    nit: can you use `{ }` here instead? We usually don't use `( )` for things like these


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63106023
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
    @@ -626,40 +626,142 @@ case class ShowCreateTableCommand(table: TableIdentifier) extends RunnableComman
         val stmt = if (DDLUtils.isDatasourceTable(tableMetadata)) {
           showCreateDataSourceTable(tableMetadata)
         } else {
    -      throw new UnsupportedOperationException(
    -        "SHOW CREATE TABLE only supports Spark SQL data source tables.")
    +      showCreateHiveTable(tableMetadata)
         }
     
         Seq(Row(stmt))
       }
     
    +  private def showCreateHiveTable(metadata: CatalogTable): String = {
    +    def reportUnsupportedError(): Unit = {
    +      throw new UnsupportedOperationException(
    +        s"Failed to execute SHOW CREATE TABLE against table ${metadata.identifier.quotedString}, " +
    +          "because it contains table structure(s) (e.g. skewed columns) that Spark SQL doesn't " +
    +          "support yet."
    +      )
    +    }
    +
    +    if (!metadata.fullyMapped) {
    +      reportUnsupportedError()
    +    }
    +
    +    val builder = StringBuilder.newBuilder
    +
    +    val tableTypeString = metadata.tableType match {
    +      case EXTERNAL => " EXTERNAL TABLE"
    +      case VIEW => " VIEW"
    +      case MANAGED => " TABLE"
    +      case INDEX => reportUnsupportedError()
    +    }
    +
    +    builder ++= s"CREATE$tableTypeString ${table.quotedString}"
    +
    +    if (metadata.tableType == VIEW) {
    +      if (metadata.schema.nonEmpty) {
    +        builder ++= metadata.schema.map(_.name).mkString("(", ", ", ")")
    +      }
    +      builder ++= metadata.viewOriginalText.mkString(" AS\n", "", "\n")
    +    } else {
    +      showHiveTableDataColumns(metadata, builder)
    +      showHiveTableNonDataColumns(metadata, builder)
    +      showHiveTableStorageInfo(metadata, builder)
    +      showHiveTableProperties(metadata, builder)
    +    }
    +
    +    builder.toString()
    +  }
    +
    +  private def showHiveTableDataColumns(metadata: CatalogTable, builder: StringBuilder): Unit = {
    +    val columns = metadata.schema.filterNot { column =>
    +      metadata.partitionColumnNames.contains(column.name)
    +    }.map(columnToDDLFragment)
    +
    +    if (columns.nonEmpty) {
    +      builder ++= columns.mkString("(", ", ", ")\n")
    +    }
    +  }
    +
    +  private def columnToDDLFragment(column: CatalogColumn): String = {
    +    val comment = column.comment.map(escapeSingleQuotedString).map(" COMMENT '" + _ + "'")
    +    s"${quoteIdentifier(column.name)} ${column.dataType}${comment.getOrElse("")}"
    +  }
    +
    +  private def showHiveTableNonDataColumns(metadata: CatalogTable, builder: StringBuilder): Unit = {
    +    if (metadata.partitionColumns.nonEmpty) {
    +      val partCols = metadata.partitionColumns.map(columnToDDLFragment)
    +      builder ++= partCols.mkString("PARTITIONED BY (", ", ", ")\n")
    +    }
    +
    +    if (metadata.bucketColumnNames.nonEmpty) {
    --- End diff --
    
    if you remove this then you probably don't need this helper method.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63108270
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
    @@ -626,40 +626,142 @@ case class ShowCreateTableCommand(table: TableIdentifier) extends RunnableComman
         val stmt = if (DDLUtils.isDatasourceTable(tableMetadata)) {
           showCreateDataSourceTable(tableMetadata)
         } else {
    -      throw new UnsupportedOperationException(
    -        "SHOW CREATE TABLE only supports Spark SQL data source tables.")
    +      showCreateHiveTable(tableMetadata)
         }
     
         Seq(Row(stmt))
       }
     
    +  private def showCreateHiveTable(metadata: CatalogTable): String = {
    +    def reportUnsupportedError(): Unit = {
    +      throw new UnsupportedOperationException(
    --- End diff --
    
    Elsewhere we normally throw `AnalysisException("Operation nor allowed: ...")`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:

    https://github.com/apache/spark/pull/13079#issuecomment-218822204
  
    test this please


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63412808
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
    @@ -626,40 +626,142 @@ case class ShowCreateTableCommand(table: TableIdentifier) extends RunnableComman
         val stmt = if (DDLUtils.isDatasourceTable(tableMetadata)) {
           showCreateDataSourceTable(tableMetadata)
         } else {
    -      throw new UnsupportedOperationException(
    -        "SHOW CREATE TABLE only supports Spark SQL data source tables.")
    +      showCreateHiveTable(tableMetadata)
         }
     
         Seq(Row(stmt))
       }
     
    +  private def showCreateHiveTable(metadata: CatalogTable): String = {
    +    def reportUnsupportedError(): Unit = {
    +      throw new UnsupportedOperationException(
    +        s"Failed to execute SHOW CREATE TABLE against table ${metadata.identifier.quotedString}, " +
    +          "because it contains table structure(s) (e.g. skewed columns) that Spark SQL doesn't " +
    +          "support yet."
    +      )
    +    }
    +
    +    if (!metadata.fullyMapped) {
    +      reportUnsupportedError()
    +    }
    +
    +    val builder = StringBuilder.newBuilder
    +
    +    val tableTypeString = metadata.tableType match {
    +      case EXTERNAL => " EXTERNAL TABLE"
    +      case VIEW => " VIEW"
    +      case MANAGED => " TABLE"
    +      case INDEX => reportUnsupportedError()
    +    }
    +
    +    builder ++= s"CREATE$tableTypeString ${table.quotedString}"
    +
    +    if (metadata.tableType == VIEW) {
    +      if (metadata.schema.nonEmpty) {
    +        builder ++= metadata.schema.map(_.name).mkString("(", ", ", ")")
    +      }
    +      builder ++= metadata.viewOriginalText.mkString(" AS\n", "", "\n")
    +    } else {
    +      showHiveTableDataColumns(metadata, builder)
    +      showHiveTableNonDataColumns(metadata, builder)
    +      showHiveTableStorageInfo(metadata, builder)
    +      showHiveTableProperties(metadata, builder)
    +    }
    +
    +    builder.toString()
    +  }
    +
    +  private def showHiveTableDataColumns(metadata: CatalogTable, builder: StringBuilder): Unit = {
    +    val columns = metadata.schema.filterNot { column =>
    +      metadata.partitionColumnNames.contains(column.name)
    +    }.map(columnToDDLFragment)
    +
    +    if (columns.nonEmpty) {
    +      builder ++= columns.mkString("(", ", ", ")\n")
    +    }
    +  }
    +
    +  private def columnToDDLFragment(column: CatalogColumn): String = {
    +    val comment = column.comment.map(escapeSingleQuotedString).map(" COMMENT '" + _ + "'")
    +    s"${quoteIdentifier(column.name)} ${column.dataType}${comment.getOrElse("")}"
    +  }
    +
    +  private def showHiveTableNonDataColumns(metadata: CatalogTable, builder: StringBuilder): Unit = {
    +    if (metadata.partitionColumns.nonEmpty) {
    +      val partCols = metadata.partitionColumns.map(columnToDDLFragment)
    +      builder ++= partCols.mkString("PARTITIONED BY (", ", ", ")\n")
    +    }
    +
    +    if (metadata.bucketColumnNames.nonEmpty) {
    --- End diff --
    
    Yea. Let's throw an exception since we do not allow users to create bucketed Hive tables.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63669445
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
    @@ -626,40 +626,149 @@ case class ShowCreateTableCommand(table: TableIdentifier) extends RunnableComman
         val stmt = if (DDLUtils.isDatasourceTable(tableMetadata)) {
           showCreateDataSourceTable(tableMetadata)
         } else {
    -      throw new UnsupportedOperationException(
    -        "SHOW CREATE TABLE only supports Spark SQL data source tables.")
    +      showCreateHiveTable(tableMetadata)
         }
     
         Seq(Row(stmt))
       }
     
    +  private def showCreateHiveTable(metadata: CatalogTable): String = {
    +    def reportUnsupportedError(): Unit = {
    +      throw new UnsupportedOperationException(
    +        s"Failed to execute SHOW CREATE TABLE against table ${metadata.identifier.quotedString}, " +
    +          "because it contains table structure(s) (e.g. skewed columns) that Spark SQL doesn't " +
    +          "support yet."
    --- End diff --
    
    Do you mean we should mention the exact keywords, or just saying that the table is created by Hive?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63489049
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
    @@ -626,40 +626,142 @@ case class ShowCreateTableCommand(table: TableIdentifier) extends RunnableComman
         val stmt = if (DDLUtils.isDatasourceTable(tableMetadata)) {
           showCreateDataSourceTable(tableMetadata)
         } else {
    -      throw new UnsupportedOperationException(
    -        "SHOW CREATE TABLE only supports Spark SQL data source tables.")
    +      showCreateHiveTable(tableMetadata)
         }
     
         Seq(Row(stmt))
       }
     
    +  private def showCreateHiveTable(metadata: CatalogTable): String = {
    +    def reportUnsupportedError(): Unit = {
    +      throw new UnsupportedOperationException(
    +        s"Failed to execute SHOW CREATE TABLE against table ${metadata.identifier.quotedString}, " +
    +          "because it contains table structure(s) (e.g. skewed columns) that Spark SQL doesn't " +
    +          "support yet."
    +      )
    +    }
    +
    +    if (!metadata.fullyMapped) {
    +      reportUnsupportedError()
    +    }
    +
    +    val builder = StringBuilder.newBuilder
    +
    +    val tableTypeString = metadata.tableType match {
    +      case EXTERNAL => " EXTERNAL TABLE"
    +      case VIEW => " VIEW"
    +      case MANAGED => " TABLE"
    +      case INDEX => reportUnsupportedError()
    +    }
    +
    +    builder ++= s"CREATE$tableTypeString ${table.quotedString}"
    +
    +    if (metadata.tableType == VIEW) {
    +      if (metadata.schema.nonEmpty) {
    +        builder ++= metadata.schema.map(_.name).mkString("(", ", ", ")")
    +      }
    +      builder ++= metadata.viewOriginalText.mkString(" AS\n", "", "\n")
    +    } else {
    +      showHiveTableDataColumns(metadata, builder)
    +      showHiveTableNonDataColumns(metadata, builder)
    +      showHiveTableStorageInfo(metadata, builder)
    +      showHiveTableProperties(metadata, builder)
    +    }
    +
    +    builder.toString()
    +  }
    +
    +  private def showHiveTableDataColumns(metadata: CatalogTable, builder: StringBuilder): Unit = {
    +    val columns = metadata.schema.filterNot { column =>
    +      metadata.partitionColumnNames.contains(column.name)
    +    }.map(columnToDDLFragment)
    +
    +    if (columns.nonEmpty) {
    +      builder ++= columns.mkString("(", ", ", ")\n")
    +    }
    +  }
    +
    +  private def columnToDDLFragment(column: CatalogColumn): String = {
    +    val comment = column.comment.map(escapeSingleQuotedString).map(" COMMENT '" + _ + "'")
    +    s"${quoteIdentifier(column.name)} ${column.dataType}${comment.getOrElse("")}"
    +  }
    +
    +  private def showHiveTableNonDataColumns(metadata: CatalogTable, builder: StringBuilder): Unit = {
    +    if (metadata.partitionColumns.nonEmpty) {
    +      val partCols = metadata.partitionColumns.map(columnToDDLFragment)
    +      builder ++= partCols.mkString("PARTITIONED BY (", ", ", ")\n")
    +    }
    +
    +    if (metadata.bucketColumnNames.nonEmpty) {
    +      builder ++= metadata.bucketColumnNames.mkString("CLUSTERED BY (", ", ", ")\n")
    +
    +      if (metadata.sortColumnNames.nonEmpty) {
    +        builder ++= metadata.bucketColumnNames.mkString("SORTED BY (", ", ", ")\n")
    +      }
    +
    +      builder ++= s"INTO ${metadata.numBuckets} BUCKETS\n"
    +    }
    +  }
    +
    +  private def showHiveTableStorageInfo(metadata: CatalogTable, builder: StringBuilder): Unit = {
    +    val storage = metadata.storage
    +
    +    storage.serde.foreach { serde =>
    +      builder ++= s"ROW FORMAT SERDE '$serde'\n"
    +
    +      val serdeProps = metadata.storage.serdeProperties.map {
    +        case (key, value) =>
    +          s"'${escapeSingleQuotedString(key)}' = '${escapeSingleQuotedString(value)}'"
    +      }
    +
    +      builder ++= serdeProps.mkString("WITH SERDEPROPERTIES (", ",\n  ", "\n)\n")
    +    }
    +
    +    if (storage.inputFormat.isDefined || storage.outputFormat.isDefined) {
    +      builder ++= "STORED AS\n"
    +
    +      storage.inputFormat.foreach { format =>
    +        builder ++= s"  INPUTFORMAT '${escapeSingleQuotedString(format)}'\n"
    +      }
    +
    +      storage.outputFormat.foreach { format =>
    +        builder ++= s"  OUTPUTFORMAT '${escapeSingleQuotedString(format)}'\n"
    +      }
    +    }
    +
    +    if (metadata.tableType == EXTERNAL) {
    +      storage.locationUri.foreach { uri =>
    +        builder ++= s"LOCATION '$uri'\n"
    +      }
    +    }
    +  }
    +
    +  private def showHiveTableProperties(metadata: CatalogTable, builder: StringBuilder): Unit = {
    +    if (metadata.properties.nonEmpty) {
    +      val props = metadata.properties.map { case (key, value) =>
    +        s"'${escapeSingleQuotedString(key)}' = '${escapeSingleQuotedString(value)}'"
    +      }
    +
    +      builder ++= props.mkString("TBLPROPERTIES (", ",\n  ", ")\n")
    --- End diff --
    
    Thanks, filtered out this property.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63414957
  
    --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/ShowCreateTableSuite.scala ---
    @@ -116,24 +116,143 @@ class ShowCreateTableSuite extends QueryTest with SQLTestUtils with TestHiveSing
             .bucketBy(2, "c", "d")
             .saveAsTable("ddl_test5")
     
    -      checkCreateTable(TableIdentifier("ddl_test5", Some("default")))
    +      checkCreateTable("ddl_test5")
    +    }
    +  }
    +
    +  test("simple hive table") {
    +    withTable("t1") {
    +      sql(
    +        s"""CREATE TABLE t1 (
    +           |  c1 INT COMMENT 'bla',
    +           |  c2 STRING
    +           |)
    +           |TBLPROPERTIES (
    +           |  'prop1' = 'value1',
    +           |  'prop2' = 'value2'
    +           |)
    +         """.stripMargin
    +      )
    +
    +      checkCreateTable("t1")
    +    }
    +  }
    +
    +  test("simple external hive table") {
    +    withTempDir { dir =>
    +      withTable("t1") {
    +        sql(
    +          s"""CREATE TABLE t1 (
    +             |  c1 INT COMMENT 'bla',
    +             |  c2 STRING
    +             |)
    +             |LOCATION '$dir'
    +             |TBLPROPERTIES (
    +             |  'prop1' = 'value1',
    +             |  'prop2' = 'value2'
    +             |)
    +           """.stripMargin
    +        )
    +
    +        checkCreateTable("t1")
    +      }
    +    }
    +  }
    +
    +  test("partitioned hive table") {
    +    withTable("t1") {
    +      sql(
    +        s"""CREATE TABLE t1 (
    +           |  c1 INT COMMENT 'bla',
    +           |  c2 STRING
    +           |)
    +           |COMMENT 'bla'
    +           |PARTITIONED BY (
    +           |  p1 BIGINT COMMENT 'bla',
    +           |  p2 STRING
    +           |)
    +         """.stripMargin
    +      )
    +
    +      checkCreateTable("t1")
    +    }
    +  }
    +
    +  test("hive table with explicit storage info") {
    +    withTable("t1") {
    +      sql(
    +        s"""CREATE TABLE t1 (
    +           |  c1 INT COMMENT 'bla',
    +           |  c2 STRING
    +           |)
    +           |ROW FORMAT DELIMITED FIELDS TERMINATED BY ','
    +           |COLLECTION ITEMS TERMINATED BY '@'
    +           |MAP KEYS TERMINATED BY '#'
    +           |NULL DEFINED AS 'NaN'
    +           |STORED AS PARQUET
    +         """.stripMargin
    +      )
    +
    +      checkCreateTable("t1")
    +    }
    +  }
    +
    +  test("hive table with serde info") {
    +    withTable("t1") {
    +      sql(
    +        s"""CREATE TABLE t1 (
    +           |  c1 INT COMMENT 'bla',
    +           |  c2 STRING
    +           |)
    +           |ROW FORMAT SERDE 'org.apache.hadoop.hive.ql.io.parquet.serde.ParquetHiveSerDe'
    +           |WITH SERDEPROPERTIES (
    +           |  'mapkey.delim' = ',',
    +           |  'field.delim' = ','
    +           |)
    +           |STORED AS
    +           |  INPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat'
    +           |  OUTPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetOutputFormat'
    +         """.stripMargin
    +      )
    +
    +      checkCreateTable("t1")
    +    }
    +  }
    +
    +  test("hive view") {
    +    withView("v1") {
    +      sql("CREATE VIEW v1 AS SELECT 1 AS a")
    +      checkCreateView("v1")
    +    }
    +  }
    +
    +  test("hive view with output columns") {
    +    withView("v1") {
    +      sql("CREATE VIEW v1 (b) AS SELECT 1 AS a")
    +      checkCreateView("v1")
    --- End diff --
    
    let's also have a test for `STORED AS`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by xwu0226 <gi...@git.apache.org>.
Github user xwu0226 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63065098
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
    @@ -626,40 +626,142 @@ case class ShowCreateTableCommand(table: TableIdentifier) extends RunnableComman
         val stmt = if (DDLUtils.isDatasourceTable(tableMetadata)) {
           showCreateDataSourceTable(tableMetadata)
         } else {
    -      throw new UnsupportedOperationException(
    -        "SHOW CREATE TABLE only supports Spark SQL data source tables.")
    +      showCreateHiveTable(tableMetadata)
         }
     
         Seq(Row(stmt))
       }
     
    +  private def showCreateHiveTable(metadata: CatalogTable): String = {
    +    def reportUnsupportedError(): Unit = {
    +      throw new UnsupportedOperationException(
    +        s"Failed to execute SHOW CREATE TABLE against table ${metadata.identifier.quotedString}, " +
    +          "because it contains table structure(s) (e.g. skewed columns) that Spark SQL doesn't " +
    +          "support yet."
    +      )
    +    }
    +
    +    if (!metadata.fullyMapped) {
    +      reportUnsupportedError()
    +    }
    +
    +    val builder = StringBuilder.newBuilder
    +
    +    val tableTypeString = metadata.tableType match {
    +      case EXTERNAL => " EXTERNAL TABLE"
    +      case VIEW => " VIEW"
    +      case MANAGED => " TABLE"
    +      case INDEX => reportUnsupportedError()
    +    }
    +
    +    builder ++= s"CREATE$tableTypeString ${table.quotedString}"
    +
    +    if (metadata.tableType == VIEW) {
    +      if (metadata.schema.nonEmpty) {
    +        builder ++= metadata.schema.map(_.name).mkString("(", ", ", ")")
    +      }
    +      builder ++= metadata.viewOriginalText.mkString(" AS\n", "", "\n")
    +    } else {
    +      showHiveTableDataColumns(metadata, builder)
    --- End diff --
    
    Should there be table comment clause generate here, after the data column list?  `COMMENT '.....'`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63411416
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
    @@ -626,40 +626,142 @@ case class ShowCreateTableCommand(table: TableIdentifier) extends RunnableComman
         val stmt = if (DDLUtils.isDatasourceTable(tableMetadata)) {
           showCreateDataSourceTable(tableMetadata)
         } else {
    -      throw new UnsupportedOperationException(
    -        "SHOW CREATE TABLE only supports Spark SQL data source tables.")
    +      showCreateHiveTable(tableMetadata)
         }
     
         Seq(Row(stmt))
       }
     
    +  private def showCreateHiveTable(metadata: CatalogTable): String = {
    +    def reportUnsupportedError(): Unit = {
    +      throw new UnsupportedOperationException(
    +        s"Failed to execute SHOW CREATE TABLE against table ${metadata.identifier.quotedString}, " +
    +          "because it contains table structure(s) (e.g. skewed columns) that Spark SQL doesn't " +
    +          "support yet."
    +      )
    +    }
    +
    +    if (!metadata.fullyMapped) {
    +      reportUnsupportedError()
    +    }
    +
    +    val builder = StringBuilder.newBuilder
    +
    +    val tableTypeString = metadata.tableType match {
    +      case EXTERNAL => " EXTERNAL TABLE"
    +      case VIEW => " VIEW"
    +      case MANAGED => " TABLE"
    +      case INDEX => reportUnsupportedError()
    +    }
    +
    +    builder ++= s"CREATE$tableTypeString ${table.quotedString}"
    +
    +    if (metadata.tableType == VIEW) {
    +      if (metadata.schema.nonEmpty) {
    +        builder ++= metadata.schema.map(_.name).mkString("(", ", ", ")")
    +      }
    +      builder ++= metadata.viewOriginalText.mkString(" AS\n", "", "\n")
    --- End diff --
    
    Looks like we need to use `viewText` instead of `viewOriginalText` to make those identifiers qualified.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:

    https://github.com/apache/spark/pull/13079


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:

    https://github.com/apache/spark/pull/13079#issuecomment-219878464
  
    LGTM. I am merging this to master and 2.0. Let's change the error message and exception class in a separate pr.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/13079#issuecomment-218823528
  
    **[Test build #58504 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58504/consoleFull)** for PR 13079 at commit [`2020d77`](https://github.com/apache/spark/commit/2020d7712e227a45307fd1564b20ebb8cdb56bcf).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63108155
  
    --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
    @@ -95,7 +101,8 @@ case class CatalogTable(
         properties: Map[String, String] = Map.empty,
         viewOriginalText: Option[String] = None,
         viewText: Option[String] = None,
    -    comment: Option[String] = None) {
    +    comment: Option[String] = None,
    +    fullyMapped: Boolean = true) {
    --- End diff --
    
    I think something like `containsMissingInfo` is more intuitive


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/13079#issuecomment-218849233
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/58504/
    Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63618404
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
    @@ -626,40 +626,142 @@ case class ShowCreateTableCommand(table: TableIdentifier) extends RunnableComman
         val stmt = if (DDLUtils.isDatasourceTable(tableMetadata)) {
           showCreateDataSourceTable(tableMetadata)
         } else {
    -      throw new UnsupportedOperationException(
    -        "SHOW CREATE TABLE only supports Spark SQL data source tables.")
    +      showCreateHiveTable(tableMetadata)
         }
     
         Seq(Row(stmt))
       }
     
    +  private def showCreateHiveTable(metadata: CatalogTable): String = {
    +    def reportUnsupportedError(): Unit = {
    +      throw new UnsupportedOperationException(
    --- End diff --
    
    Let's be consistent with other places and change this to AnalysisException. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/13079#issuecomment-219784648
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63489249
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
    @@ -626,40 +626,142 @@ case class ShowCreateTableCommand(table: TableIdentifier) extends RunnableComman
         val stmt = if (DDLUtils.isDatasourceTable(tableMetadata)) {
           showCreateDataSourceTable(tableMetadata)
         } else {
    -      throw new UnsupportedOperationException(
    -        "SHOW CREATE TABLE only supports Spark SQL data source tables.")
    +      showCreateHiveTable(tableMetadata)
         }
     
         Seq(Row(stmt))
       }
     
    +  private def showCreateHiveTable(metadata: CatalogTable): String = {
    +    def reportUnsupportedError(): Unit = {
    +      throw new UnsupportedOperationException(
    +        s"Failed to execute SHOW CREATE TABLE against table ${metadata.identifier.quotedString}, " +
    +          "because it contains table structure(s) (e.g. skewed columns) that Spark SQL doesn't " +
    +          "support yet."
    +      )
    +    }
    +
    +    if (!metadata.fullyMapped) {
    +      reportUnsupportedError()
    +    }
    +
    +    val builder = StringBuilder.newBuilder
    +
    +    val tableTypeString = metadata.tableType match {
    +      case EXTERNAL => " EXTERNAL TABLE"
    +      case VIEW => " VIEW"
    +      case MANAGED => " TABLE"
    +      case INDEX => reportUnsupportedError()
    +    }
    +
    +    builder ++= s"CREATE$tableTypeString ${table.quotedString}"
    +
    +    if (metadata.tableType == VIEW) {
    +      if (metadata.schema.nonEmpty) {
    +        builder ++= metadata.schema.map(_.name).mkString("(", ", ", ")")
    +      }
    +      builder ++= metadata.viewOriginalText.mkString(" AS\n", "", "\n")
    --- End diff --
    
    Good point, thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:

    https://github.com/apache/spark/pull/13079#issuecomment-218849230
  
    Merged build finished. Test PASSed.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63570204
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
    @@ -626,40 +626,149 @@ case class ShowCreateTableCommand(table: TableIdentifier) extends RunnableComman
         val stmt = if (DDLUtils.isDatasourceTable(tableMetadata)) {
           showCreateDataSourceTable(tableMetadata)
         } else {
    -      throw new UnsupportedOperationException(
    -        "SHOW CREATE TABLE only supports Spark SQL data source tables.")
    +      showCreateHiveTable(tableMetadata)
         }
     
         Seq(Row(stmt))
       }
     
    +  private def showCreateHiveTable(metadata: CatalogTable): String = {
    +    def reportUnsupportedError(): Unit = {
    +      throw new UnsupportedOperationException(
    +        s"Failed to execute SHOW CREATE TABLE against table ${metadata.identifier.quotedString}, " +
    +          "because it contains table structure(s) (e.g. skewed columns) that Spark SQL doesn't " +
    +          "support yet."
    --- End diff --
    
    I think we need to explicitly say that the table was created by Hive with keywords that we do not support.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63488949
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
    @@ -626,40 +626,142 @@ case class ShowCreateTableCommand(table: TableIdentifier) extends RunnableComman
         val stmt = if (DDLUtils.isDatasourceTable(tableMetadata)) {
           showCreateDataSourceTable(tableMetadata)
         } else {
    -      throw new UnsupportedOperationException(
    -        "SHOW CREATE TABLE only supports Spark SQL data source tables.")
    +      showCreateHiveTable(tableMetadata)
         }
     
         Seq(Row(stmt))
       }
     
    +  private def showCreateHiveTable(metadata: CatalogTable): String = {
    +    def reportUnsupportedError(): Unit = {
    +      throw new UnsupportedOperationException(
    --- End diff --
    
    But this isn't an analysis failure, is it?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:

    https://github.com/apache/spark/pull/13079#issuecomment-218848924
  
    **[Test build #58504 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/58504/consoleFull)** for PR 13079 at commit [`2020d77`](https://github.com/apache/spark/commit/2020d7712e227a45307fd1564b20ebb8cdb56bcf).
     * This patch passes all tests.
     * This patch merges cleanly.
     * This patch adds no public classes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63105981
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
    @@ -626,40 +626,142 @@ case class ShowCreateTableCommand(table: TableIdentifier) extends RunnableComman
         val stmt = if (DDLUtils.isDatasourceTable(tableMetadata)) {
           showCreateDataSourceTable(tableMetadata)
         } else {
    -      throw new UnsupportedOperationException(
    -        "SHOW CREATE TABLE only supports Spark SQL data source tables.")
    +      showCreateHiveTable(tableMetadata)
         }
     
         Seq(Row(stmt))
       }
     
    +  private def showCreateHiveTable(metadata: CatalogTable): String = {
    +    def reportUnsupportedError(): Unit = {
    +      throw new UnsupportedOperationException(
    +        s"Failed to execute SHOW CREATE TABLE against table ${metadata.identifier.quotedString}, " +
    +          "because it contains table structure(s) (e.g. skewed columns) that Spark SQL doesn't " +
    +          "support yet."
    +      )
    +    }
    +
    +    if (!metadata.fullyMapped) {
    +      reportUnsupportedError()
    +    }
    +
    +    val builder = StringBuilder.newBuilder
    +
    +    val tableTypeString = metadata.tableType match {
    +      case EXTERNAL => " EXTERNAL TABLE"
    +      case VIEW => " VIEW"
    +      case MANAGED => " TABLE"
    +      case INDEX => reportUnsupportedError()
    +    }
    +
    +    builder ++= s"CREATE$tableTypeString ${table.quotedString}"
    +
    +    if (metadata.tableType == VIEW) {
    +      if (metadata.schema.nonEmpty) {
    +        builder ++= metadata.schema.map(_.name).mkString("(", ", ", ")")
    +      }
    +      builder ++= metadata.viewOriginalText.mkString(" AS\n", "", "\n")
    +    } else {
    +      showHiveTableDataColumns(metadata, builder)
    +      showHiveTableNonDataColumns(metadata, builder)
    +      showHiveTableStorageInfo(metadata, builder)
    +      showHiveTableProperties(metadata, builder)
    +    }
    +
    +    builder.toString()
    +  }
    +
    +  private def showHiveTableDataColumns(metadata: CatalogTable, builder: StringBuilder): Unit = {
    +    val columns = metadata.schema.filterNot { column =>
    +      metadata.partitionColumnNames.contains(column.name)
    +    }.map(columnToDDLFragment)
    +
    +    if (columns.nonEmpty) {
    +      builder ++= columns.mkString("(", ", ", ")\n")
    +    }
    +  }
    +
    +  private def columnToDDLFragment(column: CatalogColumn): String = {
    +    val comment = column.comment.map(escapeSingleQuotedString).map(" COMMENT '" + _ + "'")
    +    s"${quoteIdentifier(column.name)} ${column.dataType}${comment.getOrElse("")}"
    +  }
    +
    +  private def showHiveTableNonDataColumns(metadata: CatalogTable, builder: StringBuilder): Unit = {
    +    if (metadata.partitionColumns.nonEmpty) {
    +      val partCols = metadata.partitionColumns.map(columnToDDLFragment)
    +      builder ++= partCols.mkString("PARTITIONED BY (", ", ", ")\n")
    +    }
    +
    +    if (metadata.bucketColumnNames.nonEmpty) {
    --- End diff --
    
    we don't support this I don't think. You can't actually create a table with bucketed columns.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63489023
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
    @@ -626,40 +626,142 @@ case class ShowCreateTableCommand(table: TableIdentifier) extends RunnableComman
         val stmt = if (DDLUtils.isDatasourceTable(tableMetadata)) {
           showCreateDataSourceTable(tableMetadata)
         } else {
    -      throw new UnsupportedOperationException(
    -        "SHOW CREATE TABLE only supports Spark SQL data source tables.")
    +      showCreateHiveTable(tableMetadata)
         }
     
         Seq(Row(stmt))
       }
     
    +  private def showCreateHiveTable(metadata: CatalogTable): String = {
    +    def reportUnsupportedError(): Unit = {
    +      throw new UnsupportedOperationException(
    +        s"Failed to execute SHOW CREATE TABLE against table ${metadata.identifier.quotedString}, " +
    +          "because it contains table structure(s) (e.g. skewed columns) that Spark SQL doesn't " +
    +          "support yet."
    +      )
    +    }
    +
    +    if (!metadata.fullyMapped) {
    +      reportUnsupportedError()
    +    }
    +
    +    val builder = StringBuilder.newBuilder
    +
    +    val tableTypeString = metadata.tableType match {
    +      case EXTERNAL => " EXTERNAL TABLE"
    +      case VIEW => " VIEW"
    +      case MANAGED => " TABLE"
    +      case INDEX => reportUnsupportedError()
    +    }
    +
    +    builder ++= s"CREATE$tableTypeString ${table.quotedString}"
    +
    +    if (metadata.tableType == VIEW) {
    +      if (metadata.schema.nonEmpty) {
    +        builder ++= metadata.schema.map(_.name).mkString("(", ", ", ")")
    +      }
    +      builder ++= metadata.viewOriginalText.mkString(" AS\n", "", "\n")
    +    } else {
    +      showHiveTableDataColumns(metadata, builder)
    --- End diff --
    
    Thanks.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63105572
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
    @@ -626,40 +626,142 @@ case class ShowCreateTableCommand(table: TableIdentifier) extends RunnableComman
         val stmt = if (DDLUtils.isDatasourceTable(tableMetadata)) {
           showCreateDataSourceTable(tableMetadata)
         } else {
    -      throw new UnsupportedOperationException(
    -        "SHOW CREATE TABLE only supports Spark SQL data source tables.")
    +      showCreateHiveTable(tableMetadata)
         }
     
         Seq(Row(stmt))
       }
     
    +  private def showCreateHiveTable(metadata: CatalogTable): String = {
    +    def reportUnsupportedError(): Unit = {
    +      throw new UnsupportedOperationException(
    +        s"Failed to execute SHOW CREATE TABLE against table ${metadata.identifier.quotedString}, " +
    +          "because it contains table structure(s) (e.g. skewed columns) that Spark SQL doesn't " +
    +          "support yet."
    +      )
    +    }
    +
    +    if (!metadata.fullyMapped) {
    +      reportUnsupportedError()
    +    }
    +
    +    val builder = StringBuilder.newBuilder
    +
    +    val tableTypeString = metadata.tableType match {
    +      case EXTERNAL => " EXTERNAL TABLE"
    +      case VIEW => " VIEW"
    +      case MANAGED => " TABLE"
    +      case INDEX => reportUnsupportedError()
    +    }
    +
    +    builder ++= s"CREATE$tableTypeString ${table.quotedString}"
    +
    +    if (metadata.tableType == VIEW) {
    +      if (metadata.schema.nonEmpty) {
    +        builder ++= metadata.schema.map(_.name).mkString("(", ", ", ")")
    +      }
    +      builder ++= metadata.viewOriginalText.mkString(" AS\n", "", "\n")
    +    } else {
    +      showHiveTableDataColumns(metadata, builder)
    +      showHiveTableNonDataColumns(metadata, builder)
    +      showHiveTableStorageInfo(metadata, builder)
    +      showHiveTableProperties(metadata, builder)
    +    }
    +
    +    builder.toString()
    +  }
    +
    +  private def showHiveTableDataColumns(metadata: CatalogTable, builder: StringBuilder): Unit = {
    +    val columns = metadata.schema.filterNot { column =>
    +      metadata.partitionColumnNames.contains(column.name)
    +    }.map(columnToDDLFragment)
    +
    +    if (columns.nonEmpty) {
    +      builder ++= columns.mkString("(", ", ", ")\n")
    +    }
    +  }
    +
    +  private def columnToDDLFragment(column: CatalogColumn): String = {
    +    val comment = column.comment.map(escapeSingleQuotedString).map(" COMMENT '" + _ + "'")
    +    s"${quoteIdentifier(column.name)} ${column.dataType}${comment.getOrElse("")}"
    +  }
    +
    +  private def showHiveTableNonDataColumns(metadata: CatalogTable, builder: StringBuilder): Unit = {
    +    if (metadata.partitionColumns.nonEmpty) {
    +      val partCols = metadata.partitionColumns.map(columnToDDLFragment)
    +      builder ++= partCols.mkString("PARTITIONED BY (", ", ", ")\n")
    +    }
    +
    +    if (metadata.bucketColumnNames.nonEmpty) {
    +      builder ++= metadata.bucketColumnNames.mkString("CLUSTERED BY (", ", ", ")\n")
    --- End diff --
    
    we don't support `CREATE TABLE ... CLUSTER BY`


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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


[GitHub] spark pull request: [SPARK-14346][SQL] Native SHOW CREATE TABLE fo...

Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:

    https://github.com/apache/spark/pull/13079#discussion_r63543604
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
    @@ -626,40 +626,142 @@ case class ShowCreateTableCommand(table: TableIdentifier) extends RunnableComman
         val stmt = if (DDLUtils.isDatasourceTable(tableMetadata)) {
           showCreateDataSourceTable(tableMetadata)
         } else {
    -      throw new UnsupportedOperationException(
    -        "SHOW CREATE TABLE only supports Spark SQL data source tables.")
    +      showCreateHiveTable(tableMetadata)
         }
     
         Seq(Row(stmt))
       }
     
    +  private def showCreateHiveTable(metadata: CatalogTable): String = {
    +    def reportUnsupportedError(): Unit = {
    +      throw new UnsupportedOperationException(
    --- End diff --
    
    Test case added.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---

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