[SPARK-18169][SQL] Suppress warnings when dropping views on a dropped table #15682

dongjoon-hyun · 2016-10-29T23:14:48Z

What changes were proposed in this pull request?

Apache Spark 2.x shows an AnalysisException warning message when dropping a view on a dropped table. This does not happen on dropping temporary views. Also, Spark 1.6.x does not show warnings. We had better suppress this to be more consistent in Spark 2.x and with Spark 1.6.x.

scala> sql("create table t(a int)")

scala> sql("create view v as select * from t")

scala> sql("create temporary view tv as select * from t")

scala> sql("drop table t")

scala> sql("drop view tv")

scala> sql("drop view v")
16/10/29 15:50:03 WARN DropTableCommand: org.apache.spark.sql.AnalysisException: Table or view not found: `default`.`t`; line 1 pos 91;
'SubqueryAlias v, `default`.`v`
+- 'Project ['gen_attr_0 AS a#19]
   +- 'SubqueryAlias t
      +- 'Project ['gen_attr_0]
         +- 'SubqueryAlias gen_subquery_0
            +- 'Project ['a AS gen_attr_0#18]
               +- 'UnresolvedRelation `default`.`t`

org.apache.spark.sql.AnalysisException: Table or view not found: `default`.`t`; line 1 pos 91;
'SubqueryAlias v, `default`.`v`
+- 'Project ['gen_attr_0 AS a#19]
   +- 'SubqueryAlias t
      +- 'Project ['gen_attr_0]
         +- 'SubqueryAlias gen_subquery_0
            +- 'Project ['a AS gen_attr_0#18]
               +- 'UnresolvedRelation `default`.`t`
...
res5: org.apache.spark.sql.DataFrame = []

Note that this is different case of dropping non-exist views. For those cases, Spark raises NoSuchTableException.

How was this patch tested?

Manual because this is a suppression of warning message.

SparkQA · 2016-10-30T01:18:38Z

Test build #67775 has finished for PR 15682 at commit 9145c60.

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

… table

SparkQA · 2016-11-10T07:42:41Z

Test build #68449 has finished for PR 15682 at commit fef9981.

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

dongjoon-hyun · 2016-11-10T07:57:29Z

Retest this please.

SparkQA · 2016-11-10T08:56:36Z

Test build #68453 has finished for PR 15682 at commit fef9981.

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

dongjoon-hyun · 2016-11-10T09:16:41Z

Hmm. randomized aggregation test fails consistently.

- randomized aggregation test - [typed, with distinct] - with grouping keys - with non-empty input

dongjoon-hyun · 2016-11-10T16:26:38Z

Retest this please

SparkQA · 2016-11-10T18:40:23Z

Test build #68479 has finished for PR 15682 at commit fef9981.

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

dongjoon-hyun · 2016-11-14T20:12:19Z

Retest this please.

SparkQA · 2016-11-14T22:57:14Z

Test build #68635 has finished for PR 15682 at commit fef9981.

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

dongjoon-hyun · 2016-11-16T10:12:32Z

Hi, @gatorsmile .
How do you think about this PR? I can close this if you don't feel strongly.

gatorsmile · 2016-11-16T19:51:46Z

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

