[SPARK-19115] [SQL] Supporting Create Table Like Location #16868

ouyangxiaochen · 2017-02-09T08:37:29Z

What changes were proposed in this pull request?

Support CREATE [EXTERNAL] TABLE LIKE LOCATION... syntax for Hive serde and datasource tables.
In this PR,we follow SparkSQL design rules :

supporting create table like view or physical table or temporary view with location.
creating a table with location,this table will be an external table other than managed table.

How was this patch tested?

Add new test cases and update existing test cases

ouyangxiaochen · 2017-02-09T08:39:33Z

I have created a new PR. Please review it, Thanks! @gatorsmile @cloud-fan

cloud-fan · 2017-02-09T08:51:24Z

OK to test

cloud-fan · 2017-02-09T08:55:40Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala

    val sourceViewName = "tab1"
    val targetTabName = "tab2"
+    var createdTableType = CatalogTableType.MANAGED


we can make it more explicitly:

val tableType = if (location.isDefiend) EXTERNAL else MANAGED

Fine, I'll update it.

gatorsmile · 2017-02-09T21:59:49Z

ok to test

gatorsmile · 2017-02-09T22:01:18Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala

@@ -682,8 +682,8 @@ class HiveDDLSuite
        ))
      }
    }
-  }


Revert this back?

gatorsmile · 2017-02-09T22:04:37Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala

+          sql(s"CREATE TABLE $targetTabName LIKE $sourceTabName")
+        } else {
+          sql(s"CREATE TABLE $targetTabName LIKE $sourceTabName" +
+            s" LOCATION '${location.getOrElse("")}'")


The above logics can be simplified to

val locationClause = if (location.nonEmpty) "LOCATION '${location.getOrElse("")}'" else "" sql(s"CREATE TABLE $targetTabName LIKE $sourceTabName $locationClause")

The same to the other test cases

ouyangxiaochen · 2017-02-11T01:47:13Z

I have run test cases successfully. Please run the test cases again.Thanks a lot! @SparkQA

cloud-fan · 2017-02-12T01:25:38Z

retest this please

SparkQA · 2017-02-12T04:00:29Z

Test build #72745 has finished for PR 16868 at commit 81feda4.

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

gatorsmile · 2017-02-12T07:18:28Z

sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala

@@ -70,12 +71,19 @@ case class CreateTableLikeCommand(
      sourceTableDesc.provider
    }

+    // If location is specified, we create an external table internally.


-> If the

gatorsmile · 2017-02-12T07:18:45Z

sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala

@@ -70,12 +71,19 @@ case class CreateTableLikeCommand(
      sourceTableDesc.provider
    }

+    // If location is specified, we create an external table internally.
+    // Else create managed table.


Otherwise, create a

gatorsmile · 2017-02-12T07:20:12Z

sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala

+      CatalogTableType.MANAGED
+    } else {
+      CatalogTableType.EXTERNAL
+    }


Shorten it to one line?

val tblType = if (location.isEmpty) CatalogTableType.MANAGED else CatalogTableType.EXTERNAL

gatorsmile · 2017-02-12T07:22:09Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala

@@ -833,54 +833,95 @@ class HiveDDLSuite
  }

  test("CREATE TABLE LIKE a temporary view") {
+    // CREATE TABLE LIKE a temporary view.
+    withCreateTableLikeTempView(None)


withCreateTableLikeTempView(None) -> withCreateTableLikeTempView(location = None)

gatorsmile · 2017-02-12T07:22:31Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala

+    withCreateTableLikeTempView(None)
+
+    // CREATE TABLE LIKE a temporary view location ...
+    withTempDir {tmpDir =>


{tmpDir -> { tmpDir

gatorsmile · 2017-02-12T07:23:10Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala

      }
    }
  }

  test("CREATE TABLE LIKE a data source table") {
+    // CREATE TABLE LIKE a data source table.
+    withCreateTableLikeDSTable(None)


The same here.

gatorsmile · 2017-02-12T07:23:25Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala

    }
  }

  test("CREATE TABLE LIKE an external data source table") {
+    // CREATE TABLE LIKE an external data source table.
+    withCreateTableLikeExtDSTable(None)


The same here

gatorsmile · 2017-02-12T07:24:20Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala

-             |LOCATION '$basePath'
-           """.stripMargin)
+             |LOCATION '$basePath1'
+             """.stripMargin)


