[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[asfgit]

Github user asfgit closed the pull request at:

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


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100874425
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/csv/CSVSuite.scala
 ---
@@ -300,28 +306,34 @@ class CSVSuite extends QueryTest with 
SharedSQLContext with SQLTestUtils {
   }
 
   test("DDL test with empty file") {
-spark.sql(s"""
-   |CREATE TEMPORARY TABLE carsTable
+withView("carsTable") {
+  spark.sql(
+s"""
+   |CREATE TEMPORARY VIEW carsTable
|(yearMade double, makeName string, modelName string, comments 
string, grp string)
|USING csv
|OPTIONS (path "${testFile(emptyFile)}", header "false")
   """.stripMargin.replaceAll("\n", " "))
 
-assert(spark.sql("SELECT count(*) FROM carsTable").collect().head(0) 
=== 0)
+  assert(spark.sql("SELECT count(*) FROM carsTable").collect().head(0) 
=== 0)
+}
   }
 
   test("DDL test with schema") {
-spark.sql(s"""
-   |CREATE TEMPORARY TABLE carsTable
+withView("carsTable") {
+  spark.sql(
+s"""
+   |CREATE TEMPORARY VIEW carsTable
--- End diff --

the indention is wrong


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100874401
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/csv/CSVSuite.scala
 ---
@@ -300,28 +306,34 @@ class CSVSuite extends QueryTest with 
SharedSQLContext with SQLTestUtils {
   }
 
   test("DDL test with empty file") {
-spark.sql(s"""
-   |CREATE TEMPORARY TABLE carsTable
+withView("carsTable") {
+  spark.sql(
+s"""
+   |CREATE TEMPORARY VIEW carsTable
--- End diff --

the indention is wrong


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100874373
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/csv/CSVSuite.scala
 ---
@@ -213,27 +215,31 @@ class CSVSuite extends QueryTest with 
SharedSQLContext with SQLTestUtils {
   }
 
   test("DDL test with tab separated file") {
-spark.sql(
-  s"""
- |CREATE TEMPORARY TABLE carsTable USING csv
+withView("carsTable") {
+  spark.sql(
+s"""
+ |CREATE TEMPORARY VIEW carsTable USING csv
  |OPTIONS (path "${testFile(carsTsvFile)}", header "true", 
delimiter "\t")
   """.stripMargin.replaceAll("\n", " "))
 
-verifyCars(spark.table("carsTable"), numFields = 6, withHeader = true, 
checkHeader = false)
+  verifyCars(spark.table("carsTable"), numFields = 6, withHeader = 
true, checkHeader = false)
+}
   }
 
   test("DDL test parsing decimal type") {
-spark.sql(
-  s"""
- |CREATE TEMPORARY TABLE carsTable
+withView("carsTable") {
+  spark.sql(
+s"""
+ |CREATE TEMPORARY VIEW carsTable
--- End diff --

the indention is wrong


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100874283
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/csv/CSVSuite.scala
 ---
@@ -187,15 +187,17 @@ class CSVSuite extends QueryTest with 
SharedSQLContext with SQLTestUtils {
 
   test("test different encoding") {
 // scalastyle:off
-spark.sql(
-  s"""
- |CREATE TEMPORARY TABLE carsTable USING csv
+withView("carsTable") {
+  spark.sql(
+s"""
+ |CREATE TEMPORARY VIEW carsTable USING csv
--- End diff --

the indention is wrong


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100874352
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/csv/CSVSuite.scala
 ---
@@ -213,27 +215,31 @@ class CSVSuite extends QueryTest with 
SharedSQLContext with SQLTestUtils {
   }
 
   test("DDL test with tab separated file") {
-spark.sql(
-  s"""
- |CREATE TEMPORARY TABLE carsTable USING csv
+withView("carsTable") {
+  spark.sql(
+s"""
+ |CREATE TEMPORARY VIEW carsTable USING csv
--- End diff --

the indention is wrong


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100873904
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala 
---
@@ -971,7 +971,7 @@ class DDLSuite extends QueryTest with SharedSQLContext 
with BeforeAndAfterEach {
 |)
   """.stripMargin)
 assert(catalog.listTables("default") == Seq(TableIdentifier("tab1")))
-sql("DROP TABLE tab1")
+sql("DROP view tab1")
--- End diff --

nit: upper case "view"


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[xwu0226]

Github user xwu0226 commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r10070
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/csv/CSVSuite.scala
 ---
@@ -187,15 +187,18 @@ class CSVSuite extends QueryTest with 
SharedSQLContext with SQLTestUtils {
 
   test("test different encoding") {
 // scalastyle:off
-spark.sql(
-  s"""
+withView("carsTable") {
+  spark.sql(
+s"""
  |CREATE TEMPORARY TABLE carsTable USING csv
--- End diff --

@cloud-fan there are test cases that are related to ALTER TABLE for the 
temp table, should we keep such test cases for a while?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[xwu0226]

Github user xwu0226 commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100697250
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala 
---
@@ -1690,6 +1690,16 @@ class DDLSuite extends QueryTest with 
SharedSQLContext with BeforeAndAfterEach {
 }
   }
 
+  test("block creating duplicate temp table") {
+withView("t_temp") {
+  sql("CREATE TEMPORARY VIEW t_temp AS SELECT 1, 2")
+  val e = intercept[TempTableAlreadyExistsException] {
+sql("CREATE TEMPORARY TABLE t_temp (c3 int, c4 string) USING JSON")
+  }.getMessage
+  assert(e.contains("already exists"))
--- End diff --

OK. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[xwu0226]

Github user xwu0226 commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100697200
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/csv/CSVSuite.scala
 ---
@@ -213,27 +216,33 @@ class CSVSuite extends QueryTest with 
SharedSQLContext with SQLTestUtils {
   }
 
   test("DDL test with tab separated file") {
-spark.sql(
-  s"""
+withView("carsTable") {
+  spark.sql(
+s"""
  |CREATE TEMPORARY TABLE carsTable USING csv
  |OPTIONS (path "${testFile(carsTsvFile)}", header "true", 
delimiter "\t")
-  """.stripMargin.replaceAll("\n", " "))
+  """.stripMargin.replaceAll("\n", " "
 
-verifyCars(spark.table("carsTable"), numFields = 6, withHeader = true, 
checkHeader = false)
+))
--- End diff --

yes. 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[xwu0226]

Github user xwu0226 commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100697245
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
@@ -425,7 +425,9 @@ class SparkSqlAstBuilder(conf: SQLConf) extends 
AstBuilder {
 
 logWarning(s"CREATE TEMPORARY TABLE ... USING ... is deprecated, 
please use " +
   "CREATE TEMPORARY VIEW ... USING ... instead")
-CreateTempViewUsing(table, schema, replace = true, global = false, 
provider, options)
+// Since we don't support IF NOT EXISTS for temp table, we should 
not allow
+// replacing existing temp table, that may accidentally remove a 
temp view in use.
--- End diff --

OK. Good point. Thanks!


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

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[xwu0226]

Github user xwu0226 commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100697216
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/csv/CSVSuite.scala
 ---
@@ -187,15 +187,18 @@ class CSVSuite extends QueryTest with 
SharedSQLContext with SQLTestUtils {
 
   test("test different encoding") {
 // scalastyle:off
-spark.sql(
-  s"""
+withView("carsTable") {
+  spark.sql(
+s"""
  |CREATE TEMPORARY TABLE carsTable USING csv
--- End diff --

Will do. 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[xwu0226]

Github user xwu0226 commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100697194
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/csv/CSVSuite.scala
 ---
@@ -187,15 +187,18 @@ class CSVSuite extends QueryTest with 
SharedSQLContext with SQLTestUtils {
 
   test("test different encoding") {
 // scalastyle:off
-spark.sql(
-  s"""
+withView("carsTable") {
+  spark.sql(
+s"""
  |CREATE TEMPORARY TABLE carsTable USING csv
  |OPTIONS (path "${testFile(carsFile8859)}", header "true",
  |charset "iso-8859-1", delimiter "þ")
-  """.stripMargin.replaceAll("\n", " "))
+  """.stripMargin.
+  replaceAll("\n", " "))
--- End diff --

yes. 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[gatorsmile]

Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100682391
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala 
---
@@ -1690,6 +1690,16 @@ class DDLSuite extends QueryTest with 
SharedSQLContext with BeforeAndAfterEach {
 }
   }
 
+  test("block creating duplicate temp table") {
+withView("t_temp") {
+  sql("CREATE TEMPORARY VIEW t_temp AS SELECT 1, 2")
+  val e = intercept[TempTableAlreadyExistsException] {
+sql("CREATE TEMPORARY TABLE t_temp (c3 int, c4 string) USING JSON")
+  }.getMessage
+  assert(e.contains("already exists"))
--- End diff --

Please capture the whole error message.


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

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[gatorsmile]

Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100682360
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
@@ -425,7 +425,9 @@ class SparkSqlAstBuilder(conf: SQLConf) extends 
AstBuilder {
 
 logWarning(s"CREATE TEMPORARY TABLE ... USING ... is deprecated, 
please use " +
   "CREATE TEMPORARY VIEW ... USING ... instead")
-CreateTempViewUsing(table, schema, replace = true, global = false, 
provider, options)
+// Since we don't support IF NOT EXISTS for temp table, we should 
not allow
+// replacing existing temp table, that may accidentally remove a 
temp view in use.
--- End diff --

Just got your points... How about update the description to
```
Unlike CREATE TEMPORARY VIEW USING, CREATE TEMPORARY TABLE USING does not 
support IF NOT EXISTS, we should not allow users replacing the existing temp 
table; otherwise, it may accidentally remove a temp view in use.
```

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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[gatorsmile]

Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100682316
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
@@ -425,7 +425,9 @@ class SparkSqlAstBuilder(conf: SQLConf) extends 
AstBuilder {
 
 logWarning(s"CREATE TEMPORARY TABLE ... USING ... is deprecated, 
please use " +
   "CREATE TEMPORARY VIEW ... USING ... instead")
-CreateTempViewUsing(table, schema, replace = true, global = false, 
provider, options)
+// Since we don't support IF NOT EXISTS for temp table, we should 
not allow
+// replacing existing temp table, that may accidentally remove a 
temp view in use.
--- End diff --

The same 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[gatorsmile]

Github user gatorsmile commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100682314
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
@@ -425,7 +425,9 @@ class SparkSqlAstBuilder(conf: SQLConf) extends 
AstBuilder {
 
 logWarning(s"CREATE TEMPORARY TABLE ... USING ... is deprecated, 
please use " +
   "CREATE TEMPORARY VIEW ... USING ... instead")
-CreateTempViewUsing(table, schema, replace = true, global = false, 
provider, options)
+// Since we don't support IF NOT EXISTS for temp table, we should 
not allow
--- End diff --

`temp table` -> `temp view`


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100681712
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/csv/CSVSuite.scala
 ---
@@ -187,15 +187,18 @@ class CSVSuite extends QueryTest with 
SharedSQLContext with SQLTestUtils {
 
   test("test different encoding") {
 // scalastyle:off
-spark.sql(
-  s"""
+withView("carsTable") {
+  spark.sql(
+s"""
  |CREATE TEMPORARY TABLE carsTable USING csv
--- End diff --

as you are fixing this test suite, can you replace all `CREATE TEMPORARY 
TABLE` to `VIEW`?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100681701
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/csv/CSVSuite.scala
 ---
@@ -213,27 +216,33 @@ class CSVSuite extends QueryTest with 
SharedSQLContext with SQLTestUtils {
   }
 
   test("DDL test with tab separated file") {
-spark.sql(
-  s"""
+withView("carsTable") {
+  spark.sql(
+s"""
  |CREATE TEMPORARY TABLE carsTable USING csv
  |OPTIONS (path "${testFile(carsTsvFile)}", header "true", 
delimiter "\t")
-  """.stripMargin.replaceAll("\n", " "))
+  """.stripMargin.replaceAll("\n", " "
 
-verifyCars(spark.table("carsTable"), numFields = 6, withHeader = true, 
checkHeader = false)
+))
--- End diff --

it can be put in the previous line


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100681692
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/csv/CSVSuite.scala
 ---
@@ -187,15 +187,18 @@ class CSVSuite extends QueryTest with 
SharedSQLContext with SQLTestUtils {
 
   test("test different encoding") {
 // scalastyle:off
-spark.sql(
-  s"""
+withView("carsTable") {
+  spark.sql(
+s"""
  |CREATE TEMPORARY TABLE carsTable USING csv
  |OPTIONS (path "${testFile(carsFile8859)}", header "true",
  |charset "iso-8859-1", delimiter "þ")
-  """.stripMargin.replaceAll("\n", " "))
+  """.stripMargin.
+  replaceAll("\n", " "))
--- End diff --

it can be put in the previous line


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100681642
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
@@ -425,7 +425,9 @@ class SparkSqlAstBuilder(conf: SQLConf) extends 
AstBuilder {
 
 logWarning(s"CREATE TEMPORARY TABLE ... USING ... is deprecated, 
please use " +
--- End diff --

it was merged to 2.0, let's keep it


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

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[xwu0226]

Github user xwu0226 commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100647313
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
@@ -425,7 +425,9 @@ class SparkSqlAstBuilder(conf: SQLConf) extends 
AstBuilder {
 
 logWarning(s"CREATE TEMPORARY TABLE ... USING ... is deprecated, 
please use " +
--- End diff --

#13414 deprecated this. Merged on Jun 7th, 2016. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[xwu0226]

Github user xwu0226 commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100646859
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
@@ -425,7 +425,9 @@ class SparkSqlAstBuilder(conf: SQLConf) extends 
AstBuilder {
 
 logWarning(s"CREATE TEMPORARY TABLE ... USING ... is deprecated, 
please use " +
--- End diff --

I see. sorry. I misunderstood. Let me check further. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100646662
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
@@ -425,7 +425,9 @@ class SparkSqlAstBuilder(conf: SQLConf) extends 
AstBuilder {
 
 logWarning(s"CREATE TEMPORARY TABLE ... USING ... is deprecated, 
please use " +
--- End diff --

I mean it was moved from other places, so this warning should have been 
added earlier.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[xwu0226]

Github user xwu0226 commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100646267
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
@@ -425,7 +425,9 @@ class SparkSqlAstBuilder(conf: SQLConf) extends 
AstBuilder {
 
 logWarning(s"CREATE TEMPORARY TABLE ... USING ... is deprecated, 
please use " +
--- End diff --

Are we referring to the same PR? I found the change here 
https://github.com/apache/spark/pull/14482/files#diff-1bb4f7bd5a2656f48bcd3c857167a11bR362,
 where this WARN message is added in `SparkSQLParser.scala`


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100644845
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
@@ -425,7 +425,9 @@ class SparkSqlAstBuilder(conf: SQLConf) extends 
AstBuilder {
 
 logWarning(s"CREATE TEMPORARY TABLE ... USING ... is deprecated, 
please use " +
--- End diff --

no, it should be earlier: 
https://github.com/apache/spark/pull/14482/files#diff-7253a38df7e111ecf6b1ef71feba383bL425


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[xwu0226]

Github user xwu0226 commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100642343
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
@@ -425,7 +425,9 @@ class SparkSqlAstBuilder(conf: SQLConf) extends 
AstBuilder {
 
 logWarning(s"CREATE TEMPORARY TABLE ... USING ... is deprecated, 
please use " +
--- End diff --

@cloud-fan Thanks for reviewing!  I found that this deprecation was added 
by your PR #14482 back on 8/5/2016.  Do you think it is the right time to 
remove `CREATE TEMPORARY TABLE...` completely?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[cloud-fan]

Github user cloud-fan commented on a diff in the pull request:

https://github.com/apache/spark/pull/16878#discussion_r100597014
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/SparkSqlParser.scala ---
@@ -425,7 +425,9 @@ class SparkSqlAstBuilder(conf: SQLConf) extends 
AstBuilder {
 
 logWarning(s"CREATE TEMPORARY TABLE ... USING ... is deprecated, 
please use " +
--- End diff --

can you check when did we forbid this? maybe it's time to fully remove the 
support


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

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

[GitHub] spark pull request #16878: [SPARK-19539][SQL] Block duplicate temp table dur...

[xwu0226]

GitHub user xwu0226 opened a pull request:

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

[SPARK-19539][SQL] Block duplicate temp table during creation

## What changes were proposed in this pull request?
Current `CREATE TEMPORARY TABLE ... ` is deprecated and recommend users to 
use `CREATE TEMPORARY VIEW ...` And it does not support `IF NOT EXISTS `clause. 
However, if there is an existing temporary view defined, it is possible to 
unintentionally replace this existing view by issuing `CREATE TEMPORARY TABLE 
...`  with the same table/view name.

This PR is to disallow `CREATE TEMPORARY TABLE ...` with an existing view 
name. 
Under the cover, `CREATE TEMPORARY TABLE ...` will be changed to create 
temporary view, however, passing in a flag `replace=false`, instead of 
currently `true`. So when creating temporary view under the cover, if there is 
existing view with the same name, the operation will be blocked. 

## How was this patch tested?
New unit test case is added and updated some existing test cases to adapt 
the new behavior

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

$ git pull https://github.com/xwu0226/spark block_duplicate_temp_table

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

https://github.com/apache/spark/pull/16878.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 #16878


commit 9da7942dc9068debb23f357454ee718fd9399e56
Author: Xin Wu 
Date:   2017-02-10T00:24:31Z

don't allow create temporary table to replace existing temp view

commit 12702b359102fe5ad979947d1170bf0df75981af
Author: Xin Wu 
Date:   2017-02-10T00:54:11Z

update testcases

commit 133bd99b1158f08bf2fe5e278cd632efc39d7ddb
Author: Xin Wu 
Date:   2017-02-10T01:19:03Z

update testcases




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

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