You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by clockfly <gi...@git.apache.org> on 2016/06/01 20:32:56 UTC

[GitHub] spark pull request #13451: [SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE...

GitHub user clockfly opened a pull request:

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

    [SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE USING AS SELECT

    ## What changes were proposed in this pull request?
    
    This PR bans syntax like `CREATE TEMPORARY TABLE USING AS SELECT`
    
    `CREATE TEMPORARY TABLE ... USING ... AS ...` is not properly implemented, the temporary data is not cleaned up when the session exits. Before a full fix, we probably should ban this syntax.
    
    This only impact syntax like `CREATE TEMPORARY TABLE ... USING ... AS ...`. 
    Other syntax like `CREATE TEMPORARY TABLE .. USING ...` and `CREATE TABLE ... USING ...` are not impacted.
    
    ## How was this patch tested?
    
    Unit test (To be added)
    


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

    $ git pull https://github.com/clockfly/spark ban_create_temp_table_using_as

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

    https://github.com/apache/spark/pull/13451.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 #13451
    
----
commit e48caf054482f883d7cce4b51c306d7163361456
Author: Sean Zhong <se...@databricks.com>
Date:   2016-06-01T20:24:20Z

    Ban CREATE TEMP TABLE USING AS SELECT for now

----


---
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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:

    https://github.com/apache/spark/pull/13451
  
    LGTM. 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 #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USI...

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

    https://github.com/apache/spark/pull/13451#discussion_r65489737
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/sources/CreateTableAsSelectSuite.scala ---
    @@ -40,172 +43,175 @@ class CreateTableAsSelectSuite extends DataSourceTest with SharedSQLContext with
       override def afterAll(): Unit = {
         try {
           spark.catalog.dropTempView("jt")
    +      if (path.exists()) {
    +        Utils.deleteRecursively(path)
    +      }
         } finally {
           super.afterAll()
         }
       }
     
    -  after {
    -    Utils.deleteRecursively(path)
    +  before {
    +    if (path.exists()) {
    +      Utils.deleteRecursively(path)
    +    }
       }
     
    -  test("CREATE TEMPORARY TABLE AS SELECT") {
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a, b FROM jt
    -      """.stripMargin)
    -
    -    checkAnswer(
    -      sql("SELECT a, b FROM jsonTable"),
    -      sql("SELECT a, b FROM jt").collect())
    -
    -    spark.catalog.dropTempView("jsonTable")
    +  test("CREATE TABLE USING AS SELECT") {
    +    withTable("jsonTable") {
    +      sql(
    +        s"""
    +           |CREATE TABLE jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT a, b FROM jt
    +         """.stripMargin)
    +
    +      checkAnswer(
    +        sql("SELECT a, b FROM jsonTable"),
    +        sql("SELECT a, b FROM jt").collect())
    +    }
       }
     
    -  test("CREATE TEMPORARY TABLE AS SELECT based on the file without write permission") {
    +  test("CREATE TABLE USING AS SELECT based on the file without write permission") {
         val childPath = new File(path.toString, "child")
         path.mkdir()
    -    childPath.createNewFile()
         path.setWritable(false)
     
    -    val e = intercept[IOException] {
    +    val e = intercept[Exception] {
           sql(
             s"""
    -           |CREATE TEMPORARY TABLE jsonTable
    +           |CREATE TABLE jsonTable
                |USING json
                |OPTIONS (
    -           |  path '${path.toString}'
    +           |  path '${childPath.toString}'
                |) AS
                |SELECT a, b FROM jt
    -        """.stripMargin)
    +         """.stripMargin)
           sql("SELECT a, b FROM jsonTable").collect()
         }
    -    assert(e.getMessage().contains("Unable to clear output directory"))
     
    +    assert(e.getMessage().contains("Job aborted"))
         path.setWritable(true)
       }
     
       test("create a table, drop it and create another one with the same name") {
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a, b FROM jt
    -      """.stripMargin)
    -
    -    checkAnswer(
    -      sql("SELECT a, b FROM jsonTable"),
    -      sql("SELECT a, b FROM jt").collect())
    -
    -    val message = intercept[ParseException]{
    +    withTable("jsonTable") {
           sql(
             s"""
    -        |CREATE TEMPORARY TABLE IF NOT EXISTS jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a * 4 FROM jt
    -      """.stripMargin)
    -    }.getMessage
    -    assert(message.toLowerCase.contains("operation not allowed"))
    -
    -    // Overwrite the temporary table.
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a * 4 FROM jt
    -      """.stripMargin)
    -    checkAnswer(
    -      sql("SELECT * FROM jsonTable"),
    -      sql("SELECT a * 4 FROM jt").collect())
    -
    -    spark.catalog.dropTempView("jsonTable")
    -    // Explicitly delete the data.
    -    if (path.exists()) Utils.deleteRecursively(path)
    -
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT b FROM jt
    -      """.stripMargin)
    -
    -    checkAnswer(
    -      sql("SELECT * FROM jsonTable"),
    -      sql("SELECT b FROM jt").collect())
    -
    -    spark.catalog.dropTempView("jsonTable")
    -  }
    +           |CREATE TABLE jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT a, b FROM jt
    +         """.stripMargin)
    +
    +      checkAnswer(
    +        sql("SELECT a, b FROM jsonTable"),
    +        sql("SELECT a, b FROM jt").collect())
    +
    +      // Creates a table of the same name with flag "if not exists", nothing happens
    +      sql(
    +        s"""
    +           |CREATE TABLE IF NOT EXISTS jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT a * 4 FROM jt
    +         """.stripMargin)
    +      checkAnswer(
    +        sql("SELECT * FROM jsonTable"),
    +        sql("SELECT a, b FROM jt").collect())
    --- End diff --
    
    Nit: The same here. You do not need to call `collect`


