[SPARK-6339][SQL] Supports CREATE TEMPORARY VIEW tableIdentifier AS query #12872

clockfly · 2016-05-03T15:59:21Z

What changes were proposed in this pull request?

This PR support new SQL syntax CREATE TEMPORARY VIEW.
Like:

CREATE TEMPORARY VIEW viewName AS SELECT * from xx
CREATE OR REPLACE TEMPORARY VIEW viewName AS SELECT * from xx
CREATE TEMPORARY VIEW viewName (c1 COMMENT 'blabla', c2 COMMENT 'blabla') AS SELECT * FROM xx

How was this patch tested?

Unit tests.

clockfly · 2016-05-03T16:00:18Z

work in progress.

rxin · 2016-05-03T16:09:30Z

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

This line can be removed now since we no longer support non native view. If there still a config option for native view, can you remove that as well?

rxin · 2016-05-03T16:09:45Z

@clockfly for work in progress pr, put WIP in the title.

…fier AS query This PR support new SQL syntax CREATE TEMPORARY VIEW. Unit tests. Author: Sean Zhong <seanzhong@apache.org>

hvanhovell · 2016-05-03T20:45:44Z

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

+  // Temporary view names should NOT contain database prefix like "database.table"
+  if (isTemporary && tableDesc.identifier.database.isDefined) {
+    val database = tableDesc.identifier.database.get
+    throw new AnalysisException(


Where does this semantic rule come from? Hive?

This is to be consistent with DataSet API. When registering a temp table, it will remove the database prefix, here is the code:

spark/sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala

Line 533 in 6ba17cd

sessionState.sqlParser.parseTableIdentifier(tableName).table,

liancheng · 2016-05-04T00:02:33Z

test this please

liancheng · 2016-05-04T00:03:27Z

add to whitelist

SparkQA · 2016-05-04T01:41:20Z

Test build #57693 has finished for PR 12872 at commit 8af667e.

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

liancheng · 2016-05-04T03:27:24Z

One thing that we've decided offline is that we should deprecate Dataset.registerTempTable() by Dataset.createTempView() in a follow-up PR. It's becoming confusing that we call the same thing "table" in Dataset API but "view" in SQL DDL.

I think the essential difference between a view and a table is that a view is basically a lineage, while a table is always materialized to disk. For example, data of temporary tables in Hive are written to scratch folder of the current session.

liancheng · 2016-05-04T03:28:36Z

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

Nit: Indentation is off

…t with "CREATE TEMPORARY TABLE" syntax

SparkQA · 2016-05-04T04:20:09Z

Test build #57707 has finished for PR 12872 at commit a497cf3.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-05-04T05:22:40Z

Test build #57710 has finished for PR 12872 at commit 1e20bb0.

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

liancheng · 2016-05-04T09:54:23Z

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

      checkAnswer(sql("select count(*) FROM jtv2"), Row(2))
+
+      // Checks temporary views
+      sql("CREATE TEMPORARY VIEW temp_jtv1 AS SELECT * FROM jt WHERE id > 3").collect()


Nit: For DDL commands like this one, you don't need to call .collect(). Unlike SELECT queries, commands are executed eagerly.

liancheng · 2016-05-04T10:25:00Z

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

-        sql("DROP VIEW testView")
+
+        val df = (1 until 10).map(i => i -> i).toDF("i", "j")
+        df.write.format("json").saveAsTable("jt2")


Similar as above, it's a waste to write a new persistent table here.

The sorted persistent table is used by the checking logic below.

checkAnswer(sql("SELECT * FROM testView ORDER BY i"), (1 to 9).map(i => Row(i, i)))

liancheng · 2016-05-04T12:46:18Z

LGTM

SparkQA · 2016-05-04T14:13:48Z

Test build #57756 has finished for PR 12872 at commit 85d121c.

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

liancheng · 2016-05-04T14:33:32Z

Would like to ask @yhuai to have a look at this.

yhuai · 2016-05-04T23:25:04Z

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

+      if (tableDesc.schema.isEmpty) {
+        analyzedPlan
+      } else {
+        val projectList = analyzedPlan.output.zip(tableDesc.schema).map {


Seems you want to check if analyzedPlan.output and tableDesc.schema have the same number of columns?

we already check the lenght

yhuai · 2016-05-05T00:24:04Z

LGTM

yhuai · 2016-05-05T01:31:53Z

Merged to master and branch 2.0.

…uery ## What changes were proposed in this pull request? This PR support new SQL syntax CREATE TEMPORARY VIEW. Like: ``` CREATE TEMPORARY VIEW viewName AS SELECT * from xx CREATE OR REPLACE TEMPORARY VIEW viewName AS SELECT * from xx CREATE TEMPORARY VIEW viewName (c1 COMMENT 'blabla', c2 COMMENT 'blabla') AS SELECT * FROM xx ``` ## How was this patch tested? Unit tests. Author: Sean Zhong <clockfly@gmail.com> Closes #12872 from clockfly/spark-6399. (cherry picked from commit 8fb1463) Signed-off-by: Yin Huai <yhuai@databricks.com>

gatorsmile · 2016-08-07T16:46:31Z

Currently, the existing DDL behaviors for views when users do not specify the database name are described below:

CREATE OR REPLACE TEMPORARY VIEW view_name creates/alters the TEMPORARY view.
CREATE OR REPLACE VIEW view_name creates/alters the PERSISTENT view.
DROP VIEW view_name OR SELECT... FROM view_name is always first applied to a TEMPORARY view, if existing. If the temporary view does not exist, we try to drop/fetch the PERSISTENT view, if existing.
ALTER VIEW view_name is only applicable to the PERSISTENT view, even if the temporary view with the same name exists.

@clockfly @rxin @yhuai @liancheng @hvanhovell @cloud-fan We are using different name resolution rules for different DDL statements, should we make them consistent?

yhuai · 2016-08-07T17:09:55Z

Can you be more specific on the inconsistency? Seems ALTER VIEW view_name is the only inconsistent command?

gatorsmile · 2016-08-07T17:50:34Z

Yeah, ALTER VIEW should be consistent with DROP VIEW, if we use the same naming rule. Should I submit a PR for it?

Another potential issue to users is the behaviors of CRAETE VIEW and CREATE TEMPORARY VIEW when users do not specify the database names. See the following example:

sql(s"CREATE TEMPORARY VIEW $viewName AS SELECT * FROM $tabName WHERE ID < 3")
sql(s"CREATE VIEW $viewName AS SELECT * FROM $tabName")

When we processing the second statement, we simply add CURRENT_DATABASE to make it a fully qualified view name. However, if users do not specify the fully qualified name in the subsequent SELECT/DROP, the persistent view is shadowed by the temporary view with the same name. The returned results might be a surprise to the Spark users, because they might not realize there exist such a temporary view in the existing session.

Of course, the existing behavior is right, but I think the better way is to force users to specify the database name when creating a persistent view if there exists a temporary view with the same name. That means, we can issue an error message here in this specific case.

clockfly force-pushed the spark-6399 branch from 301855e to 6689db7 Compare May 3, 2016 16:06

rxin reviewed May 3, 2016
View reviewed changes

* [SPARK-6339][SQL] Supports create CREATE TEMPORARY VIEW tableIdenti…

ce139a6

…fier AS query This PR support new SQL syntax CREATE TEMPORARY VIEW. Unit tests. Author: Sean Zhong <seanzhong@apache.org>

clockfly force-pushed the spark-6399 branch from 6689db7 to ce139a6 Compare May 3, 2016 16:11

clockfly changed the title ~~[SPARK-6339][SQL] Supports create CREATE TEMPORARY VIEW tableIdentifier AS query~~ [SPARK-6339][SQL][WIP] Supports create CREATE TEMPORARY VIEW tableIdentifier AS query May 3, 2016

remove SQLConf.NATIVE_VIEW conf

8af667e

hvanhovell reviewed May 3, 2016
View reviewed changes

liancheng reviewed May 4, 2016
View reviewed changes

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

Copy link

Contributor

liancheng May 4, 2016

Choose a reason for hiding this comment

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

Nit: Indentation is off

clockfly added 2 commits May 4, 2016 11:40

disallow syntax "CREATE TEMPORARY VIEW IF NOT EXISTS" to be consisten…

238db12

…t with "CREATE TEMPORARY TABLE" syntax

fix indentation

1e20bb0

clockfly force-pushed the spark-6399 branch from a497cf3 to 1e20bb0 Compare May 4, 2016 03:46

clockfly changed the title ~~[SPARK-6339][SQL][WIP] Supports create CREATE TEMPORARY VIEW tableIdentifier AS query~~ [SPARK-6339][SQL] Supports create CREATE TEMPORARY VIEW tableIdentifier AS query May 4, 2016

liancheng reviewed May 4, 2016
View reviewed changes

fix style

85d121c

clockfly changed the title ~~[SPARK-6339][SQL] Supports create CREATE TEMPORARY VIEW tableIdentifier AS query~~ [SPARK-6339][SQL] Supports CREATE TEMPORARY VIEW tableIdentifier AS query May 4, 2016

yhuai reviewed May 4, 2016
View reviewed changes

asfgit closed this in 8fb1463 May 5, 2016

[SPARK-6339][SQL] Supports CREATE TEMPORARY VIEW tableIdentifier AS query #12872

[SPARK-6339][SQL] Supports CREATE TEMPORARY VIEW tableIdentifier AS query #12872

Uh oh!

Conversation

clockfly commented May 3, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

clockfly commented May 3, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rxin commented May 3, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liancheng commented May 4, 2016

Uh oh!

liancheng commented May 4, 2016

Uh oh!

SparkQA commented May 4, 2016

Uh oh!

liancheng commented May 4, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

SparkQA commented May 4, 2016

Uh oh!

SparkQA commented May 4, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

liancheng commented May 4, 2016

Uh oh!

SparkQA commented May 4, 2016

Uh oh!

liancheng commented May 4, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yhuai commented May 5, 2016

Uh oh!

yhuai commented May 5, 2016

Uh oh!

gatorsmile commented Aug 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

yhuai commented Aug 7, 2016

Uh oh!

gatorsmile commented Aug 7, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

clockfly commented May 3, 2016 •

edited

Loading

liancheng commented May 4, 2016 •

edited

Loading

gatorsmile commented Aug 7, 2016 •

edited

Loading

gatorsmile commented Aug 7, 2016 •

edited

Loading