[SPARK-33414][SQL] Migrate SHOW CREATE TABLE command to use UnresolvedTableOrView to resolve the identifier #30321

imback82 · 2020-11-10T19:46:26Z

What changes were proposed in this pull request?

This PR proposes to migrate SHOW CREATE TABLE to use UnresolvedTableOrView to resolve the table 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 or proposal doc.

Note that SHOW CREATE TABLE works only with a v1 table and a permanent view, and not supported for v2 tables.

Why are the changes needed?

The changes allow consistent resolution behavior when resolving the table identifier. For example, the following is the current behavior:

sql("CREATE TEMPORARY VIEW t AS SELECT 1")
sql("CREATE DATABASE db")
sql("CREATE TABLE t (key INT, value STRING) USING hive")
sql("USE db")
sql("SHOW CREATE TABLE t AS SERDE") // Succeeds

With this change, SHOW CREATE TABLE ... AS SERDE above fails with the following:

org.apache.spark.sql.AnalysisException: t is a temp view not table or permanent view.; line 1 pos 0
  at org.apache.spark.sql.catalyst.analysis.package$AnalysisErrorAt.failAnalysis(package.scala:42)
  at org.apache.spark.sql.catalyst.analysis.Analyzer$ResolveTempViews$$anonfun$apply$7.$anonfun$applyOrElse$43(Analyzer.scala:883)
  at scala.Option.map(Option.scala:230)

, which is expected since temporary view is resolved first and SHOW CREATE TABLE ... AS SERDE doesn't support a temporary view.

Note that there is no behavior change for SHOW CREATE TABLE without AS SERDE since it was already resolving to a temporary view first. See below for more detail.

Does this PR introduce any user-facing change?

After this PR, SHOW CREATE TABLE t AS SERDE is resolved to a temp view t instead of table db.t in the above scenario.

Note that there is no behavior change for SHOW CREATE TABLE without AS SERDE, but the exception message changes from SHOW CREATE TABLE is not supported on a temporary view to t is a temp view not table or permanent view.

How was this patch tested?

Updated existing tests.

SparkQA · 2020-11-10T20:35:56Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35496/

SparkQA · 2020-11-10T21:06:59Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/35496/

imback82 · 2020-11-10T21:33:01Z

cc @cloud-fan (GA passed)

SparkQA · 2020-11-10T21:43:25Z

Test build #130890 has finished for PR 30321 at commit e534ebf.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds the following public classes (experimental):
case class ShowCreateTable(child: LogicalPlan, asSerde: Boolean = false) extends Command

cloud-fan · 2020-11-11T05:49:00Z

sql/core/src/test/scala/org/apache/spark/sql/ShowCreateTableSuite.scala

@@ -155,16 +155,17 @@ abstract class ShowCreateTableSuite extends QueryTest with SQLTestUtils {
      val ex = intercept[AnalysisException] {
        sql(s"SHOW CREATE TABLE $viewName")
      }
-      assert(ex.getMessage.contains("SHOW CREATE TABLE is not supported on a temporary view"))
+      assert(ex.getMessage.contains(s"$viewName is a temp view not table or permanent view"))


Not related to this PR: One idea to improve the error message is to put the command name in UnresolvedTableOrView or other similar plans when the parser creates the command logical plan.

A good idea! I will follow up on this.

cloud-fan · 2020-11-11T05:54:26Z

thanks, merging to master!

…edTable ### What changes were proposed in this pull request? This PR proposes to improve the exception messages while `UnresolvedTable` is handled based on this suggestion: #30321 (comment). Currently, when an identifier is resolved to a view when a table is expected, the following exception message is displayed (e.g., for `COMMENT ON TABLE`): ``` v is a temp view not table. ``` After this PR, the message will be: ``` v is a temp view. 'COMMENT ON TABLE' expects a table. ``` Also, if an identifier is not resolved, the following exception message is currently used: ``` Table not found: t ``` After this PR, the message will be: ``` Table not found for 'COMMENT ON TABLE': t ``` ### Why are the changes needed? To improve the exception message. ### Does this PR introduce _any_ user-facing change? Yes, the exception message will be changed as described above. ### How was this patch tested? Updated existing tests. Closes #30461 from imback82/unresolved_table_message. Authored-by: Terry Kim <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

…edTableOrView ### What changes were proposed in this pull request? This PR proposes to improve the exception messages while `UnresolvedTableOrView` is handled based on this suggestion: #30321 (comment). Currently, when an identifier is resolved to a temp view when a table/permanent view is expected, the following exception message is displayed (e.g., for `SHOW CREATE TABLE`): ``` t is a temp view not table or permanent view. ``` After this PR, the message will be: ``` t is a temp view. 'SHOW CREATE TABLE' expects a table or permanent view. ``` Also, if an identifier is not resolved, the following exception message is currently used: ``` Table or view not found: t ``` After this PR, the message will be: ``` Table or permanent view not found for 'SHOW CREATE TABLE': t ``` or ``` Table or view not found for 'ANALYZE TABLE ... FOR COLUMNS ...': t ``` ### Why are the changes needed? To improve the exception message. ### Does this PR introduce _any_ user-facing change? Yes, the exception message will be changed as described above. ### How was this patch tested? Updated existing tests. Closes #30475 from imback82/unresolved_table_or_view. Authored-by: Terry Kim <[email protected]> Signed-off-by: Wenchen Fan <[email protected]>

initial commit

e534ebf

github-actions bot added the SQL label Nov 10, 2020

cloud-fan reviewed Nov 11, 2020

View reviewed changes

cloud-fan closed this in 6d5d030 Nov 11, 2020

imback82 deleted the show_create_table branch November 11, 2020 07:05

imback82 mentioned this pull request Nov 23, 2020

[SPARK-33515][SQL] Improve exception messages while handling UnresolvedTable #30461

Closed

imback82 mentioned this pull request Nov 23, 2020

[SPARK-33522][SQL] Improve exception messages while handling UnresolvedTableOrView #30475

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[SPARK-33414][SQL] Migrate SHOW CREATE TABLE command to use UnresolvedTableOrView to resolve the identifier #30321

[SPARK-33414][SQL] Migrate SHOW CREATE TABLE command to use UnresolvedTableOrView to resolve the identifier #30321

Uh oh!

imback82 commented Nov 10, 2020

Uh oh!

SparkQA commented Nov 10, 2020

Uh oh!

SparkQA commented Nov 10, 2020

Uh oh!

imback82 commented Nov 10, 2020 •

edited

Loading

Uh oh!

SparkQA commented Nov 10, 2020

Uh oh!

cloud-fan Nov 11, 2020

Uh oh!

imback82 Nov 11, 2020

Uh oh!

cloud-fan commented Nov 11, 2020

Uh oh!

Uh oh!

[SPARK-33414][SQL] Migrate SHOW CREATE TABLE command to use UnresolvedTableOrView to resolve the identifier #30321

[SPARK-33414][SQL] Migrate SHOW CREATE TABLE command to use UnresolvedTableOrView to resolve the identifier #30321

Uh oh!

Conversation

imback82 commented Nov 10, 2020

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

SparkQA commented Nov 10, 2020

Uh oh!

SparkQA commented Nov 10, 2020

Uh oh!

imback82 commented Nov 10, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SparkQA commented Nov 10, 2020

Uh oh!

cloud-fan Nov 11, 2020

Choose a reason for hiding this comment

Uh oh!

imback82 Nov 11, 2020

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Nov 11, 2020

Uh oh!

Uh oh!

imback82 commented Nov 10, 2020 •

edited

Loading