Skip to content

Conversation

@gatorsmile
Copy link
Member

@gatorsmile gatorsmile commented Jul 22, 2016

What changes were proposed in this pull request?

Issue 1: Disallow Creating/Altering a View when the same-name Table Exists (without IF NOT EXISTS)
When we create OR alter a view, we check whether the view already exists. In the current implementation, if a table with the same name exists, we treat it as a view. However, this is not the right behavior. We should follow what Hive does. For example,

hive> CREATE TABLE tab1 (id int);
OK
Time taken: 0.196 seconds
hive> CREATE OR REPLACE VIEW tab1 AS SELECT * FROM t1;
FAILED: SemanticException [Error 10218]: Existing table is not a view
 The following is an existing table, not a view: default.tab1
hive> ALTER VIEW tab1 AS SELECT * FROM t1;
FAILED: SemanticException [Error 10218]: Existing table is not a view
 The following is an existing table, not a view: default.tab1
hive> CREATE VIEW IF NOT EXISTS tab1 AS SELECT * FROM t1;
OK
Time taken: 0.678 seconds

Issue 2: Strange Error when Issuing Load Table Against A View
Users should not be allowed to issue LOAD DATA against a view. Currently, when users doing it, we got a very strange runtime error. For example,

LOAD DATA LOCAL INPATH "$testData" INTO TABLE $viewName
java.lang.reflect.InvocationTargetException was thrown.
java.lang.reflect.InvocationTargetException
    at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
    at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.lang.reflect.Method.invoke(Method.java:606)
    at org.apache.spark.sql.hive.client.Shim_v0_14.loadTable(HiveShim.scala:680)

How was this patch tested?

Added test cases

@gatorsmile gatorsmile changed the title [SPARK-16678] [SPARK-16677] Fix two View-related bugs [SPARK-16678] [SPARK-16677] [SQL] Fix two View-related bugs Jul 22, 2016
if (sessionState.catalog.tableExists(tableIdentifier)) {
val tableMetadata = sessionState.catalog.getTableMetadata(tableIdentifier)
if (tableMetadata.tableType != CatalogTableType.VIEW) {
throw new AnalysisException(
Copy link
Member

@viirya viirya Jul 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If allowing existing, do we still need to throw this exception?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, a good question. Just did a check using Hive. It is allowed by Hive.

hive> CREATE VIEW IF NOT EXISTS tab1 AS SELECT * FROM t1;
OK
Time taken: 0.678 seconds

Let me move this a little bit down.

@SparkQA
Copy link

SparkQA commented Jul 22, 2016

Test build #62712 has finished for PR 14314 at commit 5c9be14.

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

@SparkQA
Copy link

SparkQA commented Jul 22, 2016

Test build #62716 has finished for PR 14314 at commit c698a4a.

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

@SparkQA
Copy link

SparkQA commented Jul 22, 2016

Test build #62717 has finished for PR 14314 at commit c64092c.

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

@gatorsmile
Copy link
Member Author

cc @cloud-fan

}.getMessage
assert(e.contains("The following is an existing table, not a view: `default`.`tab1`"))
e = intercept[AnalysisException] {
sql("CREATE VIEW tab1 AS SELECT * FROM jt")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what's hive's error message for this case? view already existing or not a view?

Copy link
Member Author

@gatorsmile gatorsmile Jul 22, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For this case, it shows table already existed. However, when we found a view/table exists, the error message is wrong:

View $tableIdentifier already exists. If you want to update the view definition,
 please use ALTER VIEW AS or CREATE OR REPLACE VIEW AS

We are unable to alter a table (that is not a view) by these commands ALTER VIEW AS or CREATE OR REPLACE VIEW AS.

See the source code

The message is better than table already exists, I think.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hive> CREATE VIEW tab1 AS SELECT * FROM t1;
FAILED: Execution Error, return code 1 from org.apache.hadoop.hive.ql.exec.DDLTask. AlreadyExistsException(message:Table tab1 already exists)

@SparkQA
Copy link

SparkQA commented Jul 25, 2016

Test build #62800 has finished for PR 14314 at commit f73d206.

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

@cloud-fan
Copy link
Contributor

LGTM, waiting for #14297

@SparkQA
Copy link

SparkQA commented Jul 25, 2016

Test build #62825 has finished for PR 14314 at commit 3f7ea99.

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

@cloud-fan
Copy link
Contributor

thanks, merging to master!

@asfgit asfgit closed this in 3fc4566 Jul 26, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants