[SPARK-22488] [SQL] Fix the view resolution issue in the SparkSession internal table() API #19713

gatorsmile · 2017-11-10T04:16:46Z

What changes were proposed in this pull request?

The current internal table() API of SparkSession bypasses the Analyzer and directly calls sessionState.catalog.lookupRelation API. This skips the view resolution logics in our Analyzer rule ResolveRelations. This internal API is widely used by various DDL commands, public and internal APIs.

Users might get the strange error caused by view resolution when the default database is different.

Table or view not found: t1; line 1 pos 14
org.apache.spark.sql.AnalysisException: Table or view not found: t1; line 1 pos 14
	at org.apache.spark.sql.catalyst.analysis.package$AnalysisErrorAt.failAnalysis(package.scala:42)

This PR is to fix it by enforcing it to use ResolveRelations to resolve the table.

How was this patch tested?

Added a test case and modified the existing test cases

gatorsmile · 2017-11-10T04:17:01Z

cc @cloud-fan @jiangxb1987

SparkQA · 2017-11-10T06:22:53Z

Test build #83664 has finished for PR 19713 at commit d87f333.

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

cloud-fan · 2017-11-10T08:10:50Z

sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala

  }

-  test("rename temporary view") {
+  test("rename temporary table") {


temporary view is more accurate?

Copied from 2.2 version... : )

cloud-fan · 2017-11-10T08:12:15Z

sql/core/src/main/scala/org/apache/spark/sql/SparkSession.scala

    table(sessionState.sqlParser.parseTableIdentifier(tableName))
  }

  private[sql] def table(tableIdent: TableIdentifier): DataFrame = {


This is also called by the public table API, so this bug affects the public interface too.

Yeah. many public APIs and DDL command processing are based on this internal API.

cloud-fan · 2017-11-10T08:34:03Z

LGTM

SparkQA · 2017-11-10T11:05:43Z

Test build #83678 has finished for PR 19713 at commit db2292f.

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

SparkQA · 2017-11-10T11:09:51Z

Test build #83674 has finished for PR 19713 at commit 99936cc.

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

cloud-fan · 2017-11-10T11:10:11Z

it's a valid failure

Failed -------------------------------------------------------------------------
1. Failure: test cache, uncache and clearCache (@test_sparkSQL.R#735) ----------
error$message does not match "Error in uncacheTable : no such table - Table or view 'foo' not found in database 'default'".
Actual value: "Error in uncacheTable : analysis error - Table or view not found: foo;"

SparkQA · 2017-11-11T08:05:02Z

Test build #83717 has finished for PR 19713 at commit c491974.

This patch fails due to an unknown error code, -9.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2017-11-11T10:11:50Z

R/pkg/tests/fulltests/test_sparkSQL.R


  expect_error(uncacheTable("foo"),
-      "Error in uncacheTable : no such table - Table or view 'foo' not found in database 'default'")
+      "Error in uncacheTable : analysis error - Table or view not found: foo")


tiny nit:

expect_error(uncacheTable("foo"), "Error in uncacheTable : analysis error - Table or view not found: foo")

SparkQA · 2017-11-11T13:26:22Z

Test build #83721 has finished for PR 19713 at commit c491974.

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

jiangxb1987

LGTM

cloud-fan · 2017-11-11T17:24:55Z

thanks, merging to master! can you send a new PR for 2.2?

felixcheung · 2017-11-11T18:11:46Z

Sounds like we should have for 2.2.1 I’m looking at cutting 2.2.1 ASAP, could someone backport this and tag me?

…internal table() API The current internal `table()` API of `SparkSession` bypasses the Analyzer and directly calls `sessionState.catalog.lookupRelation` API. This skips the view resolution logics in our Analyzer rule `ResolveRelations`. This internal API is widely used by various DDL commands, public and internal APIs. Users might get the strange error caused by view resolution when the default database is different. ``` Table or view not found: t1; line 1 pos 14 org.apache.spark.sql.AnalysisException: Table or view not found: t1; line 1 pos 14 at org.apache.spark.sql.catalyst.analysis.package$AnalysisErrorAt.failAnalysis(package.scala:42) ``` This PR is to fix it by enforcing it to use `ResolveRelations` to resolve the table. Added a test case and modified the existing test cases Author: gatorsmile <[email protected]> Closes apache#19713 from gatorsmile/viewResolution.

dongjoon-hyun · 2017-12-05T05:13:38Z

Hi, All.
It seems that we missed DROP TABLE IF EXISTS case here.
(I also forgot to check that during 2.2-backporting of this PR.)
This PR swallows NoSuchTableException and emits AnalysisException. DROP TABLE IF EXISTS expected NoSuchTableException in ddl.scala. It becomes a regression on 2.2.1.

…tion ## What changes were proposed in this pull request? During [SPARK-22488](apache#19713) to fix view resolution issue, there occurs a regression at `2.2.1` and `master` branch like the following. This PR fixes that. ```scala scala> spark.version res2: String = 2.2.1 scala> sql("DROP TABLE IF EXISTS t").show 17/12/04 21:01:06 WARN DropTableCommand: org.apache.spark.sql.AnalysisException: Table or view not found: t; org.apache.spark.sql.AnalysisException: Table or view not found: t; ``` ## How was this patch tested? Manual. Author: Dongjoon Hyun <[email protected]> Closes apache#19888 from dongjoon-hyun/SPARK-22686.

…tion ## What changes were proposed in this pull request? During [SPARK-22488](#19713) to fix view resolution issue, there occurs a regression at `2.2.1` and `master` branch like the following. This PR fixes that. ```scala scala> spark.version res2: String = 2.2.1 scala> sql("DROP TABLE IF EXISTS t").show 17/12/04 21:01:06 WARN DropTableCommand: org.apache.spark.sql.AnalysisException: Table or view not found: t; org.apache.spark.sql.AnalysisException: Table or view not found: t; ``` ## How was this patch tested? Manual. Author: Dongjoon Hyun <[email protected]> Closes #19888 from dongjoon-hyun/SPARK-22686. (cherry picked from commit 82183f7) Signed-off-by: Wenchen Fan <[email protected]>

…tion ## What changes were proposed in this pull request? During [SPARK-22488](apache#19713) to fix view resolution issue, there occurs a regression at `2.2.1` and `master` branch like the following. This PR fixes that. ```scala scala> spark.version res2: String = 2.2.1 scala> sql("DROP TABLE IF EXISTS t").show 17/12/04 21:01:06 WARN DropTableCommand: org.apache.spark.sql.AnalysisException: Table or view not found: t; org.apache.spark.sql.AnalysisException: Table or view not found: t; ``` ## How was this patch tested? Manual. Author: Dongjoon Hyun <[email protected]> Closes apache#19888 from dongjoon-hyun/SPARK-22686. (cherry picked from commit 82183f7) Signed-off-by: Wenchen Fan <[email protected]>

fix.

d87f333

fix

99936cc

cloud-fan reviewed Nov 10, 2017

View reviewed changes

fix

db2292f

fix

c491974

HyukjinKwon reviewed Nov 11, 2017

View reviewed changes

jiangxb1987 approved these changes Nov 11, 2017

View reviewed changes

asfgit closed this in d6ee69e Nov 11, 2017

dongjoon-hyun mentioned this pull request Dec 5, 2017

[SPARK-22686][SQL] DROP TABLE IF EXISTS should not show AnalysisException #19888

Closed

[SPARK-22488] [SQL] Fix the view resolution issue in the SparkSession internal table() API #19713

[SPARK-22488] [SQL] Fix the view resolution issue in the SparkSession internal table() API #19713

Uh oh!

Conversation

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

Uh oh!

What changes were proposed in this pull request?

How was this patch tested?

Uh oh!

gatorsmile commented Nov 10, 2017

Uh oh!

SparkQA commented Nov 10, 2017

Uh oh!

cloud-fan Nov 10, 2017

Choose a reason for hiding this comment

Uh oh!

gatorsmile Nov 10, 2017

Choose a reason for hiding this comment

Uh oh!

cloud-fan Nov 10, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gatorsmile Nov 10, 2017

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Nov 10, 2017

Uh oh!

SparkQA commented Nov 10, 2017

Uh oh!

SparkQA commented Nov 10, 2017

Uh oh!

cloud-fan commented Nov 10, 2017

Uh oh!

SparkQA commented Nov 11, 2017

Uh oh!

HyukjinKwon Nov 11, 2017

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Nov 11, 2017

Uh oh!

jiangxb1987 left a comment

Choose a reason for hiding this comment

Uh oh!

cloud-fan commented Nov 11, 2017

Uh oh!

felixcheung commented Nov 11, 2017 via email

Uh oh!

dongjoon-hyun commented Dec 5, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

gatorsmile commented Nov 10, 2017 •

edited

Loading

cloud-fan Nov 10, 2017 •

edited

Loading

dongjoon-hyun commented Dec 5, 2017 •

edited

Loading