---
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 #13451: [SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE...

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

    https://github.com/apache/spark/pull/13451#discussion_r65483261
  
    --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/MetastoreDataSourcesSuite.scala ---
    @@ -1104,4 +1104,22 @@ class MetastoreDataSourcesSuite extends QueryTest with SQLTestUtils with TestHiv
           }
         }
       }
    +
    +  test("CTAS: disallow create temporary table ... using ... as ...") {
    --- End diff --
    
    @gatorsmile  Thanks. The test should be included in CreateTableAsSelectSuite


---
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 #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USI...

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

    https://github.com/apache/spark/pull/13451#discussion_r65495897
  
    --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala ---
    @@ -1399,52 +1399,6 @@ class SQLQuerySuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
         }
       }
     
    -  test(
    --- End diff --
    
    Both are ok for me.


---
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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

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

    https://github.com/apache/spark/pull/13451
  
    @andrewor14 Could you please help sign off this one? 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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

Posted by clockfly <gi...@git.apache.org>.
Github user clockfly commented on the issue:

    https://github.com/apache/spark/pull/13451
  
    cc @cloud-fan @liancheng @andrewor14 


---
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 issue #13451: [SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE USING ...

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

    https://github.com/apache/spark/pull/13451
  
    **[Test build #59770 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59770/consoleFull)** for PR 13451 at commit [`fa1db6a`](https://github.com/apache/spark/commit/fa1db6a88423946b5c4986189f6d6c347cb34e2f).
     * This patch **fails Spark unit 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 #13451: [SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE...

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

    https://github.com/apache/spark/pull/13451#discussion_r65483550
  
    --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala ---
    @@ -1399,52 +1399,6 @@ class SQLQuerySuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
         }
       }
     
    -  test(
    --- End diff --
    
    @viirya 
    
    In the future, it should not be difficult to look through the history to find the code.
    
    Besides, the future implementation and expectation of `create temporary table using .. as query` probably will be different than it is now. So, maybe removing it for now creates less confusion?



---
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 #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USI...

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

    https://github.com/apache/spark/pull/13451#discussion_r65489841
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/sources/CreateTableAsSelectSuite.scala ---
    @@ -40,172 +43,175 @@ class CreateTableAsSelectSuite extends DataSourceTest with SharedSQLContext with
       override def afterAll(): Unit = {
         try {
           spark.catalog.dropTempView("jt")
    +      if (path.exists()) {
    +        Utils.deleteRecursively(path)
    +      }
         } finally {
           super.afterAll()
         }
       }
     
    -  after {
    -    Utils.deleteRecursively(path)
    +  before {
    +    if (path.exists()) {
    +      Utils.deleteRecursively(path)
    +    }
       }
     
    -  test("CREATE TEMPORARY TABLE AS SELECT") {
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a, b FROM jt
    -      """.stripMargin)
    -
    -    checkAnswer(
    -      sql("SELECT a, b FROM jsonTable"),
    -      sql("SELECT a, b FROM jt").collect())
    -
    -    spark.catalog.dropTempView("jsonTable")
    +  test("CREATE TABLE USING AS SELECT") {
    +    withTable("jsonTable") {
    +      sql(
    +        s"""
    +           |CREATE TABLE jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT a, b FROM jt
    +         """.stripMargin)
    +
    +      checkAnswer(
    +        sql("SELECT a, b FROM jsonTable"),
    +        sql("SELECT a, b FROM jt").collect())
    +    }
       }
     
    -  test("CREATE TEMPORARY TABLE AS SELECT based on the file without write permission") {
    +  test("CREATE TABLE USING AS SELECT based on the file without write permission") {
         val childPath = new File(path.toString, "child")
         path.mkdir()
    -    childPath.createNewFile()
         path.setWritable(false)
     
    -    val e = intercept[IOException] {
    +    val e = intercept[Exception] {
           sql(
             s"""
    -           |CREATE TEMPORARY TABLE jsonTable
    +           |CREATE TABLE jsonTable
                |USING json
                |OPTIONS (
    -           |  path '${path.toString}'
    +           |  path '${childPath.toString}'
                |) AS
                |SELECT a, b FROM jt
    -        """.stripMargin)
    +         """.stripMargin)
           sql("SELECT a, b FROM jsonTable").collect()
         }
    -    assert(e.getMessage().contains("Unable to clear output directory"))
     
    +    assert(e.getMessage().contains("Job aborted"))
         path.setWritable(true)
       }
     
       test("create a table, drop it and create another one with the same name") {
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a, b FROM jt
    -      """.stripMargin)
    -
    -    checkAnswer(
    -      sql("SELECT a, b FROM jsonTable"),
    -      sql("SELECT a, b FROM jt").collect())
    -
    -    val message = intercept[ParseException]{
    +    withTable("jsonTable") {
           sql(
             s"""
    -        |CREATE TEMPORARY TABLE IF NOT EXISTS jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a * 4 FROM jt
    -      """.stripMargin)
    -    }.getMessage
    -    assert(message.toLowerCase.contains("operation not allowed"))
    -
    -    // Overwrite the temporary table.
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a * 4 FROM jt
    -      """.stripMargin)
    -    checkAnswer(
    -      sql("SELECT * FROM jsonTable"),
    -      sql("SELECT a * 4 FROM jt").collect())
    -
    -    spark.catalog.dropTempView("jsonTable")
    -    // Explicitly delete the data.
    -    if (path.exists()) Utils.deleteRecursively(path)
    -
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT b FROM jt
    -      """.stripMargin)
    -
    -    checkAnswer(
    -      sql("SELECT * FROM jsonTable"),
    -      sql("SELECT b FROM jt").collect())
    -
    -    spark.catalog.dropTempView("jsonTable")
    -  }
    +           |CREATE TABLE jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT a, b FROM jt
    +         """.stripMargin)
    +
    +      checkAnswer(
    +        sql("SELECT a, b FROM jsonTable"),
    +        sql("SELECT a, b FROM jt").collect())
    +
    +      // Creates a table of the same name with flag "if not exists", nothing happens
    +      sql(
    +        s"""
    +           |CREATE TABLE IF NOT EXISTS jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT a * 4 FROM jt
    +         """.stripMargin)
    +      checkAnswer(
    +        sql("SELECT * FROM jsonTable"),
    +        sql("SELECT a, b FROM jt").collect())
    +
    +      // Explicitly drops the table and deletes the underlying data.
    +      sql("DROP TABLE jsonTable")
    +      if (path.exists()) Utils.deleteRecursively(path)
     
    -  test("CREATE TEMPORARY TABLE AS SELECT with IF NOT EXISTS is not allowed") {
    -    val message = intercept[ParseException]{
    +      // Creates a table of the same name again, this time we succeed.
           sql(
             s"""
    -        |CREATE TEMPORARY TABLE IF NOT EXISTS jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT b FROM jt
    -      """.stripMargin)
    -    }.getMessage
    -    assert(message.toLowerCase.contains("operation not allowed"))
    +           |CREATE TABLE jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT b FROM jt
    +         """.stripMargin)
    +
    +      checkAnswer(
    +        sql("SELECT * FROM jsonTable"),
    +        sql("SELECT b FROM jt").collect())
    +    }
    +  }
    +
    +  test("disallows CREATE TEMPORARY TABLE ... USING ... AS query") {
    +    withTable("t") {
    +      val error = intercept[ParseException] {
    +        sql(
    +          s"""
    +             |CREATE TEMPORARY TABLE t USING PARQUET
    +             |OPTIONS (PATH '${path.toString}')
    +             |PARTITIONED BY (a)
    +             |AS SELECT 1 AS a, 2 AS b
    +           """.stripMargin
    +        )
    +      }.getMessage
    +      assert(error.contains("Operation not allowed") &&
    +        error.contains("CREATE TEMPORARY TABLE ... USING ... AS query"))
    +    }
       }
     
    -  test("a CTAS statement with column definitions is not allowed") {
    -    intercept[AnalysisException]{
    +  test("disallows CREATE EXTERNAL TABLE ... USING ... AS query") {
    +    withTable("t") {
    +      val error = intercept[ParseException] {
    +        sql(
    +          s"""
    +             |CREATE EXTERNAL TABLE t USING PARQUET
    +             |OPTIONS (PATH '${path.toString}')
    +             |AS SELECT 1 AS a, 2 AS b
    +           """.stripMargin
    +        )
    +      }.getMessage
    +
    +      assert(error.contains("Operation not allowed") &&
    +        error.contains("CREATE EXTERNAL TABLE ... USING"))
    +    }
    +  }
    +
    +  test("create table using as select - with partitioned by") {
    +    val catalog = spark.sessionState.catalog
    +    withTable("t") {
           sql(
             s"""
    -        |CREATE TEMPORARY TABLE jsonTable (a int, b string)
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a, b FROM jt
    -      """.stripMargin)
    +           |CREATE TABLE t USING PARQUET
    +           |OPTIONS (PATH '${path.toString}')
    +           |PARTITIONED BY (a)
    +           |AS SELECT 1 AS a, 2 AS b
    +           """.stripMargin
    --- End diff --
    
    Nit: The indent issue


---
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 #13451: [SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE...

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

    https://github.com/apache/spark/pull/13451#discussion_r65477320
  
    --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLQuerySuite.scala ---
    @@ -1399,52 +1399,6 @@ class SQLQuerySuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
         }
       }
     
    -  test(
    --- End diff --
    
    If we plan to bring this syntax back after fixing the problem, should we let it ignore and add TODO instead of completely removing these tests?


---
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 #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USI...

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

    https://github.com/apache/spark/pull/13451#discussion_r65489722
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/sources/CreateTableAsSelectSuite.scala ---
    @@ -40,172 +43,175 @@ class CreateTableAsSelectSuite extends DataSourceTest with SharedSQLContext with
       override def afterAll(): Unit = {
         try {
           spark.catalog.dropTempView("jt")
    +      if (path.exists()) {
    +        Utils.deleteRecursively(path)
    +      }
         } finally {
           super.afterAll()
         }
       }
     
    -  after {
    -    Utils.deleteRecursively(path)
    +  before {
    +    if (path.exists()) {
    +      Utils.deleteRecursively(path)
    +    }
       }
     
    -  test("CREATE TEMPORARY TABLE AS SELECT") {
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a, b FROM jt
    -      """.stripMargin)
    -
    -    checkAnswer(
    -      sql("SELECT a, b FROM jsonTable"),
    -      sql("SELECT a, b FROM jt").collect())
    -
    -    spark.catalog.dropTempView("jsonTable")
    +  test("CREATE TABLE USING AS SELECT") {
    +    withTable("jsonTable") {
    +      sql(
    +        s"""
    +           |CREATE TABLE jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT a, b FROM jt
    +         """.stripMargin)
    +
    +      checkAnswer(
    +        sql("SELECT a, b FROM jsonTable"),
    +        sql("SELECT a, b FROM jt").collect())
    +    }
       }
     
    -  test("CREATE TEMPORARY TABLE AS SELECT based on the file without write permission") {
    +  test("CREATE TABLE USING AS SELECT based on the file without write permission") {
         val childPath = new File(path.toString, "child")
         path.mkdir()
    -    childPath.createNewFile()
         path.setWritable(false)
     
    -    val e = intercept[IOException] {
    +    val e = intercept[Exception] {
           sql(
             s"""
    -           |CREATE TEMPORARY TABLE jsonTable
    +           |CREATE TABLE jsonTable
                |USING json
                |OPTIONS (
    -           |  path '${path.toString}'
    +           |  path '${childPath.toString}'
                |) AS
                |SELECT a, b FROM jt
    -        """.stripMargin)
    +         """.stripMargin)
           sql("SELECT a, b FROM jsonTable").collect()
         }
    -    assert(e.getMessage().contains("Unable to clear output directory"))
     
    +    assert(e.getMessage().contains("Job aborted"))
         path.setWritable(true)
       }
     
       test("create a table, drop it and create another one with the same name") {
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a, b FROM jt
    -      """.stripMargin)
    -
    -    checkAnswer(
    -      sql("SELECT a, b FROM jsonTable"),
    -      sql("SELECT a, b FROM jt").collect())
    -
    -    val message = intercept[ParseException]{
    +    withTable("jsonTable") {
           sql(
             s"""
    -        |CREATE TEMPORARY TABLE IF NOT EXISTS jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a * 4 FROM jt
    -      """.stripMargin)
    -    }.getMessage
    -    assert(message.toLowerCase.contains("operation not allowed"))
    -
    -    // Overwrite the temporary table.
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a * 4 FROM jt
    -      """.stripMargin)
    -    checkAnswer(
    -      sql("SELECT * FROM jsonTable"),
    -      sql("SELECT a * 4 FROM jt").collect())
    -
    -    spark.catalog.dropTempView("jsonTable")
    -    // Explicitly delete the data.
    -    if (path.exists()) Utils.deleteRecursively(path)
    -
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT b FROM jt
    -      """.stripMargin)
    -
    -    checkAnswer(
    -      sql("SELECT * FROM jsonTable"),
    -      sql("SELECT b FROM jt").collect())
    -
    -    spark.catalog.dropTempView("jsonTable")
    -  }
    +           |CREATE TABLE jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT a, b FROM jt
    +         """.stripMargin)
    +
    +      checkAnswer(
    +        sql("SELECT a, b FROM jsonTable"),
    +        sql("SELECT a, b FROM jt").collect())
    --- End diff --
    
    Nit: The same here. You do not need to call `collect`


