You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by gatorsmile <gi...@git.apache.org> on 2016/04/10 08:19:18 UTC
[GitHub] spark pull request: [SPARK-14362][SPARK-14406][SQL][Follow-up] DDL...
GitHub user gatorsmile opened a pull request:
https://github.com/apache/spark/pull/12284
[SPARK-14362][SPARK-14406][SQL][Follow-up] DDL Native Support: Drop View and Drop Table
#### What changes were proposed in this pull request?
This PR is to remove the function `isViewSupported` from `SessionCatalog`. After the removal, we still can capture the user errors if users try to drop a table using `DROP VIEW`.
#### How was this patch tested?
Modified the existing test cases
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/gatorsmile/spark followupDropTable
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/12284.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 #12284
----
commit 434e077683a22cb35f87009a9d486bbfc9fe4159
Author: gatorsmile <ga...@gmail.com>
Date: 2016-04-10T06:08:16Z
remove isViewSupport
commit c2627251dabb269a7ffffa727c73d400930b97b8
Author: gatorsmile <ga...@gmail.com>
Date: 2016-04-10T06:12:10Z
update the comment
commit 0e11331a6715d02c4dd167abf6bdde365e2c044a
Author: gatorsmile <ga...@gmail.com>
Date: 2016-04-10T06:15:45Z
remove an empty line
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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-14362][SPARK-14406][SQL][Follow-up] DDL...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12284#issuecomment-208058417
**[Test build #55489 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55489/consoleFull)** for PR 12284 at commit [`8296bc6`](https://github.com/apache/spark/commit/8296bc6cf9bfa76102345b607f401737c29a05fb).
---
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-14362][SPARK-14406][SQL][Follow-up] DDL...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12284#issuecomment-208131833
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55495/
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-14362][SPARK-14406][SQL][Follow-up] DDL...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12284#discussion_r59139028
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -191,34 +191,40 @@ case class DropTable(
override def run(sqlContext: SQLContext): Seq[Row] = {
val catalog = sqlContext.sessionState.catalog
- if (isView && !catalog.isViewSupported) {
- throw new AnalysisException(s"Not supported object: views")
- }
- // If the command DROP VIEW is to drop a table or DROP TABLE is to drop a view
- // issue an exception.
- catalog.getTableMetadataOption(tableName).map(_.tableType match {
- case CatalogTableType.VIRTUAL_VIEW if !isView =>
- throw new AnalysisException(
- "Cannot drop a view with DROP TABLE. Please use DROP VIEW instead")
- case o if o != CatalogTableType.VIRTUAL_VIEW && isView =>
- throw new AnalysisException(
- s"Cannot drop a table with DROP VIEW. Please use DROP TABLE instead")
- case _ =>
- })
-
- try {
- sqlContext.cacheManager.tryUncacheQuery(sqlContext.table(tableName.quotedString))
- } catch {
- // This table's metadata is not in Hive metastore (e.g. the table does not exist).
- case e if e.getClass.getName == "org.apache.hadoop.hive.ql.metadata.InvalidTableException" =>
- case _: org.apache.spark.sql.catalyst.analysis.NoSuchTableException =>
- // Other Throwables can be caused by users providing wrong parameters in OPTIONS
- // (e.g. invalid paths). We catch it and log a warning message.
- // Users should be able to drop such kinds of tables regardless if there is an error.
- case e: Throwable => log.warn(s"${e.getMessage}", e)
+ if (!catalog.tableExists(tableName)) {
+ if (!ifExists) {
+ // When ifExists is false, no exception is issued when the table does not exist.
+ // Instead, log it as an error message.
+ val objectName = if (isView) "View" else "Table"
+ logError(s"$objectName '${tableName.quotedString}' does not exist")
--- End diff --
Sure, we might need to change a couple of messages for it. Let me try it. Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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-14362][SPARK-14406][SQL][Follow-up] DDL...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12284#issuecomment-208081165
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55489/
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-14362][SPARK-14406][SQL][Follow-up] DDL...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12284#issuecomment-208131830
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-14362][SPARK-14406][SQL][Follow-up] DDL...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12284#issuecomment-207943795
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-14362][SPARK-14406][SQL][Follow-up] DDL...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12284#issuecomment-208081163
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-14362][SPARK-14406][SQL][Follow-up] DDL...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:
https://github.com/apache/spark/pull/12284#issuecomment-207994961
cc @yhuai @andrewor14 Thanks!
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at 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-14362][SPARK-14406][SQL][Follow-up] DDL...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12284#issuecomment-208080895
**[Test build #55489 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55489/consoleFull)** for PR 12284 at commit [`8296bc6`](https://github.com/apache/spark/commit/8296bc6cf9bfa76102345b607f401737c29a05fb).
* This patch **fails SparkR 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-14362][SPARK-14406][SQL][Follow-up] DDL...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12284#issuecomment-207943698
**[Test build #55470 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55470/consoleFull)** for PR 12284 at commit [`0e11331`](https://github.com/apache/spark/commit/0e11331a6715d02c4dd167abf6bdde365e2c044a).
* 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-14362][SPARK-14406][SQL][Follow-up] DDL...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12284#discussion_r59138655
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -191,34 +191,40 @@ case class DropTable(
override def run(sqlContext: SQLContext): Seq[Row] = {
val catalog = sqlContext.sessionState.catalog
- if (isView && !catalog.isViewSupported) {
- throw new AnalysisException(s"Not supported object: views")
- }
- // If the command DROP VIEW is to drop a table or DROP TABLE is to drop a view
- // issue an exception.
- catalog.getTableMetadataOption(tableName).map(_.tableType match {
- case CatalogTableType.VIRTUAL_VIEW if !isView =>
- throw new AnalysisException(
- "Cannot drop a view with DROP TABLE. Please use DROP VIEW instead")
- case o if o != CatalogTableType.VIRTUAL_VIEW && isView =>
- throw new AnalysisException(
- s"Cannot drop a table with DROP VIEW. Please use DROP TABLE instead")
- case _ =>
- })
-
- try {
- sqlContext.cacheManager.tryUncacheQuery(sqlContext.table(tableName.quotedString))
- } catch {
- // This table's metadata is not in Hive metastore (e.g. the table does not exist).
- case e if e.getClass.getName == "org.apache.hadoop.hive.ql.metadata.InvalidTableException" =>
- case _: org.apache.spark.sql.catalyst.analysis.NoSuchTableException =>
- // Other Throwables can be caused by users providing wrong parameters in OPTIONS
- // (e.g. invalid paths). We catch it and log a warning message.
- // Users should be able to drop such kinds of tables regardless if there is an error.
- case e: Throwable => log.warn(s"${e.getMessage}", e)
+ if (!catalog.tableExists(tableName)) {
+ if (!ifExists) {
+ // When ifExists is false, no exception is issued when the table does not exist.
+ // Instead, log it as an error message.
+ val objectName = if (isView) "View" else "Table"
+ logError(s"$objectName '${tableName.quotedString}' does not exist")
--- End diff --
Should we just let SessionCatalog to log something like `Table or View ${name.quotedString} does not exist`?
---
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-14362][SPARK-14406][SQL][Follow-up] DDL...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12284#issuecomment-208131432
**[Test build #55495 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55495/consoleFull)** for PR 12284 at commit [`9271a03`](https://github.com/apache/spark/commit/9271a03ccaf2f86db5e543ac6f87ba7f541c4f7d).
* 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-14362][SPARK-14406][SQL][Follow-up] DDL...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12284#discussion_r59128029
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -191,34 +191,40 @@ case class DropTable(
override def run(sqlContext: SQLContext): Seq[Row] = {
val catalog = sqlContext.sessionState.catalog
- if (isView && !catalog.isViewSupported) {
- throw new AnalysisException(s"Not supported object: views")
- }
- // If the command DROP VIEW is to drop a table or DROP TABLE is to drop a view
- // issue an exception.
- catalog.getTableMetadataOption(tableName).map(_.tableType match {
- case CatalogTableType.VIRTUAL_VIEW if !isView =>
- throw new AnalysisException(
- "Cannot drop a view with DROP TABLE. Please use DROP VIEW instead")
- case o if o != CatalogTableType.VIRTUAL_VIEW && isView =>
- throw new AnalysisException(
- s"Cannot drop a table with DROP VIEW. Please use DROP TABLE instead")
- case _ =>
- })
-
- try {
- sqlContext.cacheManager.tryUncacheQuery(sqlContext.table(tableName.quotedString))
- } catch {
- // This table's metadata is not in Hive metastore (e.g. the table does not exist).
- case e if e.getClass.getName == "org.apache.hadoop.hive.ql.metadata.InvalidTableException" =>
- case _: org.apache.spark.sql.catalyst.analysis.NoSuchTableException =>
- // Other Throwables can be caused by users providing wrong parameters in OPTIONS
- // (e.g. invalid paths). We catch it and log a warning message.
- // Users should be able to drop such kinds of tables regardless if there is an error.
- case e: Throwable => log.warn(s"${e.getMessage}", e)
+ if (!catalog.tableExists(tableName)) {
+ if (!ifExists) {
+ // When ifExists is false, no exception is issued when the table does not exist.
+ // Instead, log it as an error message.
+ val objectName = if (isView) "View" else "Table"
+ logError(s"$objectName '${tableName.quotedString}' does not exist")
--- End diff --
Why we need to check `tableExists` at the beginning?
Here, the reason is to avoid issuing a confusing message. Without the change, users might get a confusing error message `table 'abc' does not exist`, although they issue a command `DROP 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-14362][SPARK-14406][SQL][Follow-up] DDL...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12284#issuecomment-208106314
**[Test build #55495 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55495/consoleFull)** for PR 12284 at commit [`9271a03`](https://github.com/apache/spark/commit/9271a03ccaf2f86db5e543ac6f87ba7f541c4f7d).
---
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-14362][SPARK-14406][SQL][Follow-up] DDL...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:
https://github.com/apache/spark/pull/12284#issuecomment-208148952
Thanks. Merging to master.
---
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-14362][SPARK-14406][SQL][Follow-up] DDL...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/12284
---
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-14362][SPARK-14406][SQL][Follow-up] DDL...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12284#issuecomment-207943796
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55470/
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-14362][SPARK-14406][SQL][Follow-up] DDL...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12284#issuecomment-207934579
**[Test build #55470 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55470/consoleFull)** for PR 12284 at commit [`0e11331`](https://github.com/apache/spark/commit/0e11331a6715d02c4dd167abf6bdde365e2c044a).
---
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