revert it back

gatorsmile · 2017-02-12T07:24:28Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala

        for (ds <- Seq("2008-04-08", "2008-04-09"); hr <- Seq("11", "12")) {
          sql(
            s"""
               |INSERT OVERWRITE TABLE $sourceTabName
               |partition (ds='$ds',hr='$hr')
               |SELECT 1, 'a'
-             """.stripMargin)
+               """.stripMargin)


revert it back

gatorsmile · 2017-02-12T07:25:10Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala

    withTempDir { tmpDir =>
-      val basePath = tmpDir.toURI
+      val basePath1 = tmpDir.toURI


revert it back?

gatorsmile · 2017-02-12T07:25:30Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala

      }
    }
  }

  test("CREATE TABLE LIKE a view") {
+    // CREATE TABLE LIKE a view.
+    withCreateTableLikeView(None)


The same here.

gatorsmile · 2017-02-12T07:28:23Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala

+    // The created table should be a MANAGED table or EXTERNAL table with empty view text
+    // and original text.
+    assert(targetTable.tableType == tableType,
+      s"the created table must be a Hive ${tableType.name} table")


For a Hive table? It could be a data source table too.

Sorry for the late reply.Ok, i will follow your suggestion.

gatorsmile · 2017-02-12T07:28:52Z

Please update the PR description. This statement can be used for data source tables too.

tejasapatil · 2017-02-12T07:57:11Z

You have good coverage for DDL statements. How about a test case for reading from external table creating using the command this PR enables ?

tejasapatil · 2017-02-12T07:59:23Z

BTW: Hive can allow location of external table to be same as the source table. Can you please try that ? It should work. In the test case validations I see this check which will fail in such case:

assert(sourceTable.storage.locationUri != targetTable.storage.locationUri, ....

gatorsmile · 2017-02-13T01:53:08Z

Please add a test case based on what @tejasapatil suggested. Thanks!

ouyangxiaochen · 2017-02-13T02:02:31Z

I think @tejasapatil's suggestion is reasonable, because the location is specified by users, So the sourceTable.storage.locationUri and targetTable.storage.locationUri can be same or different, Whether we need to be exactly the same as Hive?

ouyangxiaochen · 2017-02-13T02:39:33Z

In @tejasapatil's comment, Whether we need to be exactly the same as Hive? @gatorsmile

tejasapatil · 2017-02-13T02:49:39Z

There are two main uses of EXTERNAL tables I am aware of:

Ingest data from non-hive locations into Hive tables. This can be covered by adding test case for reading from external table creating using the command this PR enables
Create a logical "pointer" to an existing hive table / partition (without creating multiple copies of the underlying data). Testing if the destination table can have the same location as of the source table will cover this.

I don't think Spark's interpretation of external tables is different from Hive's so its OK to support both.

BTW: If you are supporting 1st use case, one can mimic to get behavior of 2nd use case by creating external table with a fake location and later issuing a ALTER TABLE SET LOCATION command to make it point to an existing table's location. There is really no mechanism to guard against having EXTERNAL tables to not point to an existing table / partition in Spark. So, both use cases were already possible in Spark

ouyangxiaochen · 2017-02-13T03:54:42Z

Do you mean that we don't need to do check whether the targetTable.storage.locationUri is the same with sourceTable.storage.locationUri or not ? @tejasapatil

tejasapatil · 2017-02-13T05:40:40Z

we don't need to do check whether the targetTable.storage.locationUri is the same with sourceTable.storage.locationUri

We should not do that check for external tables. But continue doing that for other types of tables.

ouyangxiaochen · 2017-02-13T07:48:37Z

Very thoughtful consideration. Thanks for your explanation and suggestion! @tejasapatil what do you think? @gatorsmile @cloud-fan

2. ignore the validation of target table's location when we creating an external table.

…DSTable(location = None)

gatorsmile · 2017-02-13T21:16:13Z

ok to test

gatorsmile · 2017-02-13T21:17:19Z

Yes, the location can be the same or different from the original table.

LGTM pending test

SparkQA · 2017-02-14T00:08:12Z

Test build #72830 has finished for PR 16868 at commit 71f4f1c.

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

gatorsmile · 2017-02-14T03:42:02Z

Thanks! Merging to master.

What changes were proposed in this pull request? Support CREATE [EXTERNAL] TABLE LIKE LOCATION... syntax for Hive serde and datasource tables. In this PR,we follow SparkSQL design rules : supporting create table like view or physical table or temporary view with location. creating a table with location,this table will be an external table other than managed table. How was this patch tested? Add new test cases and update existing test cases Author: ouyangxiaochen <ou.yangxiaochen@zte.com.cn> Closes apache#16868 from ouyangxiaochen/spark19115.

resolve the conflicts

2221809

cloud-fan reviewed Feb 9, 2017

View reviewed changes

make code more explicitly

e904993

gatorsmile reviewed Feb 9, 2017

View reviewed changes

Simplified code logic

81feda4

gatorsmile reviewed Feb 12, 2017

View reviewed changes

ouyangxiaochen added 2 commits February 13, 2017 16:18

1. code optimization based on reviewer's recommendations

fa1784e

2. ignore the validation of target table's location when we creating an external table.

modifiy withCreateTableLikeExtDSTable(None) to withCreateTableLikeExt…

71f4f1c

…DSTable(location = None)

ouyangxiaochen changed the title ~~[SPARK-19115] [SQL] Supporting Create External Table Like Location~~ [SPARK-19115] [SQL] Supporting Create Table Like Location Feb 13, 2017

asfgit closed this in 6e45b54 Feb 14, 2017

ouyangxiaochen deleted the spark19115 branch February 14, 2017 06:27

tejasapatil mentioned this pull request Feb 15, 2017

[SPARK-19583][SQL]CTAS for data source table with a created location should succeed #16938

Closed

dongjoon-hyun mentioned this pull request May 10, 2023

[SPARK-43405][SQL] Remove useless getRowFormatSQL, inputRowFormatSQL, outputRowFormatSQL method #41063

Closed

@@ @@ -682,8 +682,8 @@ class HiveDDLSuite @@
                       ))
                     }
                   }
-                }

[SPARK-19115] [SQL] Supporting Create Table Like Location #16868

[SPARK-19115] [SQL] Supporting Create Table Like Location #16868

Conversation

ouyangxiaochen commented Feb 9, 2017 • edited Loading

ouyangxiaochen commented Feb 9, 2017

cloud-fan commented Feb 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gatorsmile commented Feb 9, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ouyangxiaochen commented Feb 11, 2017

cloud-fan commented Feb 12, 2017

SparkQA commented Feb 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gatorsmile commented Feb 12, 2017

tejasapatil commented Feb 12, 2017

tejasapatil commented Feb 12, 2017

gatorsmile commented Feb 13, 2017

ouyangxiaochen commented Feb 13, 2017

ouyangxiaochen commented Feb 13, 2017

tejasapatil commented Feb 13, 2017 • edited Loading

ouyangxiaochen commented Feb 13, 2017

tejasapatil commented Feb 13, 2017

ouyangxiaochen commented Feb 13, 2017

gatorsmile commented Feb 13, 2017

gatorsmile commented Feb 13, 2017

SparkQA commented Feb 14, 2017

gatorsmile commented Feb 14, 2017

ouyangxiaochen commented Feb 9, 2017 •

edited

Loading

tejasapatil commented Feb 13, 2017 •

edited

Loading