@@ -202,6 +202,7 @@ case class DropTableCommand(
      sparkSession.sharedState.cacheManager.uncacheQuery(
        sparkSession.table(tableName.quotedString))
    } catch {
+      case ae: AnalysisException if ae.getMessage.contains("Table or view not found") =>


Is it NoSuchTableException?

Thank you for review, @gatorsmile .
It's originally NoSuchTableException, but Analyzer changed into this at here.
So, the catch expression becomes ugly.

~~May I fix that to throw the original exception? NoSuchTableException extends AnalysisException and has more context like database name.~~ Oops, Sorry. We cannot. The current AnalysisException has the line number and start position.

gatorsmile · 2016-11-16T21:45:08Z

Based on my understanding, when we try to drop a view, we look up and analyze the view and then issue an exception if any involved table has been dropped. We might have a bug here.

That means, we will not call uncacheQuery, if any involved table is dropped before dropping the view. It is a bug, right?

dongjoon-hyun · 2016-11-16T21:59:12Z

Sorry, but uncacheQuery is related to this PR?

dongjoon-hyun · 2016-11-16T22:01:28Z

Ah, you mean that code you gave the pointer.

dongjoon-hyun · 2016-11-16T22:02:30Z

Yep. uncachedQuery is not called here. Yep. I got it. It's a bug. I'll fix that. Thank you, @gatorsmile !

gatorsmile · 2016-11-16T22:02:57Z

This case is more complicated. Please try to check whether there is a bug. If yes, please fix it in this PR. Suppress warnings might be easily fixed when you fix this bug.

dongjoon-hyun · 2016-11-16T22:05:33Z

Yep. I investigate the behavior here.

gatorsmile · 2016-11-16T22:13:44Z

Thank you!

I think the PR #15896 is fixing a related issue in override def uncacheTable(tableName: String). The fixed behavior needs to be consistent with each other.

TruncateTableCommand has the same issue. We need to suppress the warnings too

dongjoon-hyun · 2016-11-16T22:15:21Z

I see. I'll include that command here, too.

gatorsmile · 2016-11-16T23:01:04Z

Please follw the ongoing design decision changes in Spark-18465

dongjoon-hyun · 2016-11-16T23:06:10Z

Sure! I'm watching that PR now. I'll wait for a while.

dongjoon-hyun · 2016-11-19T12:42:47Z

Hi, @gatorsmile .
Is there any decision for the following?

The fixed behavior needs to be consistent with each other.

dongjoon-hyun · 2016-11-19T13:04:06Z

If the issue (and bugs) about uncacheQuery and uncacheTable is postpone, I'd like to focus on the warning messages in this PR. Is it okay? We can revisit that uncache cases for Apache Spark 2.2.

For TruncateTableCommand command, it's not allowed for view. So, it seems not to be related with dropped table case, here. I couldn't reproduce that.

gatorsmile · 2016-11-22T23:07:54Z

We should not suppress the warnings. Instead, we should improve the error message to show the unache attempt failed, right?

hvanhovell · 2016-11-22T23:11:05Z

@dongjoon-hyun @gatorsmile I totally missed this one. Shall we try to get this in 2.1?

gatorsmile · 2016-11-22T23:16:32Z

The warning message is valid. We might need to issue a better exception here.

To resolve the root cause, we might need adding a new API for uncaching the view in which a relevant table has been dropped? I am not confident how to do it in a perfect way. Let me open a JIRA now and cc both of you in the discussion.

dongjoon-hyun · 2016-11-22T23:19:16Z

Thank you for coming back, @gatorsmile and @hvanhovell !

gatorsmile · 2016-11-22T23:22:14Z

A JIRA is created: https://issues.apache.org/jira/browse/SPARK-18549

dongjoon-hyun · 2016-11-22T23:28:34Z

Ah, In fact, that issue includes this. May I close this PR and the issue SPARK-18169 ?

gatorsmile · 2016-11-22T23:29:48Z

Sure, maybe close it now. If we are unable to contain it in Spark 2.1, we can continue this work to issue a better error message.

dongjoon-hyun · 2016-11-22T23:33:12Z

I guess SPARK-18169 will cover the better error message too. That issue will need to revisit all occurrence of uncache and related exception handling. Thank you, @gatorsmile and @hvanhovell . I close this in favor of SPARK-18549 .

[SPARK-18169][SQL] Suppress warnings when dropping views on a dropped…

fef9981

… table

gatorsmile reviewed Nov 16, 2016

View reviewed changes

gatorsmile mentioned this pull request Nov 16, 2016

[SPARK-18465] Add 'IF EXISTS' clause to 'UNCACHE' to not throw exceptions when table doesn't exist #15896

Closed

dongjoon-hyun mentioned this pull request Nov 21, 2016

[SPARK-18517][SQL] DROP TABLE IF EXISTS should not warn for non-existing tables #15953

Closed

dongjoon-hyun closed this Nov 22, 2016

dongjoon-hyun deleted the SPARK-18169 branch January 7, 2019 07:03

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-18169][SQL] Suppress warnings when dropping views on a dropped table #15682

[SPARK-18169][SQL] Suppress warnings when dropping views on a dropped table #15682

dongjoon-hyun commented Oct 29, 2016 •

