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/05/03 15:59:23 UTC
[GitHub] spark pull request: [SPARK-6339][SQL] Supports create CREATE TEMPO...
GitHub user clockfly opened a pull request:
https://github.com/apache/spark/pull/12872
[SPARK-6339][SQL] Supports create CREATE TEMPORARY VIEW tableIdentifier AS query
## 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 IF NOT EXISTS viewName AS SELECT * from xx
CREATE OR REPLACE TEMPORARY VIEW IF NOT EXISTS viewName AS SELECT * from xx
CREATE TEMPORARY VIEW IF NOT EXISTS viewName (c1 COMMENT 'blabla', c2 COMMENT 'blabla') AS SELECT * FROM xx
```
## How was this patch tested?
Unit tests.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/clockfly/spark spark-6399
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/12872.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 #12872
----
commit 301855e22cc8aea628341232616274e17e60c4c2
Author: Sean Zhong <cl...@gmail.com>
Date: 2016-04-30T09:08:18Z
* [SPARK-6339][SQL] Supports create CREATE TEMPORARY VIEW tableIdentifier AS query
This PR support new SQL syntax CREATE TEMPORARY VIEW.
Unit tests.
Author: Sean Zhong <se...@apache.org>
----
---
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: [SPARK-6339][SQL] Supports CREATE TEMPORARY VI...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r62131182
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
@@ -71,29 +87,59 @@ case class CreateViewCommand(
require(tableDesc.schema == Nil || tableDesc.schema.length == analyzedPlan.output.length)
val sessionState = sparkSession.sessionState
- if (sessionState.catalog.tableExists(tableIdentifier)) {
- if (allowExisting) {
- // Handles `CREATE VIEW IF NOT EXISTS v0 AS SELECT ...`. Does nothing when the target view
- // already exists.
- } else if (replace) {
- // Handles `CREATE OR REPLACE VIEW v0 AS SELECT ...`
- sessionState.catalog.alterTable(prepareTable(sparkSession, analyzedPlan))
+ if (isTemporary) {
+ createTemporaryView(tableDesc.identifier, sparkSession, analyzedPlan)
+ } else {
+ // Adds default database for permanent table if it doesn't exist, so that tableExists()
+ // only check permanent tables.
+ val database = tableDesc.identifier.database.getOrElse(
+ sessionState.catalog.getCurrentDatabase)
+ val tableIdentifier = tableDesc.identifier.copy(database = Option(database))
+
+ if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (allowExisting) {
+ // Handles `CREATE VIEW IF NOT EXISTS v0 AS SELECT ...`. Does nothing when the target view
+ // already exists.
+ } else if (replace) {
+ // Handles `CREATE OR REPLACE VIEW v0 AS SELECT ...`
+ sessionState.catalog.alterTable(prepareTable(sparkSession, analyzedPlan))
+ } else {
+ // Handles `CREATE VIEW v0 AS SELECT ...`. Throws exception when the target view already
+ // exists.
+ throw new AnalysisException(
+ s"View $tableIdentifier already exists. If you want to update the view definition, " +
+ "please use ALTER VIEW AS or CREATE OR REPLACE VIEW AS")
+ }
} else {
- // Handles `CREATE VIEW v0 AS SELECT ...`. Throws exception when the target view already
- // exists.
- throw new AnalysisException(s"View $tableIdentifier already exists. " +
- "If you want to update the view definition, please use ALTER VIEW AS or " +
- "CREATE OR REPLACE VIEW AS")
+ // Create the view if it doesn't exist.
+ sessionState.catalog.createTable(
+ prepareTable(sparkSession, analyzedPlan), ignoreIfExists = false)
}
- } else {
- // Create the view if it doesn't exist.
- sessionState.catalog.createTable(
- prepareTable(sparkSession, analyzedPlan), ignoreIfExists = false)
}
-
Seq.empty[Row]
}
+ private def createTemporaryView(
+ table: TableIdentifier, sparkSession: SparkSession, analyzedPlan: LogicalPlan): Unit = {
+
+ val sessionState = sparkSession.sessionState
+ val catalog = sessionState.catalog
+
+ // Projects column names to alias names
+ val logicalPlan = {
+ if (tableDesc.schema.isEmpty) {
+ analyzedPlan
+ } else {
+ val projectList = analyzedPlan.output.zip(tableDesc.schema).map {
--- End diff --
Seems you want to check if `analyzedPlan.output` and `tableDesc.schema` have the same number of columns?
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216877975
**[Test build #57756 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57756/consoleFull)** for PR 12872 at commit [`85d121c`](https://github.com/apache/spark/commit/85d121cc79e3a17bb5d22eb7000997b4d0f981cc).
* 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 pull request: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216577266
Can one of the admins verify this patch?
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r62018062
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala ---
@@ -69,18 +108,71 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
}
}
+ test("correctly parse CREATE TEMPORARY VIEW statement") {
+ withView("testView") {
+ sql(
+ """CREATE TEMPORARY VIEW
+ |testView (c1 COMMENT 'blabla', c2 COMMENT 'blabla')
+ |TBLPROPERTIES ('a' = 'b')
+ |AS SELECT * FROM jt""".stripMargin)
+ checkAnswer(sql("SELECT c1, c2 FROM testView ORDER BY c1"), (1 to 9).map(i => Row(i, i)))
+ }
+ }
+
+ test("should NOT allow CREATE TEMPORARY VIEW when TEMPORARY VIEW with same name exists") {
+ withView("testView") {
+ sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt")
+
+ val e = intercept[AnalysisException] {
+ sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt").collect()
+ }
+
+ assert(e.message.contains("Temporary table") && e.message.contains("already exists"))
+ }
+ }
+
+ test("should allow CREATE TEMPORARY VIEW when a permanent VIEW with same name exists") {
+ withView("testView", "default.testView") {
+ sql("CREATE VIEW testView AS SELECT id FROM jt")
+ sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt").collect()
+ }
+ }
+
+ test("should allow CREATE permanent VIEW when a TEMPORARY VIEW with same name exists") {
+ withView("testView", "default.testView") {
+ sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt")
+ sql("CREATE VIEW testView AS SELECT id FROM jt").collect()
+ }
+ }
+
test("correctly handle CREATE VIEW IF NOT EXISTS") {
withSQLConf(SQLConf.NATIVE_VIEW.key -> "true") {
withTable("jt2") {
- sql("CREATE VIEW testView AS SELECT id FROM jt")
+ withView("testView") {
+ sql("CREATE VIEW testView AS SELECT id FROM jt")
- val df = (1 until 10).map(i => i -> i).toDF("i", "j")
- df.write.format("json").saveAsTable("jt2")
- sql("CREATE VIEW IF NOT EXISTS testView AS SELECT * FROM jt2")
+ val df = (1 until 10).map(i => i -> i).toDF("i", "j")
+ df.write.format("json").saveAsTable("jt2")
+ sql("CREATE VIEW IF NOT EXISTS testView AS SELECT * FROM jt2")
+
+ // make sure our view doesn't change.
+ checkAnswer(sql("SELECT * FROM testView ORDER BY id"), (1 to 9).map(i => Row(i)))
+ }
+ }
+ }
+ }
- // make sure our view doesn't change.
+ test(s"correctly handle CREATE OR REPLACE TEMPORARY VIEW") {
+ withTable("jt2") {
+ withView("testView") {
+ sql("CREATE OR REPLACE TEMPORARY VIEW testView AS SELECT id FROM jt")
checkAnswer(sql("SELECT * FROM testView ORDER BY id"), (1 to 9).map(i => Row(i)))
- sql("DROP VIEW testView")
+
+ val df = (1 until 10).map(i => i -> i).toDF("i", "j")
+ df.write.format("json").saveAsTable("jt2")
--- End diff --
Similar as above, it's a waste to write a new persistent table 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 pull request: [SPARK-6339][SQL] Supports CREATE TEMPORARY VI...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-217054995
Merged to master and branch 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 pull request: [SPARK-6339][SQL][WIP] Supports create CREATE ...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216733049
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.
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216737681
**[Test build #57707 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57707/consoleFull)** for PR 12872 at commit [`a497cf3`](https://github.com/apache/spark/commit/a497cf3894d7ef6d39fc370665a7767e4cc2c76c).
* 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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216737747
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57707/
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 pull request: [SPARK-6339][SQL][WIP] Supports create CREATE ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216704088
**[Test build #57693 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57693/consoleFull)** for PR 12872 at commit [`8af667e`](https://github.com/apache/spark/commit/8af667eda1f6a33774734b5a42ffae62bca22015).
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216878330
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57756/
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216878324
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 #12872: [SPARK-6339][SQL] Supports CREATE TEMPORARY VIEW tableId...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/12872
Currently, the existing DDL behaviors for temporary views when users do not specify the database name:
- `CREATE OR REPLACE TEMPORARY VIEW view_name` alters the TEMPORARY view if the temporary view already exists.
- `DROP VIEW view_name` OR `SELECT... FROM view_name` will be always first applied to a TEMPORARY view, if existing. If the temporary view does not exist, we will 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 Should we make them consistent?
---
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: [SPARK-6339][SQL][WIP] Supports create CREATE ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216717929
**[Test build #57693 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57693/consoleFull)** for PR 12872 at commit [`8af667e`](https://github.com/apache/spark/commit/8af667eda1f6a33774734b5a42ffae62bca22015).
* 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 #12872: [SPARK-6339][SQL] Supports CREATE TEMPORARY VIEW tableId...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/12872
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 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.
---
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: [SPARK-6339][SQL][WIP] Supports create CREATE ...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r61953261
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
@@ -71,27 +81,69 @@ case class CreateViewCommand(
require(tableDesc.schema == Nil || tableDesc.schema.length == analyzedPlan.output.length)
val sessionState = sparkSession.sessionState
- if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (isTemporary) {
+ createTemporaryView(tableDesc.identifier, sparkSession, analyzedPlan)
+ } else {
+ // Adds default database for permanent table if it doesn't exist, so that tableExists()
+ // only check permanent tables.
+ val database = tableDesc.identifier.database.getOrElse(
+ sessionState.catalog.getCurrentDatabase)
+ val tableIdentifier = tableDesc.identifier.copy(database = Option(database))
+
+ if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (allowExisting) {
--- End diff --
You can simplify the code here by ommiting this if clause and adding the explicit check `!allowExisting` to the else branch; this also makes is easier to follow the logic. Keep the comment though.
---
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: [SPARK-6339][SQL][WIP] Supports create CREATE ...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r61954025
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
@@ -71,27 +81,69 @@ case class CreateViewCommand(
require(tableDesc.schema == Nil || tableDesc.schema.length == analyzedPlan.output.length)
val sessionState = sparkSession.sessionState
- if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (isTemporary) {
+ createTemporaryView(tableDesc.identifier, sparkSession, analyzedPlan)
+ } else {
+ // Adds default database for permanent table if it doesn't exist, so that tableExists()
+ // only check permanent tables.
+ val database = tableDesc.identifier.database.getOrElse(
+ sessionState.catalog.getCurrentDatabase)
+ val tableIdentifier = tableDesc.identifier.copy(database = Option(database))
+
+ if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (allowExisting) {
+ // Handles `CREATE VIEW IF NOT EXISTS v0 AS SELECT ...`. Does nothing when the target view
+ // already exists.
+ } else if (replace) {
+ // Handles `CREATE OR REPLACE VIEW v0 AS SELECT ...`
+ sessionState.catalog.alterTable(prepareTable(sparkSession, analyzedPlan))
+ } else {
+ // Handles `CREATE VIEW v0 AS SELECT ...`. Throws exception when the target view already
+ // exists.
+ throw new AnalysisException(s"View $tableIdentifier already exists. " +
+ "If you want to update the view definition, please use ALTER VIEW AS or " +
+ "CREATE OR REPLACE VIEW AS")
+ }
+ } else {
+ // Create the view if it doesn't exist.
+ sessionState.catalog.createTable(
+ prepareTable(sparkSession, analyzedPlan), ignoreIfExists = false)
+ }
+ }
+ Seq.empty[Row]
+ }
+
+ private def createTemporaryView(table: TableIdentifier, sparkSession: SparkSession,
+ analyzedPlan: LogicalPlan): Unit = {
+
+ val sessionState = sparkSession.sessionState
+ val catalog = sessionState.catalog
+
+ // Projects column names to alias names
+ val logicalPlan = {
--- End diff --
Is this possible? To have different columns and query output?
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r62014991
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala ---
@@ -37,11 +37,21 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
sqlContext.sql(s"DROP TABLE IF EXISTS jt")
}
- test("nested views") {
- withView("jtv1", "jtv2") {
+ test("nested views (interleaved with temporary views)") {
+ withView("jtv1", "jtv2", "jtv3", "temp_jtv1", "temp_jtv2", "temp_jtv3") {
sql("CREATE VIEW jtv1 AS SELECT * FROM jt WHERE id > 3").collect()
sql("CREATE VIEW jtv2 AS SELECT * FROM jtv1 WHERE id < 6").collect()
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()
--- End diff --
Nit: For DDL commands like this one, you don't need to call `.collect()`. Unlike `SELECT` queries, commands are executed eagerly.
---
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: [SPARK-6339][SQL] Supports CREATE TEMPORARY VI...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-217046086
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 pull request: [SPARK-6339][SQL][WIP] Supports create CREATE ...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216703301
test this please
---
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: [SPARK-6339][SQL][WIP] Supports create CREATE ...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r61987175
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
@@ -71,29 +87,59 @@ case class CreateViewCommand(
require(tableDesc.schema == Nil || tableDesc.schema.length == analyzedPlan.output.length)
val sessionState = sparkSession.sessionState
- if (sessionState.catalog.tableExists(tableIdentifier)) {
- if (allowExisting) {
- // Handles `CREATE VIEW IF NOT EXISTS v0 AS SELECT ...`. Does nothing when the target view
- // already exists.
- } else if (replace) {
- // Handles `CREATE OR REPLACE VIEW v0 AS SELECT ...`
- sessionState.catalog.alterTable(prepareTable(sparkSession, analyzedPlan))
+ if (isTemporary) {
+ createTemporaryView(tableDesc.identifier, sparkSession, analyzedPlan)
+ } else {
+ // Adds default database for permanent table if it doesn't exist, so that tableExists()
+ // only check permanent tables.
+ val database = tableDesc.identifier.database.getOrElse(
+ sessionState.catalog.getCurrentDatabase)
+ val tableIdentifier = tableDesc.identifier.copy(database = Option(database))
+
+ if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (allowExisting) {
+ // Handles `CREATE VIEW IF NOT EXISTS v0 AS SELECT ...`. Does nothing when the target view
+ // already exists.
+ } else if (replace) {
+ // Handles `CREATE OR REPLACE VIEW v0 AS SELECT ...`
+ sessionState.catalog.alterTable(prepareTable(sparkSession, analyzedPlan))
+ } else {
+ // Handles `CREATE VIEW v0 AS SELECT ...`. Throws exception when the target view already
+ // exists.
+ throw new AnalysisException(s"View $tableIdentifier already exists. " +
--- End diff --
Nit: Wrapping this line using the following style:
```scala
throw new AnalysisException(
s"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 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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r62017918
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala ---
@@ -69,18 +108,71 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
}
}
+ test("correctly parse CREATE TEMPORARY VIEW statement") {
+ withView("testView") {
+ sql(
+ """CREATE TEMPORARY VIEW
+ |testView (c1 COMMENT 'blabla', c2 COMMENT 'blabla')
+ |TBLPROPERTIES ('a' = 'b')
+ |AS SELECT * FROM jt""".stripMargin)
+ checkAnswer(sql("SELECT c1, c2 FROM testView ORDER BY c1"), (1 to 9).map(i => Row(i, i)))
+ }
+ }
+
+ test("should NOT allow CREATE TEMPORARY VIEW when TEMPORARY VIEW with same name exists") {
+ withView("testView") {
+ sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt")
+
+ val e = intercept[AnalysisException] {
+ sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt").collect()
+ }
+
+ assert(e.message.contains("Temporary table") && e.message.contains("already exists"))
+ }
+ }
+
+ test("should allow CREATE TEMPORARY VIEW when a permanent VIEW with same name exists") {
+ withView("testView", "default.testView") {
+ sql("CREATE VIEW testView AS SELECT id FROM jt")
+ sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt").collect()
+ }
+ }
+
+ test("should allow CREATE permanent VIEW when a TEMPORARY VIEW with same name exists") {
+ withView("testView", "default.testView") {
+ sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt")
+ sql("CREATE VIEW testView AS SELECT id FROM jt").collect()
+ }
+ }
+
test("correctly handle CREATE VIEW IF NOT EXISTS") {
withSQLConf(SQLConf.NATIVE_VIEW.key -> "true") {
withTable("jt2") {
- sql("CREATE VIEW testView AS SELECT id FROM jt")
+ withView("testView") {
+ sql("CREATE VIEW testView AS SELECT id FROM jt")
- val df = (1 until 10).map(i => i -> i).toDF("i", "j")
- df.write.format("json").saveAsTable("jt2")
- sql("CREATE VIEW IF NOT EXISTS testView AS SELECT * FROM jt2")
+ val df = (1 until 10).map(i => i -> i).toDF("i", "j")
+ df.write.format("json").saveAsTable("jt2")
+ sql("CREATE VIEW IF NOT EXISTS testView AS SELECT * FROM jt2")
--- End diff --
Not related to this PR, but it's a waste to write a new persisted table here to do the test. We can simply create a view with different column number here:
```scala
sql("CREATE VIEW IF NOT EXISTS testView AS SELECT id AS a, id AS b FROM jt2")
```
---
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: [SPARK-6339][SQL][WIP] Supports create CREATE ...
Posted by clockfly <gi...@git.apache.org>.
Github user clockfly commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r61987856
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
@@ -71,27 +81,69 @@ case class CreateViewCommand(
require(tableDesc.schema == Nil || tableDesc.schema.length == analyzedPlan.output.length)
val sessionState = sparkSession.sessionState
- if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (isTemporary) {
+ createTemporaryView(tableDesc.identifier, sparkSession, analyzedPlan)
+ } else {
+ // Adds default database for permanent table if it doesn't exist, so that tableExists()
+ // only check permanent tables.
+ val database = tableDesc.identifier.database.getOrElse(
+ sessionState.catalog.getCurrentDatabase)
+ val tableIdentifier = tableDesc.identifier.copy(database = Option(database))
+
+ if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (allowExisting) {
--- End diff --
Fixed by disallowing "IF NOT EXISTS" syntax to be consistent with "CREATE TEMPORARY TABLE" behavior.
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r62017251
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala ---
@@ -69,18 +108,71 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
}
}
+ test("correctly parse CREATE TEMPORARY VIEW statement") {
+ withView("testView") {
+ sql(
+ """CREATE TEMPORARY VIEW
+ |testView (c1 COMMENT 'blabla', c2 COMMENT 'blabla')
+ |TBLPROPERTIES ('a' = 'b')
+ |AS SELECT * FROM jt""".stripMargin)
--- End diff --
Nit: For multi-line strings, we usually use the following formats:
```scala
"""First
|Second
|Third
""".stripMargin // Note that this format may end up with unexpected trailing spaces
"""First
|Second
|Third
|""".stripMargin
```
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216851229
**[Test build #57756 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57756/consoleFull)** for PR 12872 at commit [`85d121c`](https://github.com/apache/spark/commit/85d121cc79e3a17bb5d22eb7000997b4d0f981cc).
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r62031763
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
@@ -71,27 +81,69 @@ case class CreateViewCommand(
require(tableDesc.schema == Nil || tableDesc.schema.length == analyzedPlan.output.length)
val sessionState = sparkSession.sessionState
- if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (isTemporary) {
+ createTemporaryView(tableDesc.identifier, sparkSession, analyzedPlan)
+ } else {
+ // Adds default database for permanent table if it doesn't exist, so that tableExists()
+ // only check permanent tables.
+ val database = tableDesc.identifier.database.getOrElse(
+ sessionState.catalog.getCurrentDatabase)
+ val tableIdentifier = tableDesc.identifier.copy(database = Option(database))
+
+ if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (allowExisting) {
+ // Handles `CREATE VIEW IF NOT EXISTS v0 AS SELECT ...`. Does nothing when the target view
+ // already exists.
+ } else if (replace) {
+ // Handles `CREATE OR REPLACE VIEW v0 AS SELECT ...`
+ sessionState.catalog.alterTable(prepareTable(sparkSession, analyzedPlan))
+ } else {
+ // Handles `CREATE VIEW v0 AS SELECT ...`. Throws exception when the target view already
+ // exists.
+ throw new AnalysisException(s"View $tableIdentifier already exists. " +
+ "If you want to update the view definition, please use ALTER VIEW AS or " +
+ "CREATE OR REPLACE VIEW AS")
+ }
+ } else {
+ // Create the view if it doesn't exist.
+ sessionState.catalog.createTable(
+ prepareTable(sparkSession, analyzedPlan), ignoreIfExists = false)
+ }
+ }
+ Seq.empty[Row]
+ }
+
+ private def createTemporaryView(table: TableIdentifier, sparkSession: SparkSession,
+ analyzedPlan: LogicalPlan): Unit = {
+
+ val sessionState = sparkSession.sessionState
+ val catalog = sessionState.catalog
+
+ // Projects column names to alias names
+ val logicalPlan = {
--- End diff --
Nvm, actually it is.
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r62016314
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala ---
@@ -37,11 +37,21 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
sqlContext.sql(s"DROP TABLE IF EXISTS jt")
}
- test("nested views") {
- withView("jtv1", "jtv2") {
+ test("nested views (interleaved with temporary views)") {
+ withView("jtv1", "jtv2", "jtv3", "temp_jtv1", "temp_jtv2", "temp_jtv3") {
--- End diff --
Does `DROP VIEW` handles temporary view now? I guess not yet? If you check definition of `withView`, you may see that it uses `DROP VIEW` to clean up testing views. If `DROP VIEW` doesn't handle temporary views, `temp_jtv1` etc. won't be cleaned up. Currently you can make sure they are dropped using `withTempTable` (which is quite confusing, since temporary table and temporary view are really the same thing for now):
```scala
withView("jtv1", "jtv2", "jtv3") {
withTempTable("temp_jtv1", "temp_jtv2", "temp_jtv3") {
// ...
}
}
```
I'd suggest to make sure that `DROP VIEW` also handles temporary view in a follow-up PR. (Or maybe a dedicated `DROP TEMPORARY VIEW` DDL command?) Also, we should add a `withTempView` testing utility method in `SQLTestUtils`.
---
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: [SPARK-6339][SQL][WIP] Supports create CREATE ...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r61952907
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
@@ -55,13 +60,18 @@ case class CreateViewCommand(
require(tableDesc.tableType == CatalogTableType.VIEW)
require(tableDesc.viewText.isDefined)
- private val tableIdentifier = tableDesc.identifier
-
if (allowExisting && replace) {
throw new AnalysisException(
"It is not allowed to define a view with both IF NOT EXISTS and OR REPLACE.")
}
+ // 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(
--- End diff --
Where does this semantic rule come from? Hive?
---
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: [SPARK-6339][SQL][WIP] Supports create CREATE ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216718015
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: [SPARK-6339][SQL][WIP] Supports create CREATE ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216731693
**[Test build #57707 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57707/consoleFull)** for PR 12872 at commit [`a497cf3`](https://github.com/apache/spark/commit/a497cf3894d7ef6d39fc370665a7767e4cc2c76c).
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216748483
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216748405
**[Test build #57710 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57710/consoleFull)** for PR 12872 at commit [`1e20bb0`](https://github.com/apache/spark/commit/1e20bb0df84ffcfe173c234375f8eb0eb7662242).
* 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 pull request: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by clockfly <gi...@git.apache.org>.
Github user clockfly commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r62031532
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
@@ -71,27 +81,69 @@ case class CreateViewCommand(
require(tableDesc.schema == Nil || tableDesc.schema.length == analyzedPlan.output.length)
val sessionState = sparkSession.sessionState
- if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (isTemporary) {
+ createTemporaryView(tableDesc.identifier, sparkSession, analyzedPlan)
+ } else {
+ // Adds default database for permanent table if it doesn't exist, so that tableExists()
+ // only check permanent tables.
+ val database = tableDesc.identifier.database.getOrElse(
+ sessionState.catalog.getCurrentDatabase)
+ val tableIdentifier = tableDesc.identifier.copy(database = Option(database))
+
+ if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (allowExisting) {
+ // Handles `CREATE VIEW IF NOT EXISTS v0 AS SELECT ...`. Does nothing when the target view
+ // already exists.
+ } else if (replace) {
+ // Handles `CREATE OR REPLACE VIEW v0 AS SELECT ...`
+ sessionState.catalog.alterTable(prepareTable(sparkSession, analyzedPlan))
+ } else {
+ // Handles `CREATE VIEW v0 AS SELECT ...`. Throws exception when the target view already
+ // exists.
+ throw new AnalysisException(s"View $tableIdentifier already exists. " +
+ "If you want to update the view definition, please use ALTER VIEW AS or " +
+ "CREATE OR REPLACE VIEW AS")
+ }
+ } else {
+ // Create the view if it doesn't exist.
+ sessionState.catalog.createTable(
+ prepareTable(sparkSession, analyzedPlan), ignoreIfExists = false)
+ }
+ }
+ Seq.empty[Row]
+ }
+
+ private def createTemporaryView(table: TableIdentifier, sparkSession: SparkSession,
+ analyzedPlan: LogicalPlan): Unit = {
+
+ val sessionState = sparkSession.sessionState
+ val catalog = sessionState.catalog
+
+ // Projects column names to alias names
+ val logicalPlan = {
--- End diff --
It is covered by UT `correctly parse CREATE TEMPORARY VIEW statement`
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216852042
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 pull request: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r62029815
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala ---
@@ -37,11 +37,21 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
sqlContext.sql(s"DROP TABLE IF EXISTS jt")
}
- test("nested views") {
- withView("jtv1", "jtv2") {
+ test("nested views (interleaved with temporary views)") {
+ withView("jtv1", "jtv2", "jtv3", "temp_jtv1", "temp_jtv2", "temp_jtv3") {
--- End diff --
@clockfly had helped verify that `DROP VIEW` actually handles temporary table, because internally `DROP VIEW` is translated to `DropTable`, which handles both persistent and temporary table, and temporary table is just temporary view. (OK, this is really confusing 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 pull request: [SPARK-6339][SQL][WIP] Supports create CREATE ...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216703452
add to whitelist
---
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: [SPARK-6339][SQL][WIP] Supports create CREATE ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216718016
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57693/
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: [SPARK-6339][SQL] Supports CREATE TEMPORARY VI...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r62136118
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
@@ -71,29 +87,59 @@ case class CreateViewCommand(
require(tableDesc.schema == Nil || tableDesc.schema.length == analyzedPlan.output.length)
val sessionState = sparkSession.sessionState
- if (sessionState.catalog.tableExists(tableIdentifier)) {
- if (allowExisting) {
- // Handles `CREATE VIEW IF NOT EXISTS v0 AS SELECT ...`. Does nothing when the target view
- // already exists.
- } else if (replace) {
- // Handles `CREATE OR REPLACE VIEW v0 AS SELECT ...`
- sessionState.catalog.alterTable(prepareTable(sparkSession, analyzedPlan))
+ if (isTemporary) {
+ createTemporaryView(tableDesc.identifier, sparkSession, analyzedPlan)
+ } else {
+ // Adds default database for permanent table if it doesn't exist, so that tableExists()
+ // only check permanent tables.
+ val database = tableDesc.identifier.database.getOrElse(
+ sessionState.catalog.getCurrentDatabase)
+ val tableIdentifier = tableDesc.identifier.copy(database = Option(database))
+
+ if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (allowExisting) {
+ // Handles `CREATE VIEW IF NOT EXISTS v0 AS SELECT ...`. Does nothing when the target view
+ // already exists.
+ } else if (replace) {
+ // Handles `CREATE OR REPLACE VIEW v0 AS SELECT ...`
+ sessionState.catalog.alterTable(prepareTable(sparkSession, analyzedPlan))
+ } else {
+ // Handles `CREATE VIEW v0 AS SELECT ...`. Throws exception when the target view already
+ // exists.
+ throw new AnalysisException(
+ s"View $tableIdentifier already exists. If you want to update the view definition, " +
+ "please use ALTER VIEW AS or CREATE OR REPLACE VIEW AS")
+ }
} else {
- // Handles `CREATE VIEW v0 AS SELECT ...`. Throws exception when the target view already
- // exists.
- throw new AnalysisException(s"View $tableIdentifier already exists. " +
- "If you want to update the view definition, please use ALTER VIEW AS or " +
- "CREATE OR REPLACE VIEW AS")
+ // Create the view if it doesn't exist.
+ sessionState.catalog.createTable(
+ prepareTable(sparkSession, analyzedPlan), ignoreIfExists = false)
}
- } else {
- // Create the view if it doesn't exist.
- sessionState.catalog.createTable(
- prepareTable(sparkSession, analyzedPlan), ignoreIfExists = false)
}
-
Seq.empty[Row]
}
+ private def createTemporaryView(
+ table: TableIdentifier, sparkSession: SparkSession, analyzedPlan: LogicalPlan): Unit = {
+
+ val sessionState = sparkSession.sessionState
+ val catalog = sessionState.catalog
+
+ // Projects column names to alias names
+ val logicalPlan = {
+ if (tableDesc.schema.isEmpty) {
+ analyzedPlan
+ } else {
+ val projectList = analyzedPlan.output.zip(tableDesc.schema).map {
--- End diff --
nvm
---
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: [SPARK-6339][SQL][WIP] Supports create CREATE ...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r61987105
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
@@ -37,13 +37,18 @@ import org.apache.spark.sql.catalyst.plans.logical.{LogicalPlan, Project}
* already exists, throws analysis exception.
* @param replace if true, and if the view already exists, updates it; if false, and if the view
* already exists, throws analysis exception.
+ * @param isTemporary if true, the view is created as a temporary view. Temporary views are dropped
+ * at the end of current Spark session. Existing permanent relations with the same
--- End diff --
Nit: Indentation is off
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216737746
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 pull request: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r62016689
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala ---
@@ -57,6 +67,35 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
}
}
+ test("error handling: fail if the temp view name contains the database prefix") {
+ // Full qualified table name is not allowed
--- End diff --
Nit: "Fully" qualified.
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r62016658
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala ---
@@ -57,6 +67,35 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
}
}
+ test("error handling: fail if the temp view name contains the database prefix") {
+ // Full qualified table name is not allowed
+ val e = intercept[AnalysisException] {
+ sql("CREATE OR REPLACE TEMPORARY VIEW default.myabcdview AS SELECT * FROM jt").collect()
+ }
+ assert(e.message.contains("It is not allowed to add database prefix"))
+ }
+
+ test("error handling: disallow IF NOT EXISTS for CREATE TEMPORARY VIEW") {
+ // Full qualified table name is not allowed
--- End diff --
Wrong comment?
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by clockfly <gi...@git.apache.org>.
Github user clockfly commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216576674
work in progress.
---
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: [SPARK-6339][SQL][WIP] Supports create CREATE ...
Posted by clockfly <gi...@git.apache.org>.
Github user clockfly commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r61984365
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
@@ -71,27 +81,69 @@ case class CreateViewCommand(
require(tableDesc.schema == Nil || tableDesc.schema.length == analyzedPlan.output.length)
val sessionState = sparkSession.sessionState
- if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (isTemporary) {
+ createTemporaryView(tableDesc.identifier, sparkSession, analyzedPlan)
+ } else {
+ // Adds default database for permanent table if it doesn't exist, so that tableExists()
+ // only check permanent tables.
+ val database = tableDesc.identifier.database.getOrElse(
+ sessionState.catalog.getCurrentDatabase)
+ val tableIdentifier = tableDesc.identifier.copy(database = Option(database))
+
+ if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (allowExisting) {
+ // Handles `CREATE VIEW IF NOT EXISTS v0 AS SELECT ...`. Does nothing when the target view
+ // already exists.
+ } else if (replace) {
+ // Handles `CREATE OR REPLACE VIEW v0 AS SELECT ...`
+ sessionState.catalog.alterTable(prepareTable(sparkSession, analyzedPlan))
+ } else {
+ // Handles `CREATE VIEW v0 AS SELECT ...`. Throws exception when the target view already
+ // exists.
+ throw new AnalysisException(s"View $tableIdentifier already exists. " +
+ "If you want to update the view definition, please use ALTER VIEW AS or " +
+ "CREATE OR REPLACE VIEW AS")
+ }
+ } else {
+ // Create the view if it doesn't exist.
+ sessionState.catalog.createTable(
+ prepareTable(sparkSession, analyzedPlan), ignoreIfExists = false)
+ }
+ }
+ Seq.empty[Row]
+ }
+
+ private def createTemporaryView(table: TableIdentifier, sparkSession: SparkSession,
+ analyzedPlan: LogicalPlan): Unit = {
+
+ val sessionState = sparkSession.sessionState
+ val catalog = sessionState.catalog
+
+ // Projects column names to alias names
+ val logicalPlan = {
--- End diff --
Yes, we can project the columns like this:
```
spark.range(1, 10).selectExpr("id", "id id1").write.format("json").saveAsTable("jt")
spark.sql("select * from jt").show()
spark.sql("CREATE TEMPORARY VIEW testView (c1 COMMENT 'column 1', c2 COMMENT 'colum 2') AS SELECT * FROM jt")
spark.sql("select * from testView").show()
```
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by clockfly <gi...@git.apache.org>.
Github user clockfly commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r62026708
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala ---
@@ -69,18 +108,71 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
}
}
+ test("correctly parse CREATE TEMPORARY VIEW statement") {
+ withView("testView") {
+ sql(
+ """CREATE TEMPORARY VIEW
+ |testView (c1 COMMENT 'blabla', c2 COMMENT 'blabla')
+ |TBLPROPERTIES ('a' = 'b')
+ |AS SELECT * FROM jt""".stripMargin)
+ checkAnswer(sql("SELECT c1, c2 FROM testView ORDER BY c1"), (1 to 9).map(i => Row(i, i)))
+ }
+ }
+
+ test("should NOT allow CREATE TEMPORARY VIEW when TEMPORARY VIEW with same name exists") {
+ withView("testView") {
+ sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt")
+
+ val e = intercept[AnalysisException] {
+ sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt").collect()
+ }
+
+ assert(e.message.contains("Temporary table") && e.message.contains("already exists"))
+ }
+ }
+
+ test("should allow CREATE TEMPORARY VIEW when a permanent VIEW with same name exists") {
+ withView("testView", "default.testView") {
+ sql("CREATE VIEW testView AS SELECT id FROM jt")
+ sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt").collect()
+ }
+ }
+
+ test("should allow CREATE permanent VIEW when a TEMPORARY VIEW with same name exists") {
+ withView("testView", "default.testView") {
+ sql("CREATE TEMPORARY VIEW testView AS SELECT id FROM jt")
+ sql("CREATE VIEW testView AS SELECT id FROM jt").collect()
+ }
+ }
+
test("correctly handle CREATE VIEW IF NOT EXISTS") {
withSQLConf(SQLConf.NATIVE_VIEW.key -> "true") {
withTable("jt2") {
- sql("CREATE VIEW testView AS SELECT id FROM jt")
+ withView("testView") {
+ sql("CREATE VIEW testView AS SELECT id FROM jt")
- val df = (1 until 10).map(i => i -> i).toDF("i", "j")
- df.write.format("json").saveAsTable("jt2")
- sql("CREATE VIEW IF NOT EXISTS testView AS SELECT * FROM jt2")
+ val df = (1 until 10).map(i => i -> i).toDF("i", "j")
+ df.write.format("json").saveAsTable("jt2")
+ sql("CREATE VIEW IF NOT EXISTS testView AS SELECT * FROM jt2")
+
+ // make sure our view doesn't change.
+ checkAnswer(sql("SELECT * FROM testView ORDER BY id"), (1 to 9).map(i => Row(i)))
+ }
+ }
+ }
+ }
- // make sure our view doesn't change.
+ test(s"correctly handle CREATE OR REPLACE TEMPORARY VIEW") {
+ withTable("jt2") {
+ withView("testView") {
+ sql("CREATE OR REPLACE TEMPORARY VIEW testView AS SELECT id FROM jt")
checkAnswer(sql("SELECT * FROM testView ORDER BY id"), (1 to 9).map(i => Row(i)))
- sql("DROP VIEW testView")
+
+ val df = (1 until 10).map(i => i -> i).toDF("i", "j")
+ df.write.format("json").saveAsTable("jt2")
--- End diff --
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)))
```
---
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 #12872: [SPARK-6339][SQL] Supports CREATE TEMPORARY VIEW tableId...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the issue:
https://github.com/apache/spark/pull/12872
Can you be more specific on the inconsistency? Seems `ALTER VIEW view_name` is the only inconsistent command?
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216748484
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/57710/
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: [SPARK-6339][SQL][WIP] Supports create CREATE ...
Posted by clockfly <gi...@git.apache.org>.
Github user clockfly commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r61986232
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
@@ -55,13 +60,18 @@ case class CreateViewCommand(
require(tableDesc.tableType == CatalogTableType.VIEW)
require(tableDesc.viewText.isDefined)
- private val tableIdentifier = tableDesc.identifier
-
if (allowExisting && replace) {
throw new AnalysisException(
"It is not allowed to define a view with both IF NOT EXISTS and OR REPLACE.")
}
+ // 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(
--- End diff --
This is to be consistent with DataSet API. When registering a temp table, it will remove the database prefix, here is the code:
https://github.com/apache/spark/blob/6ba17cd147277a20a7fbb244c040e694de486c36/sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala#L533
---
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: [SPARK-6339][SQL][WIP] Supports create CREATE ...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r61952975
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
@@ -71,27 +81,69 @@ case class CreateViewCommand(
require(tableDesc.schema == Nil || tableDesc.schema.length == analyzedPlan.output.length)
val sessionState = sparkSession.sessionState
- if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (isTemporary) {
+ createTemporaryView(tableDesc.identifier, sparkSession, analyzedPlan)
+ } else {
+ // Adds default database for permanent table if it doesn't exist, so that tableExists()
+ // only check permanent tables.
+ val database = tableDesc.identifier.database.getOrElse(
+ sessionState.catalog.getCurrentDatabase)
+ val tableIdentifier = tableDesc.identifier.copy(database = Option(database))
+
+ if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (allowExisting) {
+ // Handles `CREATE VIEW IF NOT EXISTS v0 AS SELECT ...`. Does nothing when the target view
+ // already exists.
+ } else if (replace) {
+ // Handles `CREATE OR REPLACE VIEW v0 AS SELECT ...`
+ sessionState.catalog.alterTable(prepareTable(sparkSession, analyzedPlan))
+ } else {
+ // Handles `CREATE VIEW v0 AS SELECT ...`. Throws exception when the target view already
+ // exists.
+ throw new AnalysisException(s"View $tableIdentifier already exists. " +
+ "If you want to update the view definition, please use ALTER VIEW AS or " +
+ "CREATE OR REPLACE VIEW AS")
+ }
+ } else {
+ // Create the view if it doesn't exist.
+ sessionState.catalog.createTable(
+ prepareTable(sparkSession, analyzedPlan), ignoreIfExists = false)
+ }
+ }
+ Seq.empty[Row]
+ }
+
+ private def createTemporaryView(table: TableIdentifier, sparkSession: SparkSession,
--- End diff --
Style
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216579404
@clockfly for work in progress pr, put WIP in the title.
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r62018292
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
@@ -71,27 +81,69 @@ case class CreateViewCommand(
require(tableDesc.schema == Nil || tableDesc.schema.length == analyzedPlan.output.length)
val sessionState = sparkSession.sessionState
- if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (isTemporary) {
+ createTemporaryView(tableDesc.identifier, sparkSession, analyzedPlan)
+ } else {
+ // Adds default database for permanent table if it doesn't exist, so that tableExists()
+ // only check permanent tables.
+ val database = tableDesc.identifier.database.getOrElse(
+ sessionState.catalog.getCurrentDatabase)
+ val tableIdentifier = tableDesc.identifier.copy(database = Option(database))
+
+ if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (allowExisting) {
+ // Handles `CREATE VIEW IF NOT EXISTS v0 AS SELECT ...`. Does nothing when the target view
+ // already exists.
+ } else if (replace) {
+ // Handles `CREATE OR REPLACE VIEW v0 AS SELECT ...`
+ sessionState.catalog.alterTable(prepareTable(sparkSession, analyzedPlan))
+ } else {
+ // Handles `CREATE VIEW v0 AS SELECT ...`. Throws exception when the target view already
+ // exists.
+ throw new AnalysisException(s"View $tableIdentifier already exists. " +
+ "If you want to update the view definition, please use ALTER VIEW AS or " +
+ "CREATE OR REPLACE VIEW AS")
+ }
+ } else {
+ // Create the view if it doesn't exist.
+ sessionState.catalog.createTable(
+ prepareTable(sparkSession, analyzedPlan), ignoreIfExists = false)
+ }
+ }
+ Seq.empty[Row]
+ }
+
+ private def createTemporaryView(table: TableIdentifier, sparkSession: SparkSession,
+ analyzedPlan: LogicalPlan): Unit = {
+
+ val sessionState = sparkSession.sessionState
+ val catalog = sessionState.catalog
+
+ // Projects column names to alias names
+ val logicalPlan = {
--- End diff --
BTW, seems that this feature is not tested yet?
---
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: [SPARK-6339][SQL] Supports CREATE TEMPORARY VI...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/12872
---
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: [SPARK-6339][SQL][WIP] Supports create CREATE ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216734829
**[Test build #57710 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/57710/consoleFull)** for PR 12872 at commit [`1e20bb0`](https://github.com/apache/spark/commit/1e20bb0df84ffcfe173c234375f8eb0eb7662242).
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/12872#issuecomment-216883910
Would like to ask @yhuai to have a look at this.
---
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: [SPARK-6339][SQL] Supports CREATE TEMPORARY VI...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r62136126
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/views.scala ---
@@ -71,29 +87,59 @@ case class CreateViewCommand(
require(tableDesc.schema == Nil || tableDesc.schema.length == analyzedPlan.output.length)
val sessionState = sparkSession.sessionState
- if (sessionState.catalog.tableExists(tableIdentifier)) {
- if (allowExisting) {
- // Handles `CREATE VIEW IF NOT EXISTS v0 AS SELECT ...`. Does nothing when the target view
- // already exists.
- } else if (replace) {
- // Handles `CREATE OR REPLACE VIEW v0 AS SELECT ...`
- sessionState.catalog.alterTable(prepareTable(sparkSession, analyzedPlan))
+ if (isTemporary) {
+ createTemporaryView(tableDesc.identifier, sparkSession, analyzedPlan)
+ } else {
+ // Adds default database for permanent table if it doesn't exist, so that tableExists()
+ // only check permanent tables.
+ val database = tableDesc.identifier.database.getOrElse(
+ sessionState.catalog.getCurrentDatabase)
+ val tableIdentifier = tableDesc.identifier.copy(database = Option(database))
+
+ if (sessionState.catalog.tableExists(tableIdentifier)) {
+ if (allowExisting) {
+ // Handles `CREATE VIEW IF NOT EXISTS v0 AS SELECT ...`. Does nothing when the target view
+ // already exists.
+ } else if (replace) {
+ // Handles `CREATE OR REPLACE VIEW v0 AS SELECT ...`
+ sessionState.catalog.alterTable(prepareTable(sparkSession, analyzedPlan))
+ } else {
+ // Handles `CREATE VIEW v0 AS SELECT ...`. Throws exception when the target view already
+ // exists.
+ throw new AnalysisException(
+ s"View $tableIdentifier already exists. If you want to update the view definition, " +
+ "please use ALTER VIEW AS or CREATE OR REPLACE VIEW AS")
+ }
} else {
- // Handles `CREATE VIEW v0 AS SELECT ...`. Throws exception when the target view already
- // exists.
- throw new AnalysisException(s"View $tableIdentifier already exists. " +
- "If you want to update the view definition, please use ALTER VIEW AS or " +
- "CREATE OR REPLACE VIEW AS")
+ // Create the view if it doesn't exist.
+ sessionState.catalog.createTable(
+ prepareTable(sparkSession, analyzedPlan), ignoreIfExists = false)
}
- } else {
- // Create the view if it doesn't exist.
- sessionState.catalog.createTable(
- prepareTable(sparkSession, analyzedPlan), ignoreIfExists = false)
}
-
Seq.empty[Row]
}
+ private def createTemporaryView(
+ table: TableIdentifier, sparkSession: SparkSession, analyzedPlan: LogicalPlan): Unit = {
+
+ val sessionState = sparkSession.sessionState
+ val catalog = sessionState.catalog
+
+ // Projects column names to alias names
+ val logicalPlan = {
+ if (tableDesc.schema.isEmpty) {
+ analyzedPlan
+ } else {
+ val projectList = analyzedPlan.output.zip(tableDesc.schema).map {
--- End diff --
we already check the lenght
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r62017488
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala ---
@@ -37,11 +37,21 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
sqlContext.sql(s"DROP TABLE IF EXISTS jt")
}
- test("nested views") {
- withView("jtv1", "jtv2") {
+ test("nested views (interleaved with temporary views)") {
+ withView("jtv1", "jtv2", "jtv3", "temp_jtv1", "temp_jtv2", "temp_jtv3") {
--- End diff --
(This applies to all test cases modified/introduced in this PR.)
---
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: [SPARK-6339][SQL] Supports create CREATE TEMPO...
Posted by rxin <gi...@git.apache.org>.
Github user rxin commented on a diff in the pull request:
https://github.com/apache/spark/pull/12872#discussion_r61908892
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/SQLViewSuite.scala ---
@@ -69,18 +100,105 @@ class SQLViewSuite extends QueryTest with SQLTestUtils with TestHiveSingleton {
}
}
+ test("correctly parse CREATE TEMPORARY VIEW statement") {
+ withSQLConf(SQLConf.NATIVE_VIEW.key -> "true") {
--- End diff --
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?
---
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