Skip to content

Conversation

@TomokoKomiyama
Copy link
Contributor

@TomokoKomiyama TomokoKomiyama commented Sep 19, 2019

What changes were proposed in this pull request?

Added try exception

Why are the changes needed?

The behaviors of run commands during exception handling are different depends on explain command. I think it should be unified.

[ >spark.sql("explain cost select * from hoge").show(false) ]
cost

[ >spark.sql("explain extended select * from hoge").show(false) ]
extemded

Does this PR introduce any user-facing change?

No

How was this patch tested?

tested manually

if (codegen) {
try {
codegenString(queryExecution.executedPlan)
} catch {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indentation should be fixed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the review.
I've done.

Copy link
Member

@srowen srowen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure we want to let the explain plan proceed here? it has failed already, and I'm not sure about just appending it to the output as if successful.

@TomokoKomiyama
Copy link
Contributor Author

TomokoKomiyama commented Sep 26, 2019

@srowen
If we use 'extend', sql behave like as follows even if we have error

scala> spark.sql("explain extended select * from hoge").show(false)
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
|plan                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                                      |
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+
|== Parsed Logical Plan ==
'Project [*]
+- 'UnresolvedRelation [hoge]

== Analyzed Logical Plan ==
org.apache.spark.sql.AnalysisException: Table or view not found: hoge; line 1 pos 31;
'Project [*]
+- 'UnresolvedRelation [hoge]

org.apache.spark.sql.AnalysisException: Table or view not found: hoge; line 1 pos 31;
'Project [*]
+- 'UnresolvedRelation [hoge]

== Optimized Logical Plan ==
org.apache.spark.sql.AnalysisException: Table or view not found: hoge; line 1 pos 31;
'Project [*]
+- 'UnresolvedRelation [hoge]

== Physical Plan ==
org.apache.spark.sql.AnalysisException: Table or view not found: hoge; line 1 pos 31;
'Project [*]
+- 'UnresolvedRelation [hoge]
|
+------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------+

, because of the try exception in sql/core/src/main/scala/org/apache/spark/sql/execution/QueryExecution.scala(L156-)

    } catch {
      case e: AnalysisException => e.toString
    }
    append(analyzedOutput)
    append("\n")
    QueryPlan.append(analyzed, append, verbose, addSuffix, maxFields)
    append("\n== Optimized Logical Plan ==\n")
    QueryPlan.append(optimizedPlan, append, verbose, addSuffix, maxFields)
    append("\n== Physical Plan ==\n")
    QueryPlan.append(executedPlan, append, verbose, addSuffix, maxFields)
  }

Other options such as cost, formated and codegen don't behave like this, so it needs something change to be consistent.
I suggest that add try exceptions in other explain commands to make these behaviors like 'extend' one.

@srowen
Copy link
Member

srowen commented Sep 26, 2019

OK, that's a good argument from consistency. @kiszk do you agree?

@kiszk
Copy link
Member

kiszk commented Sep 27, 2019

I agree to make their behaviors in explain consistent.

@SparkQA
Copy link

SparkQA commented Sep 27, 2019

Test build #4885 has finished for PR 25848 at commit 9430c12.

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

@srowen srowen closed this in 67d5b9b Sep 29, 2019
@srowen
Copy link
Member

srowen commented Sep 29, 2019

Merged to master

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants