[SPARK-28195][SQL] Fix CheckAnalysis not working for InsertIntoDataSourceDirCommand and report misleading error message by liupc · Pull Request #25019 · apache/spark

liupc · 2019-07-01T04:20:06Z

What changes were proposed in this pull request?

This PR will try to fix the issue that the sub plan of InsertDataSourceDirCommand is not being checked for analysis and sometimes will report misleading error message.
An example sql is like
insert overwrite directory '/path' using parquet select * from table1

When "table1" does not exists, we will finally got a misleading error message:

Caused by: org.apache.spark.sql.catalyst.analysis.UnresolvedException: Invalid call to dataType on unresolved object, tree: 'kr.objective_id
at org.apache.spark.sql.catalyst.analysis.UnresolvedAttribute.dataType(unresolved.scala:105)
at org.apache.spark.sql.types.StructType$$anonfun$fromAttributes$1.apply(StructType.scala:440)
at org.apache.spark.sql.types.StructType$$anonfun$fromAttributes$1.apply(StructType.scala:440)
at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234)
at scala.collection.TraversableLike$$anonfun$map$1.apply(TraversableLike.scala:234)
at scala.collection.immutable.List.foreach(List.scala:381)
at scala.collection.TraversableLike$class.map(TraversableLike.scala:234)
at scala.collection.immutable.List.map(List.scala:285)
at org.apache.spark.sql.types.StructType$.fromAttributes(StructType.scala:440)
at org.apache.spark.sql.catalyst.plans.QueryPlan.schema$lzycompute(QueryPlan.scala:159)
at org.apache.spark.sql.catalyst.plans.QueryPlan.schema(QueryPlan.scala:159)
at org.apache.spark.sql.execution.datasources.DataSource.planForWriting(DataSource.scala:544)
at org.apache.spark.sql.execution.command.InsertIntoDataSourceDirCommand.run(InsertIntoDataSourceDirCommand.scala:70)
at org.apache.spark.sql.execution.command.ExecutedCommandExec.sideEffectResult$lzycompute(commands.scala:70)
at org.apache.spark.sql.execution.command.ExecutedCommandExec.sideEffectResult(commands.scala:68)
at org.apache.spark.sql.execution.command.ExecutedCommandExec.executeCollect(commands.scala:79)
at org.apache.spark.sql.execution.adaptive.QueryStage.executeCollect(QueryStage.scala:246)
at org.apache.spark.sql.Dataset$$anonfun$6.apply(Dataset.scala:190)
at org.apache.spark.sql.Dataset$$anonfun$6.apply(Dataset.scala:190)
at org.apache.spark.sql.Dataset$$anonfun$52.apply(Dataset.scala:3277)
at org.apache.spark.sql.execution.SQLExecution$.withNewExecutionId(SQLExecution.scala:77)
at org.apache.spark.sql.Dataset.withAction(Dataset.scala:3276)
at org.apache.spark.sql.Dataset.&lt;init&gt;(Dataset.scala:190)
at org.apache.spark.sql.Dataset$.ofRows(Dataset.scala:75)
at org.apache.spark.sql.SparkSession.sql(SparkSession.scala:642)
at org.apache.spark.sql.SQLContext.sql(SQLContext.scala:694)
at org.apache.spark.sql.hive.thriftserver.SparkExecuteStatementOperation.org$apache$spark$sql$hive$thriftserver$SparkExecuteStatementOperation$$execute(SparkExecuteStatementOperation.scala:277)
... 11 more

How was this patch tested?

exist UT(AnalysisSuite)

…leading error message

maropu · 2019-07-01T04:27:49Z

ok to test

maropu · 2019-07-01T04:29:51Z

This issue only exists in INSERT command? Probably, you'd be better to make the title and description more concrete...

SparkQA · 2019-07-01T08:09:52Z

Test build #107059 has finished for PR 25019 at commit 9a32d8e.

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

liupc · 2019-07-01T09:44:32Z

I checked the command implementations, I'm not very sure, but it seems only affect the INSERT command, thank you! @maropu

maropu · 2019-07-02T01:31:01Z

Also, can you add some tests? This pr will change exception messages, right?

liupc · 2019-07-02T04:32:15Z

OK, I will add some test later.

SparkQA · 2019-07-02T13:06:25Z

Test build #107114 has finished for PR 25019 at commit ec711af.

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

maropu · 2019-07-05T02:01:50Z

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

