[SPARK-11102] [SQL] Uninformative exception when specifing non-exist

[zjffdu]

Rebase the code base and create another PR, and close the previous PR #9223
Paste comment from last PR here to give more context

The JsonRelation has another special case that it allow input paths as empty when it takes RDD[String] as input. So HadoopFsRelation can not assume all its sub classes must have non-empty input paths. Maybe need to add another flag "allowEmptyInputPaths" in HadoopFsRelation and allow its implementation to decide that.

[srowen]

CC @chenghao-intel

[SparkQA]

Test build #45111 has finished for PR 9490 at commit 7a14b8c.

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

[rxin]

cc @liancheng

[SparkQA]

Test build #45178 has finished for PR 9490 at commit 63ebcdf.

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

[zjffdu]

Looks like the failed test is not related. @liancheng Please help review it. Thanks

[marmbrus]

test this please

[SparkQA]

Test build #45500 has finished for PR 9490 at commit 63ebcdf.

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

[SparkQA]

Test build #45590 has finished for PR 9490 at commit 8f7ebc6.

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

[SparkQA]

Test build #45606 has finished for PR 9490 at commit 219db87.

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

[zjffdu]

Comments for the change:
The case of empty or non-exist inputs is a little tricky. Here's the several cases I summarize

• Only parse the inputs at execution stage. e.g. TextRelation
• Need parse the inputs at analysis stage. e.g. JsonRelation, ParquetRelation & OrcRelation
• Don't need to parse the inputs if the schema is provided. (when creating table) e.g. ParquetRelation & OrcRelation
• Empty is also valid. e.g. JsonRelation can accept RDD[String] rather than from hdfs
• Empty inputs is valid for creating table.

So for these cases, I do the following changes

• Add 2 api in HadoopFsRelation. sub classes can override it. Now only JsonRelation will override it.
  ** def inputExists: Boolean = fileStatusCache.inputExists
  ** def readFromHDFS: Boolean = true
• If the inputs are only empty directories, it should be valid, just return EmptyRDD
• If the inputs are not-existed directories/files, it is invalid, just throw exception.
• If it needs to parse data in the analysis stage, it is sub classes' responsibility to check whether inputs is empty or not. Parent class (HadoopFsRelation) only check the inputs at execution stage.

[liancheng]

This can be quite expensive since each fs.exists(qualified) call invokes FileSystem.getFileStatus(), which is an RPC call. On the other hand, we've already called fs.listStatus(qualified) below. Would be better to merge these two.

[liancheng]

Another issue is that this block doesn't handle the case of parallel file listing (the other if block above).

[liancheng]

Hey @zjffdu, are you still working on this?

[zjffdu]

@liancheng sorry for late response, will try to make a new commit this weekend

[SparkQA]

Test build #46809 has finished for PR 9490 at commit 9dd1801.

• This patch fails Scala style tests.
• This patch does not merge cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #46810 has finished for PR 9490 at commit d8e7f5c.

• This patch passes all tests.
• This patch does not merge cleanly.
• This patch adds no public classes.

[SparkQA]

Test build #46821 has finished for PR 9490 at commit 9e67a3d.

• This patch passes all tests.
• This patch does not merge cleanly.
• This patch adds no public classes.

[yhuai]

@zjffdu I feel it is not a good idea to change lots of code just to get a better error message. Is it possible to have a small change to achieve the goal?

[zjffdu]

@yhuai Only changing JSONRelation/TextRelation can be a short-term solution. But since this is a general issue for HadoopFsRelation. so it would be better to do it in HadoopFsRelation. Otherwise we may meet same issue when we have new HadoopFsRelation implemention or user create a custom HadoopFsRelation.

[zjffdu]

@yhuai @liancheng any more comments ?

[yhuai]

@zjffdu How about we revisit it after we release 1.6?

[zjffdu]

@yhuai Sure, np

[srowen]

Please close this PR

[HyukjinKwon]

ping @zjffdu