---
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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:

    https://github.com/apache/spark/pull/13451
  
    LGTM except some minor issues which are already pointed out by others.


---
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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

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

    https://github.com/apache/spark/pull/13451
  
    **[Test build #59834 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59834/consoleFull)** for PR 13451 at commit [`aaca7f0`](https://github.com/apache/spark/commit/aaca7f01882de19c9dda47500f7d4f03712d3383).


---
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 #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USI...

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

    https://github.com/apache/spark/pull/13451#discussion_r65489271
  
    --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/MetastoreDataSourcesSuite.scala ---
    @@ -1104,4 +1104,22 @@ class MetastoreDataSourcesSuite extends QueryTest with SQLTestUtils with TestHiv
           }
         }
       }
    +
    +  test("CTAS: disallow create temporary table ... using ... as ...") {
    --- End diff --
    
    Normally, parser-specific test cases are put in the same suite. I am fine if you put it to the CTAS-specific test suite, especially when we will support it in the near future.



---
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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

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

    https://github.com/apache/spark/pull/13451
  
    **[Test build #59822 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59822/consoleFull)** for PR 13451 at commit [`b7bb43d`](https://github.com/apache/spark/commit/b7bb43dadd71ea6c24f50eb24234a778c68a21df).
     * 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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

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

    https://github.com/apache/spark/pull/13451
  
    **[Test build #59818 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59818/consoleFull)** for PR 13451 at commit [`d4f5ab6`](https://github.com/apache/spark/commit/d4f5ab6a3a68cee466181af3a0ca06427c15c25f).
     * This patch passes all tests.
     * This patch **does not merge 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 #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USI...

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

    https://github.com/apache/spark/pull/13451#discussion_r65489815
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/sources/CreateTableAsSelectSuite.scala ---
    @@ -40,172 +43,175 @@ class CreateTableAsSelectSuite extends DataSourceTest with SharedSQLContext with
       override def afterAll(): Unit = {
         try {
           spark.catalog.dropTempView("jt")
    +      if (path.exists()) {
    +        Utils.deleteRecursively(path)
    +      }
         } finally {
           super.afterAll()
         }
       }
     
    -  after {
    -    Utils.deleteRecursively(path)
    +  before {
    +    if (path.exists()) {
    +      Utils.deleteRecursively(path)
    +    }
       }
     
    -  test("CREATE TEMPORARY TABLE AS SELECT") {
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a, b FROM jt
    -      """.stripMargin)
    -
    -    checkAnswer(
    -      sql("SELECT a, b FROM jsonTable"),
    -      sql("SELECT a, b FROM jt").collect())
    -
    -    spark.catalog.dropTempView("jsonTable")
    +  test("CREATE TABLE USING AS SELECT") {
    +    withTable("jsonTable") {
    +      sql(
    +        s"""
    +           |CREATE TABLE jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT a, b FROM jt
    +         """.stripMargin)
    +
    +      checkAnswer(
    +        sql("SELECT a, b FROM jsonTable"),
    +        sql("SELECT a, b FROM jt").collect())
    +    }
       }
     
    -  test("CREATE TEMPORARY TABLE AS SELECT based on the file without write permission") {
    +  test("CREATE TABLE USING AS SELECT based on the file without write permission") {
         val childPath = new File(path.toString, "child")
         path.mkdir()
    -    childPath.createNewFile()
         path.setWritable(false)
     
    -    val e = intercept[IOException] {
    +    val e = intercept[Exception] {
           sql(
             s"""
    -           |CREATE TEMPORARY TABLE jsonTable
    +           |CREATE TABLE jsonTable
                |USING json
                |OPTIONS (
    -           |  path '${path.toString}'
    +           |  path '${childPath.toString}'
                |) AS
                |SELECT a, b FROM jt
    -        """.stripMargin)
    +         """.stripMargin)
           sql("SELECT a, b FROM jsonTable").collect()
         }
    -    assert(e.getMessage().contains("Unable to clear output directory"))
     
    +    assert(e.getMessage().contains("Job aborted"))
         path.setWritable(true)
       }
     
       test("create a table, drop it and create another one with the same name") {
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a, b FROM jt
    -      """.stripMargin)
    -
    -    checkAnswer(
    -      sql("SELECT a, b FROM jsonTable"),
    -      sql("SELECT a, b FROM jt").collect())
    -
    -    val message = intercept[ParseException]{
    +    withTable("jsonTable") {
           sql(
             s"""
    -        |CREATE TEMPORARY TABLE IF NOT EXISTS jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a * 4 FROM jt
    -      """.stripMargin)
    -    }.getMessage
    -    assert(message.toLowerCase.contains("operation not allowed"))
    -
    -    // Overwrite the temporary table.
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a * 4 FROM jt
    -      """.stripMargin)
    -    checkAnswer(
    -      sql("SELECT * FROM jsonTable"),
    -      sql("SELECT a * 4 FROM jt").collect())
    -
    -    spark.catalog.dropTempView("jsonTable")
    -    // Explicitly delete the data.
    -    if (path.exists()) Utils.deleteRecursively(path)
    -
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT b FROM jt
    -      """.stripMargin)
    -
    -    checkAnswer(
    -      sql("SELECT * FROM jsonTable"),
    -      sql("SELECT b FROM jt").collect())
    -
    -    spark.catalog.dropTempView("jsonTable")
    -  }
    +           |CREATE TABLE jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT a, b FROM jt
    +         """.stripMargin)
    +
    +      checkAnswer(
    +        sql("SELECT a, b FROM jsonTable"),
    +        sql("SELECT a, b FROM jt").collect())
    +
    +      // Creates a table of the same name with flag "if not exists", nothing happens
    +      sql(
    +        s"""
    +           |CREATE TABLE IF NOT EXISTS jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT a * 4 FROM jt
    +         """.stripMargin)
    +      checkAnswer(
    +        sql("SELECT * FROM jsonTable"),
    +        sql("SELECT a, b FROM jt").collect())
    +
    +      // Explicitly drops the table and deletes the underlying data.
    +      sql("DROP TABLE jsonTable")
    +      if (path.exists()) Utils.deleteRecursively(path)
     
    -  test("CREATE TEMPORARY TABLE AS SELECT with IF NOT EXISTS is not allowed") {
    -    val message = intercept[ParseException]{
    +      // Creates a table of the same name again, this time we succeed.
           sql(
             s"""
    -        |CREATE TEMPORARY TABLE IF NOT EXISTS jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT b FROM jt
    -      """.stripMargin)
    -    }.getMessage
    -    assert(message.toLowerCase.contains("operation not allowed"))
    +           |CREATE TABLE jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT b FROM jt
    +         """.stripMargin)
    +
    +      checkAnswer(
    +        sql("SELECT * FROM jsonTable"),
    +        sql("SELECT b FROM jt").collect())
    --- End diff --
    
    Nit: The same here. You do not need to call `collect`


---
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 issue #13451: [SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE USING ...

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

    https://github.com/apache/spark/pull/13451
  
    Merged build finished. Test FAILed.


---
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 issue #13451: [SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE USING ...

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

    https://github.com/apache/spark/pull/13451
  
    **[Test build #59770 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59770/consoleFull)** for PR 13451 at commit [`fa1db6a`](https://github.com/apache/spark/commit/fa1db6a88423946b5c4986189f6d6c347cb34e2f).


---
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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the issue:

    https://github.com/apache/spark/pull/13451
  
    Merging into amster 2.0


---
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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

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

    https://github.com/apache/spark/pull/13451
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59828/
    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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the issue:

    https://github.com/apache/spark/pull/13451
  
    LGTM!


---
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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

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

    https://github.com/apache/spark/pull/13451
  
    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 #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USI...

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

    https://github.com/apache/spark/pull/13451#discussion_r65489614
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/sources/CreateTableAsSelectSuite.scala ---
    @@ -40,172 +43,175 @@ class CreateTableAsSelectSuite extends DataSourceTest with SharedSQLContext with
       override def afterAll(): Unit = {
         try {
           spark.catalog.dropTempView("jt")
    +      if (path.exists()) {
    +        Utils.deleteRecursively(path)
    +      }
         } finally {
           super.afterAll()
         }
       }
     
    -  after {
    -    Utils.deleteRecursively(path)
    +  before {
    +    if (path.exists()) {
    +      Utils.deleteRecursively(path)
    +    }
       }
     
    -  test("CREATE TEMPORARY TABLE AS SELECT") {
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a, b FROM jt
    -      """.stripMargin)
    -
    -    checkAnswer(
    -      sql("SELECT a, b FROM jsonTable"),
    -      sql("SELECT a, b FROM jt").collect())
    -
    -    spark.catalog.dropTempView("jsonTable")
    +  test("CREATE TABLE USING AS SELECT") {
    +    withTable("jsonTable") {
    +      sql(
    +        s"""
    +           |CREATE TABLE jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT a, b FROM jt
    +         """.stripMargin)
    +
    +      checkAnswer(
    +        sql("SELECT a, b FROM jsonTable"),
    +        sql("SELECT a, b FROM jt").collect())
    +    }
       }
     
    -  test("CREATE TEMPORARY TABLE AS SELECT based on the file without write permission") {
    +  test("CREATE TABLE USING AS SELECT based on the file without write permission") {
         val childPath = new File(path.toString, "child")
         path.mkdir()
    -    childPath.createNewFile()
         path.setWritable(false)
     
    -    val e = intercept[IOException] {
    +    val e = intercept[Exception] {
    --- End diff --
    
    What is the `Exception` type you got 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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

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

    https://github.com/apache/spark/pull/13451
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59818/
    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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

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

    https://github.com/apache/spark/pull/13451
  
    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 issue #13451: [SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE USING ...

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

    https://github.com/apache/spark/pull/13451
  
    Merged build finished. Test FAILed.


---
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 issue #13451: [SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE USING ...

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

    https://github.com/apache/spark/pull/13451
  
    **[Test build #59822 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59822/consoleFull)** for PR 13451 at commit [`b7bb43d`](https://github.com/apache/spark/commit/b7bb43dadd71ea6c24f50eb24234a778c68a21df).


---
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 issue #13451: [SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE USING ...

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

    https://github.com/apache/spark/pull/13451
  
    **[Test build #59776 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59776/consoleFull)** for PR 13451 at commit [`9bedd70`](https://github.com/apache/spark/commit/9bedd702546e5e02346eac50fac120656cae37e2).
     * This patch **fails Spark unit 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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

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

    https://github.com/apache/spark/pull/13451
  
    **[Test build #59828 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59828/consoleFull)** for PR 13451 at commit [`90fb80a`](https://github.com/apache/spark/commit/90fb80a61c701a80718eece67c64d848aa88a5aa).
     * 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 issue #13451: [SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE USING ...

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

    https://github.com/apache/spark/pull/13451
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59776/
    Test FAILed.


---
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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

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

    https://github.com/apache/spark/pull/13451
  
    **[Test build #59830 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59830/consoleFull)** for PR 13451 at commit [`2f0355f`](https://github.com/apache/spark/commit/2f0355fea74a029209d39a107e691ac95f0e0997).


---
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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

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

    https://github.com/apache/spark/pull/13451
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59834/
    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 issue #13451: [SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE USING ...

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

    https://github.com/apache/spark/pull/13451
  
    **[Test build #59828 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59828/consoleFull)** for PR 13451 at commit [`90fb80a`](https://github.com/apache/spark/commit/90fb80a61c701a80718eece67c64d848aa88a5aa).


---
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 #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USI...

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

    https://github.com/apache/spark/pull/13451#discussion_r65553085
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
    @@ -317,17 +317,19 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder {
           // Get the backing query.
           val query = plan(ctx.query)
     
    +      if (temp) {
    --- End diff --
    
    nit: can you please rename this to `temporary`


---
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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

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

    https://github.com/apache/spark/pull/13451
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59830/
    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 issue #13451: [SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE USING ...

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

    https://github.com/apache/spark/pull/13451
  
    **[Test build #59818 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59818/consoleFull)** for PR 13451 at commit [`d4f5ab6`](https://github.com/apache/spark/commit/d4f5ab6a3a68cee466181af3a0ca06427c15c25f).


---
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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

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

    https://github.com/apache/spark/pull/13451
  
    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 #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USI...

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

    https://github.com/apache/spark/pull/13451#discussion_r65489508
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/sources/CreateTableAsSelectSuite.scala ---
    @@ -40,172 +43,175 @@ class CreateTableAsSelectSuite extends DataSourceTest with SharedSQLContext with
       override def afterAll(): Unit = {
         try {
           spark.catalog.dropTempView("jt")
    +      if (path.exists()) {
    +        Utils.deleteRecursively(path)
    +      }
         } finally {
           super.afterAll()
         }
       }
     
    -  after {
    -    Utils.deleteRecursively(path)
    +  before {
    +    if (path.exists()) {
    +      Utils.deleteRecursively(path)
    +    }
       }
     
    -  test("CREATE TEMPORARY TABLE AS SELECT") {
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a, b FROM jt
    -      """.stripMargin)
    -
    -    checkAnswer(
    -      sql("SELECT a, b FROM jsonTable"),
    -      sql("SELECT a, b FROM jt").collect())
    -
    -    spark.catalog.dropTempView("jsonTable")
    +  test("CREATE TABLE USING AS SELECT") {
    +    withTable("jsonTable") {
    +      sql(
    +        s"""
    +           |CREATE TABLE jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT a, b FROM jt
    +         """.stripMargin)
    +
    +      checkAnswer(
    +        sql("SELECT a, b FROM jsonTable"),
    +        sql("SELECT a, b FROM jt").collect())
    --- End diff --
    
    Nit: you do not need to call `collect`


---
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 #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USI...

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

    https://github.com/apache/spark/pull/13451#discussion_r65489880
  
    --- Diff: sql/core/src/test/scala/org/apache/spark/sql/sources/CreateTableAsSelectSuite.scala ---
    @@ -40,172 +43,175 @@ class CreateTableAsSelectSuite extends DataSourceTest with SharedSQLContext with
       override def afterAll(): Unit = {
         try {
           spark.catalog.dropTempView("jt")
    +      if (path.exists()) {
    +        Utils.deleteRecursively(path)
    +      }
         } finally {
           super.afterAll()
         }
       }
     
    -  after {
    -    Utils.deleteRecursively(path)
    +  before {
    +    if (path.exists()) {
    +      Utils.deleteRecursively(path)
    +    }
       }
     
    -  test("CREATE TEMPORARY TABLE AS SELECT") {
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a, b FROM jt
    -      """.stripMargin)
    -
    -    checkAnswer(
    -      sql("SELECT a, b FROM jsonTable"),
    -      sql("SELECT a, b FROM jt").collect())
    -
    -    spark.catalog.dropTempView("jsonTable")
    +  test("CREATE TABLE USING AS SELECT") {
    +    withTable("jsonTable") {
    +      sql(
    +        s"""
    +           |CREATE TABLE jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT a, b FROM jt
    +         """.stripMargin)
    +
    +      checkAnswer(
    +        sql("SELECT a, b FROM jsonTable"),
    +        sql("SELECT a, b FROM jt").collect())
    +    }
       }
     
    -  test("CREATE TEMPORARY TABLE AS SELECT based on the file without write permission") {
    +  test("CREATE TABLE USING AS SELECT based on the file without write permission") {
         val childPath = new File(path.toString, "child")
         path.mkdir()
    -    childPath.createNewFile()
         path.setWritable(false)
     
    -    val e = intercept[IOException] {
    +    val e = intercept[Exception] {
           sql(
             s"""
    -           |CREATE TEMPORARY TABLE jsonTable
    +           |CREATE TABLE jsonTable
                |USING json
                |OPTIONS (
    -           |  path '${path.toString}'
    +           |  path '${childPath.toString}'
                |) AS
                |SELECT a, b FROM jt
    -        """.stripMargin)
    +         """.stripMargin)
           sql("SELECT a, b FROM jsonTable").collect()
         }
    -    assert(e.getMessage().contains("Unable to clear output directory"))
     
    +    assert(e.getMessage().contains("Job aborted"))
         path.setWritable(true)
       }
     
       test("create a table, drop it and create another one with the same name") {
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a, b FROM jt
    -      """.stripMargin)
    -
    -    checkAnswer(
    -      sql("SELECT a, b FROM jsonTable"),
    -      sql("SELECT a, b FROM jt").collect())
    -
    -    val message = intercept[ParseException]{
    +    withTable("jsonTable") {
           sql(
             s"""
    -        |CREATE TEMPORARY TABLE IF NOT EXISTS jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a * 4 FROM jt
    -      """.stripMargin)
    -    }.getMessage
    -    assert(message.toLowerCase.contains("operation not allowed"))
    -
    -    // Overwrite the temporary table.
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a * 4 FROM jt
    -      """.stripMargin)
    -    checkAnswer(
    -      sql("SELECT * FROM jsonTable"),
    -      sql("SELECT a * 4 FROM jt").collect())
    -
    -    spark.catalog.dropTempView("jsonTable")
    -    // Explicitly delete the data.
    -    if (path.exists()) Utils.deleteRecursively(path)
    -
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT b FROM jt
    -      """.stripMargin)
    -
    -    checkAnswer(
    -      sql("SELECT * FROM jsonTable"),
    -      sql("SELECT b FROM jt").collect())
    -
    -    spark.catalog.dropTempView("jsonTable")
    -  }
    +           |CREATE TABLE jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT a, b FROM jt
    +         """.stripMargin)
    +
    +      checkAnswer(
    +        sql("SELECT a, b FROM jsonTable"),
    +        sql("SELECT a, b FROM jt").collect())
    +
    +      // Creates a table of the same name with flag "if not exists", nothing happens
    +      sql(
    +        s"""
    +           |CREATE TABLE IF NOT EXISTS jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT a * 4 FROM jt
    +         """.stripMargin)
    +      checkAnswer(
    +        sql("SELECT * FROM jsonTable"),
    +        sql("SELECT a, b FROM jt").collect())
    +
    +      // Explicitly drops the table and deletes the underlying data.
    +      sql("DROP TABLE jsonTable")
    +      if (path.exists()) Utils.deleteRecursively(path)
     
    -  test("CREATE TEMPORARY TABLE AS SELECT with IF NOT EXISTS is not allowed") {
    -    val message = intercept[ParseException]{
    +      // Creates a table of the same name again, this time we succeed.
           sql(
             s"""
    -        |CREATE TEMPORARY TABLE IF NOT EXISTS jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT b FROM jt
    -      """.stripMargin)
    -    }.getMessage
    -    assert(message.toLowerCase.contains("operation not allowed"))
    +           |CREATE TABLE jsonTable
    +           |USING json
    +           |OPTIONS (
    +           |  path '${path.toString}'
    +           |) AS
    +           |SELECT b FROM jt
    +         """.stripMargin)
    +
    +      checkAnswer(
    +        sql("SELECT * FROM jsonTable"),
    +        sql("SELECT b FROM jt").collect())
    +    }
    +  }
    +
    +  test("disallows CREATE TEMPORARY TABLE ... USING ... AS query") {
    +    withTable("t") {
    +      val error = intercept[ParseException] {
    +        sql(
    +          s"""
    +             |CREATE TEMPORARY TABLE t USING PARQUET
    +             |OPTIONS (PATH '${path.toString}')
    +             |PARTITIONED BY (a)
    +             |AS SELECT 1 AS a, 2 AS b
    +           """.stripMargin
    +        )
    +      }.getMessage
    +      assert(error.contains("Operation not allowed") &&
    +        error.contains("CREATE TEMPORARY TABLE ... USING ... AS query"))
    +    }
       }
     
    -  test("a CTAS statement with column definitions is not allowed") {
    -    intercept[AnalysisException]{
    +  test("disallows CREATE EXTERNAL TABLE ... USING ... AS query") {
    +    withTable("t") {
    +      val error = intercept[ParseException] {
    +        sql(
    +          s"""
    +             |CREATE EXTERNAL TABLE t USING PARQUET
    +             |OPTIONS (PATH '${path.toString}')
    +             |AS SELECT 1 AS a, 2 AS b
    +           """.stripMargin
    +        )
    +      }.getMessage
    +
    +      assert(error.contains("Operation not allowed") &&
    +        error.contains("CREATE EXTERNAL TABLE ... USING"))
    +    }
    +  }
    +
    +  test("create table using as select - with partitioned by") {
    +    val catalog = spark.sessionState.catalog
    +    withTable("t") {
           sql(
             s"""
    -        |CREATE TEMPORARY TABLE jsonTable (a int, b string)
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a, b FROM jt
    -      """.stripMargin)
    +           |CREATE TABLE t USING PARQUET
    +           |OPTIONS (PATH '${path.toString}')
    +           |PARTITIONED BY (a)
    +           |AS SELECT 1 AS a, 2 AS b
    +           """.stripMargin
    +      )
    +      val table = catalog.getTableMetadata(TableIdentifier("t"))
    +      assert(DDLUtils.getPartitionColumnsFromTableProperties(table) == Seq("a"))
         }
       }
     
    -  test("it is not allowed to write to a table while querying it.") {
    -    sql(
    -      s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a, b FROM jt
    -      """.stripMargin)
    -
    -    val message = intercept[AnalysisException] {
    +  test("create table using as select - with bucket") {
    +    val catalog = spark.sessionState.catalog
    +    withTable("t") {
           sql(
             s"""
    -        |CREATE TEMPORARY TABLE jsonTable
    -        |USING json
    -        |OPTIONS (
    -        |  path '${path.toString}'
    -        |) AS
    -        |SELECT a, b FROM jsonTable
    -      """.stripMargin)
    -    }.getMessage
    -    assert(
    -      message.contains("Cannot overwrite table "),
    -      "Writing to a table while querying it should not be allowed.")
    +           |CREATE TABLE t USING PARQUET
    +           |OPTIONS (PATH '${path.toString}')
    +           |CLUSTERED BY (a) SORTED BY (b) INTO 5 BUCKETS
    +           |AS SELECT 1 AS a, 2 AS b
    +           """.stripMargin
    --- End diff --
    
    Nit: The indent issue


---
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 #13451: [SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE...

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

    https://github.com/apache/spark/pull/13451#discussion_r65470631
  
    --- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/MetastoreDataSourcesSuite.scala ---
    @@ -1104,4 +1104,22 @@ class MetastoreDataSourcesSuite extends QueryTest with SQLTestUtils with TestHiv
           }
         }
       }
    +
    +  test("CTAS: disallow create temporary table ... using ... as ...") {
    --- End diff --
    
    Can you move this test case to `HiveDDLCommandSuite`, which contains the Hive-specific parser test cases? You also can see the other CTAS test cases there. 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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

Posted by clockfly <gi...@git.apache.org>.
Github user clockfly commented on the issue:

    https://github.com/apache/spark/pull/13451
  
    Thanks @gatorsmile 


---
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 issue #13451: [SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE USING ...

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

    https://github.com/apache/spark/pull/13451
  
    **[Test build #59776 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59776/consoleFull)** for PR 13451 at commit [`9bedd70`](https://github.com/apache/spark/commit/9bedd702546e5e02346eac50fac120656cae37e2).


---
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 #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USI...

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

    https://github.com/apache/spark/pull/13451#discussion_r65572608
  
    --- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
    @@ -317,17 +317,19 @@ class SparkSqlAstBuilder(conf: SQLConf) extends AstBuilder {
           // Get the backing query.
           val query = plan(ctx.query)
     
    +      if (temp) {
    --- End diff --
    
    Probably not in the scope of this PR, temp is a existing variable defined at https://github.com/clockfly/spark/blob/aaca7f01882de19c9dda47500f7d4f03712d3383/sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala#L304. 


---
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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

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

    https://github.com/apache/spark/pull/13451
  
    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 issue #13451: [SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE USING ...

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

    https://github.com/apache/spark/pull/13451
  
    Test FAILed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59770/
    Test FAILed.


---
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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

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

    https://github.com/apache/spark/pull/13451
  
    **[Test build #59834 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59834/consoleFull)** for PR 13451 at commit [`aaca7f0`](https://github.com/apache/spark/commit/aaca7f01882de19c9dda47500f7d4f03712d3383).
     * 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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

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

    https://github.com/apache/spark/pull/13451
  
    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 #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USI...

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

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


---
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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

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

    https://github.com/apache/spark/pull/13451
  
    **[Test build #59830 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59830/consoleFull)** for PR 13451 at commit [`2f0355f`](https://github.com/apache/spark/commit/2f0355fea74a029209d39a107e691ac95f0e0997).
     * 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 issue #13451: [SPARK-15711][SQL] Ban CREATE TEMPORARY TABLE USING AS S...

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

    https://github.com/apache/spark/pull/13451
  
    Test PASSed.
    Refer to this link for build results (access rights to CI server needed): 
    https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59822/
    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 issue #13451: [SPARK-15711][SQL][WIP]Ban CREATE TEMPORARY TABLE USING ...

Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the issue:

    https://github.com/apache/spark/pull/13451
  
    Looks good. Can you add a test for the new exception that you throw?


---
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