[GitHub] spark pull request #20227: [SPARK-23035] Fix warning: TEMPORARY TABLE ... US...

[dongjoon-hyun]

Github user dongjoon-hyun commented on a diff in the pull request:

https://github.com/apache/spark/pull/20227#discussion_r160867160
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala 
---
@@ -814,7 +814,7 @@ abstract class DDLSuite extends QueryTest with 
SQLTestUtils {
 withTempView("tab1") {
   sql(
 """
-  |CREATE TEMPORARY TABLE tab1
--- End diff --

For this one, I think we should keep the test coverage because we still 
support this legacy syntax.
Could you revert this kind of changes?


---

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

[GitHub] spark pull request #20227: [SPARK-23035] Fix warning: TEMPORARY TABLE ... US...

[dongjoon-hyun]

Github user dongjoon-hyun commented on a diff in the pull request:

https://github.com/apache/spark/pull/20227#discussion_r160867069
  
--- Diff: 
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/AlreadyExistException.scala
 ---
@@ -33,6 +33,9 @@ class TableAlreadyExistsException(db: String, table: 
String)
 class TempTableAlreadyExistsException(table: String)
   extends AnalysisException(s"Temporary table '$table' already exists")
 
+class TempViewAlreadyExistsException(table: String)
+  extends AnalysisException(s"Temporary view '$table' already exists")
--- End diff --

+1 for adding this.


---

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

[GitHub] spark pull request #20227: [SPARK-23035] Fix warning: TEMPORARY TABLE ... US...

[xubo245]

GitHub user xubo245 opened a pull request:

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

[SPARK-23035] Fix warning: TEMPORARY TABLE ... USING ... is deprecated and 
use TempViewAlreadyExistsException when create temp view


Fix warning: TEMPORARY TABLE ... USING ... is deprecated and use 
TempViewAlreadyExistsException when create temp view
There are warning when run test: test("rename temporary view - destination 
table with database name")

Another problem, it throw TempTableAlreadyExistsException and output 
"Temporary table '$table' already exists" when we create temp view by using 
org.apache.spark.sql.catalyst.catalog.GlobalTempViewManager#create, it's 
improper.

## What changes were proposed in this pull request?

Fix some warning by changing "TEMPORARY TABLE ... USING ... " to "TEMPORARY 
VIEW ... USING 
... "

Fix improper information about TempTableAlreadyExistsException when create 
temp view

## How was this patch tested?

use old test cases, such as " test("create temporary view using") "




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

$ git pull https://github.com/xubo245/spark fixDeprecated

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

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


commit b97834a58fb2a0a98eb2645bd9e77e97209b
Author: xubo245 <601450868@...>
Date:   2018-01-11T01:58:48Z

[SPARK-23035] Fix warning: TEMPORARY TABLE ... USING ... is deprecated and 
use TempViewAlreadyExistsException when create temp view

Fix warning: TEMPORARY TABLE ... USING ... is deprecated and use 
TempViewAlreadyExistsException when create temp view
There are warning when run test: test("rename temporary view - destination 
table with database name")

Another problem, it throw TempTableAlreadyExistsException and output 
"Temporary table '$table' already exists" when we create temp view by using 
org.apache.spark.sql.catalyst.catalog.GlobalTempViewManager#create, it's 
improper.




---

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