You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by sureshthalamati <gi...@git.apache.org> on 2016/05/25 21:45:54 UTC
[GitHub] spark pull request: [SPARK-15538][SQL] Adding error check for trun...
GitHub user sureshthalamati opened a pull request:
https://github.com/apache/spark/pull/13305
[SPARK-15538][SQL] Adding error check for truncate table on a datasource table or view.
## What changes were proposed in this pull request?
Adds check to throw error when user attempts truncate table on data source table or view. Currently
TRUNCATE TABLE returns success when user attempts truncate table on data source table or view. But the table is not truncated.
## How was this patch tested?
Added unit test cases.
@hvanhovell
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/sureshthalamati/spark truncate_not_allowed-SPARK-15538
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/13305.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 #13305
----
commit fd2bf48b3805967b1908ee594044ecea7866a3ce
Author: sureshthalamati <su...@gmail.com>
Date: 2016-05-25T21:19:48Z
Adding not allowed error check for truncate table on datasource table.
----
---
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-15538][SQL] Adding error check for trun...
Posted by sureshthalamati <gi...@git.apache.org>.
Github user sureshthalamati closed the pull request at:
https://github.com/apache/spark/pull/13305
---
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-15538][SQL] Adding error check for trun...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/13305#issuecomment-221750415
@sureshthalamati I've taken over this patch at #13315 with a different solution. Thanks for working on 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-15538][SQL] Adding error check for trun...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/13305#issuecomment-221724081
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-15538][SQL] Adding error check for trun...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/13305#discussion_r64662860
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
@@ -292,10 +292,18 @@ case class TruncateTableCommand(
} else if (catalog.isTemporaryTable(tableName)) {
logError(s"table '$tableName' in TRUNCATE TABLE is a temporary table.")
} else {
+ val table = catalog.getTableMetadata(tableName)
+ if (table.tableType == VIEW ) {
+ throw new AnalysisException(
+ s"TRUNCATE TABLE is not allowed on a view: ${table.qualifiedName}")
--- End diff --
with that format you can just use `assertUnsupported` in `DDLSuite` when you add a test for each of these cases
---
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-15538][SQL] Adding error check for trun...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/13305#issuecomment-221735811
```
scala> sql("CREATE TABLE boxes (length INT, height INT, width INT) USING parquet")
scala> (1 to 3).map { i => (i, i * 2, i * 3) }.toDF("height", "length", "width").write.insertInto("boxes")
scala> spark.table("boxes").show()
+------+------+-----+
|length|height|width|
+------+------+-----+
| 1| 2| 3|
| 2| 4| 6|
| 3| 6| 9|
+------+------+-----+
scala> sql("TRUNCATE TABLE boxes")
scala> spark.catalog.refreshTable("boxes") // necessary for now, but that's a bug
scala> spark.table("boxes").show()
+------+------+-----+
|length|height|width|
+------+------+-----+
+------+------+-----+
```
---
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-15538][SQL] Adding error check for trun...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the pull request:
https://github.com/apache/spark/pull/13305#issuecomment-221719109
@sureshthalamati I think https://github.com/apache/spark/pull/13302 already takes care of 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-15538][SQL] Adding error check for trun...
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the pull request:
https://github.com/apache/spark/pull/13305#issuecomment-221724543
@sureshthalamati Yeah you are right. Sorry for jumping the gun.
---
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-15538][SQL] Adding error check for trun...
Posted by sureshthalamati <gi...@git.apache.org>.
Github user sureshthalamati commented on the pull request:
https://github.com/apache/spark/pull/13305#issuecomment-221724406
sure. I can add for EXTERNAL table also. Thank you for reviewing the PR, Andrew.
---
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-15538][SQL] Adding error check for trun...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/13305#issuecomment-221735424
Actually I was able to get `TRUNCATE TABLE` to work on data source tables, though I have to add a call to `spark.catalog.refershTable` every time after I call `TRUNCATE TABLE` to update the cached relation.
---
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-15538][SQL] Adding error check for trun...
Posted by sureshthalamati <gi...@git.apache.org>.
Github user sureshthalamati commented on the pull request:
https://github.com/apache/spark/pull/13305#issuecomment-221762895
Thank you Andrew , Herman for your input. I am closing this pull request.
---
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-15538][SQL] Adding error check for trun...
Posted by sureshthalamati <gi...@git.apache.org>.
Github user sureshthalamati commented on the pull request:
https://github.com/apache/spark/pull/13305#issuecomment-221722772
@hvanhovell
#13302 change does not seem to check for truncate table data source table/view. It is throwing exception for table not found. The case I am addressing is specific to data source table.
@andrewor14
---
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-15538][SQL] Adding error check for trun...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/13305#issuecomment-221736590
Actually, it looks like it's a slightly bigger change so we can always do it in a future patch if you prefer.
---
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-15538][SQL] Adding error check for trun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/13305#issuecomment-221718036
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-15538][SQL] Adding error check for trun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/13305#issuecomment-221726235
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-15538][SQL] Adding error check for trun...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/13305#issuecomment-221726236
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/59301/
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-15538][SQL] Adding error check for trun...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/13305#issuecomment-221736081
@sureshthalamati I was able to get it working with your example as well. I think it does work with datasource table. We just need to add a call to `spark.catalog.refreshTable` at the end of `TruncateTableCommand`. Can you update that and add tests for it?
---
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-15538][SQL] Adding error check for trun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/13305#issuecomment-221726211
**[Test build #59301 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59301/consoleFull)** for PR 13305 at commit [`fd2bf48`](https://github.com/apache/spark/commit/fd2bf48b3805967b1908ee594044ecea7866a3ce).
* This patch **fails MiMa 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-15538][SQL] Adding error check for trun...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/13305#issuecomment-221723642
@sureshthalamati can you also add a case for EXTERNAL table? You can also add `[SPARK-15536]` to 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-15538][SQL] Adding error check for trun...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/13305#discussion_r64662387
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
@@ -292,10 +292,18 @@ case class TruncateTableCommand(
} else if (catalog.isTemporaryTable(tableName)) {
logError(s"table '$tableName' in TRUNCATE TABLE is a temporary table.")
} else {
+ val table = catalog.getTableMetadata(tableName)
+ if (table.tableType == VIEW ) {
--- End diff --
no space after 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-15538][SQL] Adding error check for trun...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/13305#discussion_r64662799
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveCommandSuite.scala ---
@@ -345,6 +345,19 @@ class HiveCommandSuite extends QueryTest with SQLTestUtils with TestHiveSingleto
checkAnswer(
sql("SELECT employeeID, employeeName FROM part_table"),
Seq.empty[Row])
+
+ withView("v1") {
+ sql(s"CREATE VIEW v1 AS SELECT * FROM non_part_table")
+ val msg = intercept[AnalysisException] {
+ sql("truncate table v1")
+ }.getMessage
+ assert(msg.contains("is not allowed on a view"))
+ }
+
+ val msg = intercept[AnalysisException] {
+ sql("TRUNCATE TABLE parquet_tab1")
+ }.getMessage
+ assert(msg.contains("is not allowed on a datasource table"))
--- End diff --
let's not add more tests to `HiveCommandSuite`; there are no more hive specific commands so we should add them to `DDLSuite` instead
---
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-15538][SQL] Adding error check for trun...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/13305#issuecomment-221724749
**[Test build #59301 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/59301/consoleFull)** for PR 13305 at commit [`fd2bf48`](https://github.com/apache/spark/commit/fd2bf48b3805967b1908ee594044ecea7866a3ce).
---
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-15538][SQL] Adding error check for trun...
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/13305#discussion_r64662478
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
@@ -292,10 +292,18 @@ case class TruncateTableCommand(
} else if (catalog.isTemporaryTable(tableName)) {
logError(s"table '$tableName' in TRUNCATE TABLE is a temporary table.")
} else {
+ val table = catalog.getTableMetadata(tableName)
+ if (table.tableType == VIEW ) {
+ throw new AnalysisException(
+ s"TRUNCATE TABLE is not allowed on a view: ${table.qualifiedName}")
--- End diff --
can you say `Operation not allowed: TRUNCATE TABLE on a view: $tableName`
---
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