You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2020/10/18 03:43:59 UTC
[GitHub] [spark] imback82 opened a new pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
imback82 opened a new pull request #30079:
URL: https://github.com/apache/spark/pull/30079
<!--
Thanks for sending a pull request! Here are some tips for you:
1. If this is your first time, please read our contributor guidelines: https://spark.apache.org/contributing.html
2. Ensure you have added or run the appropriate tests for your PR: https://spark.apache.org/developer-tools.html
3. If the PR is unfinished, add '[WIP]' in your PR title, e.g., '[WIP][SPARK-XXXX] Your PR title ...'.
4. Be sure to keep the PR description updated to reflect all changes.
5. Please write your PR title to summarize what this PR proposes.
6. If possible, provide a concise example to reproduce the issue for a faster review.
7. If you want to add a new configuration, please read the guideline first for naming configurations in
'core/src/main/scala/org/apache/spark/internal/config/ConfigEntry.scala'.
-->
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
This PR proposes to migrate `DROP TABLE` to use `UnresolvedTableOrView` to resolve the table/view identifier. This allows consistent resolution rules (temp view first, etc.) to be applied for both v1/v2 commands. More info about the consistent resolution rule proposal can be found in [JIRA](https://issues.apache.org/jira/browse/SPARK-29900) or [proposal doc](https://docs.google.com/document/d/1hvLjGA8y_W_hhilpngXVub1Ebv8RsMap986nENCFnrg/edit?usp=sharing).
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
The current behavior is not consistent between v1 and v2 commands when resolving a temp view.
In v2, the `t` in the following example is resolved to a table:
```scala
sql("CREATE TABLE testcat.ns.t (id bigint) USING foo")
sql("CREATE TEMPORARY VIEW t AS SELECT 2")
sql("USE testcat.ns")
sql("DROP TABLE t") // 't' is resolved to testcat.ns.t
```
whereas in v1, the `t` is resolved to a temp view:
```scala
sql("CREATE DATABASE test")
sql("CREATE TABLE spark_catalog.test.t (id bigint) USING csv")
sql("CREATE TEMPORARY VIEW t AS SELECT 2")
sql("USE spark_catalog.test")
sql("DROP TABLE t") // 't' is resolved to a temp view
```
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
After this PR, for v2, `DROP TABLE t` is resolved to a temp view `t` instead of `testcat.ns.t`, consistent with v1 behavior.
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
Added a new test
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711451853
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711126934
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/129952/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711427658
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711126839
**[Test build #129952 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129952/testReport)** for PR 30079 at commit [`468925c`](https://github.com/apache/spark/commit/468925c232040812d452d12e3d3a2cbf28cb8fce).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715026041
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130189/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-717642722
**[Test build #130340 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130340/testReport)** for PR 30079 at commit [`dfc4492`](https://github.com/apache/spark/commit/dfc44923d10fa7cbb5cd6cde4dfcffa19a9e994d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class ResolvedView(identifier: Identifier, isTemp: Boolean) extends LeafNode `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-717643342
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-714893004
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715466722
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34812/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711417660
**[Test build #129969 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129969/testReport)** for PR 30079 at commit [`f23e36d`](https://github.com/apache/spark/commit/f23e36d9aae80f1d491ba9ca61ef85d19cf61fb5).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715477298
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711117211
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715477298
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711425698
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-714883002
**[Test build #130188 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130188/testReport)** for PR 30079 at commit [`432eb9d`](https://github.com/apache/spark/commit/432eb9d11671c4d17e0da0a96ed709527ad3b814).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715434241
**[Test build #130211 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130211/testReport)** for PR 30079 at commit [`b4ea475`](https://github.com/apache/spark/commit/b4ea47566457ae81b928a19fb1598f332550e569).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-714900584
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711443242
**[Test build #129968 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129968/testReport)** for PR 30079 at commit [`01affe4`](https://github.com/apache/spark/commit/01affe462166e28d209a8a94c0ca0337bd68b6a9).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r517786526
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -228,8 +228,11 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
case DescribeColumn(_: ResolvedTable, _, _) =>
throw new AnalysisException("Describing columns is not supported for v2 tables.")
- case DropTable(catalog, ident, ifExists) =>
- DropTableExec(catalog, ident, ifExists) :: Nil
+ case DropTable(r: ResolvedTable, ifExists, _) =>
Review comment:
Silently ignore the `purge` option as the default behavior looks risky. How about making the new variant of `dropTable` fail by default if `purge==true`?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715626874
**[Test build #130214 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130214/testReport)** for PR 30079 at commit [`43bb465`](https://github.com/apache/spark/commit/43bb465dcbda783de4ac1d3e1987e6035a311ad0).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-714905105
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r510593591
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/connector/TestV2SessionCatalogBase.scala
##########
@@ -74,8 +82,11 @@ private[connector] trait TestV2SessionCatalogBase[T <: Table] extends Delegating
}
def clearTables(): Unit = {
- assert(!tables.isEmpty, "Tables were empty, maybe didn't use the session catalog code path?")
+ assert(
+ tableCreated.get,
Review comment:
Why doesn't `!tables.isEmpty` work?
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/connector/TestV2SessionCatalogBase.scala
##########
@@ -74,8 +82,11 @@ private[connector] trait TestV2SessionCatalogBase[T <: Table] extends Delegating
}
def clearTables(): Unit = {
- assert(!tables.isEmpty, "Tables were empty, maybe didn't use the session catalog code path?")
+ assert(
+ tableCreated.get,
Review comment:
Why doesn't `!tables.isEmpty` work?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715579551
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715523169
**[Test build #130214 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130214/testReport)** for PR 30079 at commit [`43bb465`](https://github.com/apache/spark/commit/43bb465dcbda783de4ac1d3e1987e6035a311ad0).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715026016
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715578985
**[Test build #130212 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130212/testReport)** for PR 30079 at commit [`0a87543`](https://github.com/apache/spark/commit/0a8754348117776296bcaca3c9af21319acf633b).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class ResolvedView(identifier: Identifier) extends LeafNode `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r513034303
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -368,9 +368,13 @@ class ResolveSessionCatalog(
orCreate = c.orCreate)
}
+ case DropTable(
+ r @ ResolvedTable(_, _, _: V1Table), ifExists, purge) if isSessionCatalog(r.catalog) =>
+ DropTableCommand(r.identifier.asTableIdentifier, ifExists, isView = false, purge = purge)
+
// v1 DROP TABLE supports temp view.
- case DropTableStatement(TempViewOrV1Table(name), ifExists, purge) =>
- DropTableCommand(name.asTableIdentifier, ifExists, isView = false, purge = purge)
+ case DropTable(r: ResolvedView, ifExists, purge) =>
Review comment:
If we put the assert here, the output call stack doesn't seem to be very helpful to the user:
```
java.lang.AssertionError: assertion failed
at scala.Predef$.assert(Predef.scala:208)
at org.apache.spark.sql.catalyst.analysis.ResolveSessionCatalog$$anonfun$apply$1.applyOrElse(ResolveSessionCatalog.scala:376)
at org.apache.spark.sql.catalyst.analysis.ResolveSessionCatalog$$anonfun$apply$1.applyOrElse(ResolveSessionCatalog.scala:47)
at org.apache.spark.sql.catalyst.plans.logical.AnalysisHelper.$anonfun$resolveOperatorsUp$3(AnalysisHelper.scala:90)
at org.apache.spark.sql.catalyst.trees.CurrentOrigin$.withOrigin(TreeNode.scala:74)
```
, also there is an existing test around it:
https://github.com/apache/spark/blob/3f2a2b5fe6ada37ef86f00737387e6cf2496df74/sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala#L1057
I am throwing an exception instead similar to how `DropTableCommand` is handling. Please let me know what you think about this.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711117209
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34558/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-714883002
**[Test build #130188 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130188/testReport)** for PR 30079 at commit [`432eb9d`](https://github.com/apache/spark/commit/432eb9d11671c4d17e0da0a96ed709527ad3b814).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-714896504
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34791/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r516901187
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -228,8 +228,11 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
case DescribeColumn(_: ResolvedTable, _, _) =>
throw new AnalysisException("Describing columns is not supported for v2 tables.")
- case DropTable(catalog, ident, ifExists) =>
- DropTableExec(catalog, ident, ifExists) :: Nil
+ case DropTable(r: ResolvedTable, ifExists, _) =>
Review comment:
The `purge` option seems that it should be handled by the catalog implementation, not by Spark (e.g., `ifExists`). We can update `TableCatalog.dropTable` to pass an additional flag, or we could introduce `TableCatalog.purgeTable` for the catalog implementor can optionally implement.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-717574278
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34942/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711116669
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/34557/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r507208340
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/connector/TestV2SessionCatalogBase.scala
##########
@@ -74,8 +82,11 @@ private[connector] trait TestV2SessionCatalogBase[T <: Table] extends Delegating
}
def clearTables(): Unit = {
- assert(!tables.isEmpty, "Tables were empty, maybe didn't use the session catalog code path?")
Review comment:
Now that `withTable` works consistently, this log should be updated.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/Analyzer.scala
##########
@@ -1091,6 +1095,19 @@ class Analyzer(
}
}
+ /**
+ * Resolve [[UnresolvedTableOrView]] by replacing it with [[NotFoundTableOrView]]
+ * if resolution of table or view is not required.
+ *
+ * This rule should run after [[ResolveRelations]] and [[ResolveTables]] are run.
Review comment:
I remember we shouldn't have a dependency on other rules. I can put these into one rule if the current approach is not desired.
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/v2ResolutionPlans.scala
##########
@@ -45,12 +45,25 @@ case class UnresolvedTable(multipartIdentifier: Seq[String]) extends LeafNode {
/**
* Holds the name of a table or view that has yet to be looked up in a catalog. It will
* be resolved to [[ResolvedTable]] or [[ResolvedView]] during analysis.
+ *
+ * If 'isResolutionRequired' is set to false and the name cannot be resolved to a table or view,
+ * [[UnresolvedTableOrView]] will be converted to [[NotFoundTableOrView]].
*/
-case class UnresolvedTableOrView(multipartIdentifier: Seq[String]) extends LeafNode {
+case class UnresolvedTableOrView(
+ multipartIdentifier: Seq[String],
+ isResolutionRequired: Boolean = true) extends LeafNode {
override lazy val resolved: Boolean = false
override def output: Seq[Attribute] = Nil
}
+/**
+ * Holds the name of a table or view that has been looked up in a catalog, but not found.
+ * This is a "resolved" logical.
+ */
+case class NotFoundTableOrView(multipartIdentifier: Seq[String]) extends LeafNode {
Review comment:
@cloud-fan `DROP TABLE` is a bit tricky to handle in the new resolution framework because the command has the "ifExists" option. So, even if a table/view cannot be resolved, we shouldn't fail if `ifExists` is set to true. Please let me know if the current approach is reasonable. Thanks in advance!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r507208340
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/connector/TestV2SessionCatalogBase.scala
##########
@@ -74,8 +82,11 @@ private[connector] trait TestV2SessionCatalogBase[T <: Table] extends Delegating
}
def clearTables(): Unit = {
- assert(!tables.isEmpty, "Tables were empty, maybe didn't use the session catalog code path?")
Review comment:
Now that `withTable` works consistently, the logic for checking if table is created should be updated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711417660
**[Test build #129969 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129969/testReport)** for PR 30079 at commit [`f23e36d`](https://github.com/apache/spark/commit/f23e36d9aae80f1d491ba9ca61ef85d19cf61fb5).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r510587530
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveNoopDropTable.scala
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.analysis
+
+import org.apache.spark.sql.catalyst.plans.logical.{DropTable, LogicalPlan, NoopDropTable}
+import org.apache.spark.sql.catalyst.rules.Rule
+
+/**
+ * A rule for handling [[DropTable]] logical plan when the table or temp view is not resolved.
+ * If "ifExists" flag is set to true, the plan is resolved to [[NoopDropTable]],
+ * which is a no-op command. If the flag is set to false, the rule throws [[NoSuchTableException]].
+ */
+object ResolveNoopDropTable extends Rule[LogicalPlan] {
+ import org.apache.spark.sql.connector.catalog.CatalogV2Implicits.MultipartIdentifierHelper
+
+ def apply(plan: LogicalPlan): LogicalPlan = plan.resolveOperatorsUp {
+ case DropTable(u: UnresolvedTableOrView, ifExists, _) =>
+ if (ifExists) {
+ NoopDropTable(u.multipartIdentifier)
+ } else {
+ throw new NoSuchTableException(
Review comment:
We can leave it to the `CheckAnalysis`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715024027
**[Test build #130189 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130189/testReport)** for PR 30079 at commit [`d886b18`](https://github.com/apache/spark/commit/d886b1834da7725c582ca2e8d59769afb85170f8).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r507208852
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/PlanResolutionSuite.scala
##########
@@ -160,6 +160,8 @@ class PlanResolutionSuite extends AnalysisTest {
new ResolveCatalogs(catalogManager),
new ResolveSessionCatalog(catalogManager, conf, _ == Seq("v"), _ => false),
analyzer.ResolveTables,
+ analyzer.ResolveUnresolvedTableOrView,
+ new ResolveSessionCatalog(catalogManager, conf, _ == Seq("v"), _ => false),
Review comment:
Unfortunately, we need to run `ResolveSessionCatalog` again if we want to test the resolution to `DropTableCommand`: https://github.com/apache/spark/pull/30079/files#diff-7ce7e7e03723d865798a6dcb6efa81aea25e925397e4c9288bbfd3317c8a515bR642
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711116664
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34557/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715026016
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-714905105
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711112654
**[Test build #129952 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129952/testReport)** for PR 30079 at commit [`468925c`](https://github.com/apache/spark/commit/468925c232040812d452d12e3d3a2cbf28cb8fce).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715464510
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34811/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r510592895
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -228,8 +228,11 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
case DescribeColumn(_: ResolvedTable, _, _) =>
throw new AnalysisException("Describing columns is not supported for v2 tables.")
- case DropTable(catalog, ident, ifExists) =>
- DropTableExec(catalog, ident, ifExists) :: Nil
+ case DropTable(r: ResolvedTable, ifExists, _) =>
+ DropTableExec(r.catalog, r.identifier, ifExists) :: Nil
+
+ case NoopDropTable(multipartIdentifier) =>
+ NoopDropTableExec(multipartIdentifier) :: Nil
Review comment:
how about `LocalTableScanExec(Nil, Nil)`? It's only for EXPLAIN, so a logical plan is good enough. We don't need to create a real physical plan for it.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715564307
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r511821763
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -368,9 +368,13 @@ class ResolveSessionCatalog(
orCreate = c.orCreate)
}
+ case DropTable(
+ r @ ResolvedTable(_, _, _: V1Table), ifExists, purge) if isSessionCatalog(r.catalog) =>
+ DropTableCommand(r.identifier.asTableIdentifier, ifExists, isView = false, purge = purge)
+
// v1 DROP TABLE supports temp view.
- case DropTableStatement(TempViewOrV1Table(name), ifExists, purge) =>
- DropTableCommand(name.asTableIdentifier, ifExists, isView = false, purge = purge)
+ case DropTable(r: ResolvedView, ifExists, purge) =>
Review comment:
nit: shall we add an assert here to make sure it's not temp view?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715579551
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r511008086
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveNoopDropTable.scala
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.analysis
+
+import org.apache.spark.sql.catalyst.plans.logical.{DropTable, LogicalPlan, NoopDropTable}
+import org.apache.spark.sql.catalyst.rules.Rule
+
+/**
+ * A rule for handling [[DropTable]] logical plan when the table or temp view is not resolved.
+ * If "ifExists" flag is set to true, the plan is resolved to [[NoopDropTable]],
+ * which is a no-op command. If the flag is set to false, the rule throws [[NoSuchTableException]].
+ */
+object ResolveNoopDropTable extends Rule[LogicalPlan] {
+ import org.apache.spark.sql.connector.catalog.CatalogV2Implicits.MultipartIdentifierHelper
+
+ def apply(plan: LogicalPlan): LogicalPlan = plan.resolveOperatorsUp {
+ case DropTable(u: UnresolvedTableOrView, ifExists, _) =>
+ if (ifExists) {
+ NoopDropTable(u.multipartIdentifier)
+ } else {
+ throw new NoSuchTableException(
Review comment:
Another approach could be introducing `UnresolvedTableOrTempView` for TABLE commands that work with both table and temp view.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-717574300
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715579557
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130212/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r510998223
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -228,8 +228,11 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
case DescribeColumn(_: ResolvedTable, _, _) =>
throw new AnalysisException("Describing columns is not supported for v2 tables.")
- case DropTable(catalog, ident, ifExists) =>
- DropTableExec(catalog, ident, ifExists) :: Nil
+ case DropTable(r: ResolvedTable, ifExists, _) =>
+ DropTableExec(r.catalog, r.identifier, ifExists) :: Nil
+
+ case NoopDropTable(multipartIdentifier) =>
+ NoopDropTableExec(multipartIdentifier) :: Nil
Review comment:
Changed. The EXPLAIN output will look like the following:
```
|== Parsed Logical Plan ==
'DropTable true, false
+- 'UnresolvedTableOrView [testcat, db, notbl]
== Analyzed Logical Plan ==
NoopDropTable [testcat, db, notbl]
== Optimized Logical Plan ==
NoopDropTable [testcat, db, notbl]
== Physical Plan ==
LocalTableScan <empty>
```
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715504848
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r511131449
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/command/PlanResolutionSuite.scala
##########
@@ -630,10 +630,10 @@ class PlanResolutionSuite extends AnalysisTest {
}
test("drop table") {
- val tableName1 = "db.tab"
- val tableIdent1 = TableIdentifier("tab", Option("db"))
- val tableName2 = "tab"
- val tableIdent2 = TableIdentifier("tab", Some("default"))
+ val tableName1 = "db.v1Table"
Review comment:
We need to use an existing table now that table resolution is happening during analysis.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711115312
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34558/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r510987054
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveNoopDropTable.scala
##########
@@ -0,0 +1,40 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.catalyst.analysis
+
+import org.apache.spark.sql.catalyst.plans.logical.{DropTable, LogicalPlan, NoopDropTable}
+import org.apache.spark.sql.catalyst.rules.Rule
+
+/**
+ * A rule for handling [[DropTable]] logical plan when the table or temp view is not resolved.
+ * If "ifExists" flag is set to true, the plan is resolved to [[NoopDropTable]],
+ * which is a no-op command. If the flag is set to false, the rule throws [[NoSuchTableException]].
+ */
+object ResolveNoopDropTable extends Rule[LogicalPlan] {
+ import org.apache.spark.sql.connector.catalog.CatalogV2Implicits.MultipartIdentifierHelper
+
+ def apply(plan: LogicalPlan): LogicalPlan = plan.resolveOperatorsUp {
+ case DropTable(u: UnresolvedTableOrView, ifExists, _) =>
+ if (ifExists) {
+ NoopDropTable(u.multipartIdentifier)
+ } else {
+ throw new NoSuchTableException(
Review comment:
OK. Now if table cannot be resolved, we will get a message like `Table or view not found: testcat.db.notbl`. I guess this is fine even though it's a bit misleading since it should be `Table or temp view not found`? `DROP TABLE` supports either a table or a temp view.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715446209
**[Test build #130212 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130212/testReport)** for PR 30079 at commit [`0a87543`](https://github.com/apache/spark/commit/0a8754348117776296bcaca3c9af21319acf633b).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711112654
**[Test build #129952 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129952/testReport)** for PR 30079 at commit [`468925c`](https://github.com/apache/spark/commit/468925c232040812d452d12e3d3a2cbf28cb8fce).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711451853
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711116667
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-714885211
**[Test build #130189 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130189/testReport)** for PR 30079 at commit [`d886b18`](https://github.com/apache/spark/commit/d886b1834da7725c582ca2e8d59769afb85170f8).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711425694
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34575/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-717542037
**[Test build #130340 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130340/testReport)** for PR 30079 at commit [`dfc4492`](https://github.com/apache/spark/commit/dfc44923d10fa7cbb5cd6cde4dfcffa19a9e994d).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-714892836
**[Test build #130188 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130188/testReport)** for PR 30079 at commit [`432eb9d`](https://github.com/apache/spark/commit/432eb9d11671c4d17e0da0a96ed709527ad3b814).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds the following public classes _(experimental)_:
* `case class UnresolvedTableOrView(multipartIdentifier: Seq[String]) extends LeafNode `
* `case class NoopDropTable(multipartIdentifier: Seq[String]) extends Command`
* `case class DropTableExec(catalog: TableCatalog, ident: Identifier, ifExists: Boolean)`
* `case class NoopDropTableExec(multipartIdentifier: Seq[String]) extends V2CommandExec `
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711427654
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34576/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-717643342
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r507208340
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/connector/TestV2SessionCatalogBase.scala
##########
@@ -74,8 +82,11 @@ private[connector] trait TestV2SessionCatalogBase[T <: Table] extends Delegating
}
def clearTables(): Unit = {
- assert(!tables.isEmpty, "Tables were empty, maybe didn't use the session catalog code path?")
Review comment:
Now that `withTable` works consistently, this logic should be updated.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-717712906
thanks, merging to master!
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan closed pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
cloud-fan closed pull request #30079:
URL: https://github.com/apache/spark/pull/30079
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r512047429
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -368,9 +368,13 @@ class ResolveSessionCatalog(
orCreate = c.orCreate)
}
+ case DropTable(
+ r @ ResolvedTable(_, _, _: V1Table), ifExists, purge) if isSessionCatalog(r.catalog) =>
+ DropTableCommand(r.identifier.asTableIdentifier, ifExists, isView = false, purge = purge)
+
// v1 DROP TABLE supports temp view.
- case DropTableStatement(TempViewOrV1Table(name), ifExists, purge) =>
- DropTableCommand(name.asTableIdentifier, ifExists, isView = false, purge = purge)
+ case DropTable(r: ResolvedView, ifExists, purge) =>
Review comment:
You mean to make sure it's a temp view?
For v1, there are few checks for handling views/temp views inside `DropTableCommand`:
https://github.com/apache/spark/blob/11bbb130df7b083f42acf0207531efe3912d89eb/sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala#L226-L229
Do you want me to copy the checks here?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-714900559
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34790/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715434241
**[Test build #130211 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130211/testReport)** for PR 30079 at commit [`b4ea475`](https://github.com/apache/spark/commit/b4ea47566457ae81b928a19fb1598f332550e569).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r513193419
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -367,9 +367,17 @@ class ResolveSessionCatalog(
orCreate = c.orCreate)
}
+ case DropTable(
+ r @ ResolvedTable(_, _, _: V1Table), ifExists, purge) if isSessionCatalog(r.catalog) =>
+ DropTableCommand(r.identifier.asTableIdentifier, ifExists, isView = false, purge = purge)
+
// v1 DROP TABLE supports temp view.
- case DropTableStatement(TempViewOrV1Table(name), ifExists, purge) =>
- DropTableCommand(name.asTableIdentifier, ifExists, isView = false, purge = purge)
+ case DropTable(r: ResolvedView, ifExists, purge) =>
+ if (!r.isTemp) {
Review comment:
Currently, it's duplicated with the check in `DropTableCommand`. I think it's OK as we want to move to v2 commands eventually.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711427658
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-717542037
**[Test build #130340 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130340/testReport)** for PR 30079 at commit [`dfc4492`](https://github.com/apache/spark/commit/dfc44923d10fa7cbb5cd6cde4dfcffa19a9e994d).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715523169
**[Test build #130214 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130214/testReport)** for PR 30079 at commit [`43bb465`](https://github.com/apache/spark/commit/43bb465dcbda783de4ac1d3e1987e6035a311ad0).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r517072152
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -228,8 +228,11 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
case DescribeColumn(_: ResolvedTable, _, _) =>
throw new AnalysisException("Describing columns is not supported for v2 tables.")
- case DropTable(catalog, ident, ifExists) =>
- DropTableExec(catalog, ident, ifExists) :: Nil
+ case DropTable(r: ResolvedTable, ifExists, _) =>
Review comment:
How useful is the `purge` option? If it's rarely used, we can just fail v2 DROP TABLE if `purge` is specified.
cc @rdblue @brkyvz
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r509947710
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/v2ResolutionPlans.scala
##########
@@ -45,12 +45,25 @@ case class UnresolvedTable(multipartIdentifier: Seq[String]) extends LeafNode {
/**
* Holds the name of a table or view that has yet to be looked up in a catalog. It will
* be resolved to [[ResolvedTable]] or [[ResolvedView]] during analysis.
+ *
+ * If 'isResolutionRequired' is set to false and the name cannot be resolved to a table or view,
+ * [[UnresolvedTableOrView]] will be converted to [[NotFoundTableOrView]].
*/
-case class UnresolvedTableOrView(multipartIdentifier: Seq[String]) extends LeafNode {
+case class UnresolvedTableOrView(
+ multipartIdentifier: Seq[String],
+ isResolutionRequired: Boolean = true) extends LeafNode {
override lazy val resolved: Boolean = false
override def output: Seq[Attribute] = Nil
}
+/**
+ * Holds the name of a table or view that has been looked up in a catalog, but not found.
+ * This is a "resolved" logical.
+ */
+case class NotFoundTableOrView(multipartIdentifier: Seq[String]) extends LeafNode {
Review comment:
If this is specific to DROP TABLE, let's avoid changing the framework. How about we add a rule in the `Post-Hoc Resolution` batch in the analyzer:
```
object ResolveNoopDropTable extends Rule[LogicalPlan] {
def apply(plan: LogicalPlan) = plan.transform {
case DropTable(_: UnresolvedTableOrView, _, true) => NoopDropTable
}
}
```
where `NoopDropTable` is a new command that does nothing but only keeping the table name for EXPLAIN.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711421566
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34575/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711126933
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711415357
**[Test build #129968 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129968/testReport)** for PR 30079 at commit [`01affe4`](https://github.com/apache/spark/commit/01affe462166e28d209a8a94c0ca0337bd68b6a9).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715478659
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/34812/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711443406
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/129968/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711117211
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r510594110
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -228,8 +228,11 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
case DescribeColumn(_: ResolvedTable, _, _) =>
throw new AnalysisException("Describing columns is not supported for v2 tables.")
- case DropTable(catalog, ident, ifExists) =>
- DropTableExec(catalog, ident, ifExists) :: Nil
+ case DropTable(r: ResolvedTable, ifExists, _) =>
Review comment:
not related to this PR, but we should think about how to handle the `purge` flag in v2 API.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-714900584
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711443406
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-717564223
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34942/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r518329054
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -228,8 +228,11 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
case DescribeColumn(_: ResolvedTable, _, _) =>
throw new AnalysisException("Describing columns is not supported for v2 tables.")
- case DropTable(catalog, ident, ifExists) =>
- DropTableExec(catalog, ident, ifExists) :: Nil
+ case DropTable(r: ResolvedTable, ifExists, _) =>
Review comment:
I created #30267 based on the feedback.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711425698
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-717574295
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711116667
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-714885211
**[Test build #130189 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130189/testReport)** for PR 30079 at commit [`d886b18`](https://github.com/apache/spark/commit/d886b1834da7725c582ca2e8d59769afb85170f8).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715627307
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-714905092
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34791/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711443401
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-714893009
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130188/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r512772861
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/catalyst/analysis/ResolveSessionCatalog.scala
##########
@@ -368,9 +368,13 @@ class ResolveSessionCatalog(
orCreate = c.orCreate)
}
+ case DropTable(
+ r @ ResolvedTable(_, _, _: V1Table), ifExists, purge) if isSessionCatalog(r.catalog) =>
+ DropTableCommand(r.identifier.asTableIdentifier, ifExists, isView = false, purge = purge)
+
// v1 DROP TABLE supports temp view.
- case DropTableStatement(TempViewOrV1Table(name), ifExists, purge) =>
- DropTableCommand(name.asTableIdentifier, ifExists, isView = false, purge = purge)
+ case DropTable(r: ResolvedView, ifExists, purge) =>
Review comment:
just `assert(r.isTemp)`
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711425052
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34576/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715477279
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34811/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-714893004
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715564285
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34814/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715564307
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715541905
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34814/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715478637
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34812/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] rdblue commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
rdblue commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r517502824
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/DataSourceV2Strategy.scala
##########
@@ -228,8 +228,11 @@ class DataSourceV2Strategy(session: SparkSession) extends Strategy with Predicat
case DescribeColumn(_: ResolvedTable, _, _) =>
throw new AnalysisException("Describing columns is not supported for v2 tables.")
- case DropTable(catalog, ident, ifExists) =>
- DropTableExec(catalog, ident, ifExists) :: Nil
+ case DropTable(r: ResolvedTable, ifExists, _) =>
Review comment:
I think it would make sense to add a variant of `dropTable` that accepts a purge option. Then have a default implementation that ignores it and calls `dropTable` without it. This seems like something that catalogs should be passed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r507261691
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/v2/jdbc/JDBCTableCatalogSuite.scala
##########
@@ -77,8 +77,8 @@ class JDBCTableCatalogSuite extends QueryTest with SharedSparkSession {
sql("DROP TABLE h2.test.to_drop")
checkAnswer(sql("SHOW TABLES IN h2.test"), Seq(Row("test", "people")))
Seq(
- "h2.test.not_existing_table" -> "Table test.not_existing_table not found",
- "h2.bad_test.not_existing_table" -> "Table bad_test.not_existing_table not found"
+ "h2.test.not_existing_table" -> "Table h2.test.not_existing_table not found",
+ "h2.bad_test.not_existing_table" -> "Table h2.bad_test.not_existing_table not found"
Review comment:
Now that table is resolved before `drop table` runs, when the table is not found, it will always print the original identifier.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715627307
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711126933
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715446209
**[Test build #130212 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130212/testReport)** for PR 30079 at commit [`0a87543`](https://github.com/apache/spark/commit/0a8754348117776296bcaca3c9af21319acf633b).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715504848
Merged build finished. Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-714894445
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34790/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] cloud-fan commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
cloud-fan commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r510587663
##########
File path: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/v2ResolutionPlans.scala
##########
@@ -81,7 +81,7 @@ case class ResolvedTable(catalog: TableCatalog, identifier: Identifier, table: T
*/
// TODO: create a generic representation for temp view, v1 view and v2 view, after we add view
// support to v2 catalog. For now we only need the identifier to fallback to v1 command.
-case class ResolvedView(identifier: Identifier) extends LeafNode {
+case class ResolvedView(identifier: Identifier, isTempView: Boolean) extends LeafNode {
Review comment:
nit: the class name is `ResolvedView`, so the flag can be named as `isTemp`, as we are already in the context of view.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715478651
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715504690
**[Test build #130211 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/130211/testReport)** for PR 30079 at commit [`b4ea475`](https://github.com/apache/spark/commit/b4ea47566457ae81b928a19fb1598f332550e569).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] AmplabJenkins removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-715504859
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/130211/
Test FAILed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711451513
**[Test build #129969 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129969/testReport)** for PR 30079 at commit [`f23e36d`](https://github.com/apache/spark/commit/f23e36d9aae80f1d491ba9ca61ef85d19cf61fb5).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA commented on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711115008
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/34557/
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] imback82 commented on a change in pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
imback82 commented on a change in pull request #30079:
URL: https://github.com/apache/spark/pull/30079#discussion_r510995012
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/connector/TestV2SessionCatalogBase.scala
##########
@@ -74,8 +82,11 @@ private[connector] trait TestV2SessionCatalogBase[T <: Table] extends Delegating
}
def clearTables(): Unit = {
- assert(!tables.isEmpty, "Tables were empty, maybe didn't use the session catalog code path?")
+ assert(
+ tableCreated.get,
Review comment:
We have tests like the following:
```scala
test("saveAsTable: Append mode should not fail if the table already exists " +
"and a same-name temp view exist") {
withTable("same_name") {
withTempView("same_name") {
```
Before, the table was not cleaned up correctly, so `!tables.isEmpty` was fine. But now that tables are cleaned up with this PR, we need a different way to check if this code path was executed.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] [spark] SparkQA removed a comment on pull request #30079: [SPARK-33174][SQL] Migrate DROP TABLE to use UnresolvedTableOrView to resolve the identifier
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #30079:
URL: https://github.com/apache/spark/pull/30079#issuecomment-711415357
**[Test build #129968 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/129968/testReport)** for PR 30079 at commit [`01affe4`](https://github.com/apache/spark/commit/01affe462166e28d209a8a94c0ca0337bd68b6a9).
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org