edited

SparkQA commented Oct 30, 2016

SparkQA commented Nov 10, 2016

dongjoon-hyun commented Nov 10, 2016

SparkQA commented Nov 10, 2016

dongjoon-hyun commented Nov 10, 2016

dongjoon-hyun commented Nov 10, 2016

SparkQA commented Nov 10, 2016

dongjoon-hyun commented Nov 14, 2016

SparkQA commented Nov 14, 2016

dongjoon-hyun commented Nov 16, 2016

gatorsmile Nov 16, 2016

dongjoon-hyun Nov 16, 2016

dongjoon-hyun Nov 16, 2016 •

edited

gatorsmile commented Nov 16, 2016 •

edited

dongjoon-hyun commented Nov 16, 2016

dongjoon-hyun commented Nov 16, 2016

dongjoon-hyun commented Nov 16, 2016 •

edited

gatorsmile commented Nov 16, 2016

dongjoon-hyun commented Nov 16, 2016

gatorsmile commented Nov 16, 2016 •

edited

dongjoon-hyun commented Nov 16, 2016

gatorsmile commented Nov 16, 2016

dongjoon-hyun commented Nov 16, 2016 •

edited

dongjoon-hyun commented Nov 19, 2016

dongjoon-hyun commented Nov 19, 2016 •

edited

gatorsmile commented Nov 22, 2016

hvanhovell commented Nov 22, 2016

gatorsmile commented Nov 22, 2016

dongjoon-hyun commented Nov 22, 2016

gatorsmile commented Nov 22, 2016

dongjoon-hyun commented Nov 22, 2016

gatorsmile commented Nov 22, 2016

dongjoon-hyun commented Nov 22, 2016

[SPARK-18169][SQL] Suppress warnings when dropping views on a dropped table #15682

[SPARK-18169][SQL] Suppress warnings when dropping views on a dropped table #15682

Conversation

dongjoon-hyun commented Oct 29, 2016 • edited

What changes were proposed in this pull request?

How was this patch tested?

SparkQA commented Oct 30, 2016

SparkQA commented Nov 10, 2016

dongjoon-hyun commented Nov 10, 2016

SparkQA commented Nov 10, 2016

dongjoon-hyun commented Nov 10, 2016

dongjoon-hyun commented Nov 10, 2016

SparkQA commented Nov 10, 2016

dongjoon-hyun commented Nov 14, 2016

SparkQA commented Nov 14, 2016

dongjoon-hyun commented Nov 16, 2016

gatorsmile Nov 16, 2016

Choose a reason for hiding this comment

dongjoon-hyun Nov 16, 2016

Choose a reason for hiding this comment

dongjoon-hyun Nov 16, 2016 • edited

Choose a reason for hiding this comment

gatorsmile commented Nov 16, 2016 • edited

dongjoon-hyun commented Nov 16, 2016

dongjoon-hyun commented Nov 16, 2016

dongjoon-hyun commented Nov 16, 2016 • edited

gatorsmile commented Nov 16, 2016

dongjoon-hyun commented Nov 16, 2016

gatorsmile commented Nov 16, 2016 • edited

dongjoon-hyun commented Nov 16, 2016

gatorsmile commented Nov 16, 2016

dongjoon-hyun commented Nov 16, 2016 • edited

dongjoon-hyun commented Nov 19, 2016

dongjoon-hyun commented Nov 19, 2016 • edited

gatorsmile commented Nov 22, 2016

hvanhovell commented Nov 22, 2016

gatorsmile commented Nov 22, 2016

dongjoon-hyun commented Nov 22, 2016

gatorsmile commented Nov 22, 2016

dongjoon-hyun commented Nov 22, 2016

gatorsmile commented Nov 22, 2016

dongjoon-hyun commented Nov 22, 2016

dongjoon-hyun commented Oct 29, 2016 •

edited

dongjoon-hyun Nov 16, 2016 •

edited

gatorsmile commented Nov 16, 2016 •

edited

dongjoon-hyun commented Nov 16, 2016 •

edited

gatorsmile commented Nov 16, 2016 •

edited

dongjoon-hyun commented Nov 16, 2016 •

edited

dongjoon-hyun commented Nov 19, 2016 •

edited