+    val exception = intercept[AnalysisException](
+      analyzer.executeAndCheck(parser.parsePlan(query), new QueryPlanningTracker))
+    assert(exception.getMessage.contains("Table or view not found: table1"))
+  }


Can we move this into InsertSuite?

maropu · 2019-07-05T02:02:27Z

sql/core/src/main/scala/org/apache/spark/sql/internal/BaseSessionStateBuilder.scala

+        case e: InsertIntoDataSourceDirCommand => e.query
+        case _ => plan
+      }
+      super.checkAnalysis(planToCheck)


Can we resolve this issue in the InsertIntoDataSourceDirCommand side?

@maropu Yes, I agree that resolving this issue inside InsertIntoDataSourceDirCommand is better, but as I have already questioned is the jira: https://issues.apache.org/jira/browse/SPARK-28195, I'm not sure whether there are some special consideration for making the children of InsertIntoDataSourceDirCommand to empty and use innerChildren instead.

You meant this issue only happened when spark.sql.runSQLOnFiles=true? What if spark.sql.runSQLOnFiles=false?

@maropu This issue affect more aspects, what I point out in the jira is just for the case "table or view not found", actually, this issue may cause many other problems.
The root cause is that the the children of InsertIntoDataSourceDirCommand is empty, thus many analysis rules after InsertIntoDataSourceDirCommand being inserted(In DataSourceAnalysis rule) may not be effective and so was it for CheckAnalysis.
I think we can fix it better inside InsertIntoDataSourceDirCommand, but I should first make it clear that why we set it's children to empty, but use innerChildren instead? Is there any PR or issue for that?

I think you should investigate it by yourself. I don't like the current approach too. Appeartly issue is minor but the fix is pretty invasive. I doesn't need to touch SessionStateBuilder side at all.

AmplabJenkins · 2019-09-16T18:10:27Z

Can one of the admins verify this patch?

HyukjinKwon · 2019-09-16T23:52:48Z

Closing this due to inactivity from its author.

[SPARK-28195]Fix CheckAnalysis not working for command and report mis…

9a32d8e

…leading error message

maropu changed the title ~~[SPARK-28195]Fix CheckAnalysis not working for command and report misleading error message~~ [SPARK-28195][SQL] Fix CheckAnalysis not working for command and report misleading error message Jul 1, 2019

liupc changed the title ~~[SPARK-28195][SQL] Fix CheckAnalysis not working for command and report misleading error message~~ [SPARK-28195][SQL] Fix CheckAnalysis not working for insert command and report misleading error message Jul 1, 2019

liupc changed the title ~~[SPARK-28195][SQL] Fix CheckAnalysis not working for insert command and report misleading error message~~ [SPARK-28195][SQL] Fix CheckAnalysis not working for INSERT command and report misleading error message Jul 1, 2019

dongjoon-hyun added the SQL label Jul 1, 2019

Fix and add UT

ec711af

liupc changed the title ~~[SPARK-28195][SQL] Fix CheckAnalysis not working for INSERT command and report misleading error message~~ [SPARK-28195][SQL] Fix CheckAnalysis not working for InsertIntoDataSourceDirCommand and report misleading error message Jul 2, 2019

maropu reviewed Jul 5, 2019

View reviewed changes

HyukjinKwon closed this Sep 16, 2019

Conversation

liupc commented Jul 1, 2019 • 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!

maropu commented Jul 1, 2019

Uh oh!

maropu commented Jul 1, 2019

Uh oh!

SparkQA commented Jul 1, 2019

Uh oh!

liupc commented Jul 1, 2019

Uh oh!

maropu commented Jul 2, 2019

Uh oh!

liupc commented Jul 2, 2019

Uh oh!

SparkQA commented Jul 2, 2019

Uh oh!

maropu Jul 5, 2019

Choose a reason for hiding this comment

Uh oh!

liupc Jul 5, 2019

Choose a reason for hiding this comment

Uh oh!

maropu Jul 5, 2019

Choose a reason for hiding this comment

Uh oh!

liupc Jul 5, 2019

Choose a reason for hiding this comment

Uh oh!

maropu Jul 7, 2019

Choose a reason for hiding this comment

Uh oh!

liupc Jul 10, 2019 • 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!

HyukjinKwon Jul 19, 2019

Choose a reason for hiding this comment

Uh oh!

AmplabJenkins commented Sep 16, 2019

Uh oh!

HyukjinKwon commented Sep 16, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

liupc commented Jul 1, 2019 •

edited

Loading

liupc Jul 10, 2019 •

edited

Loading