[SPARK-16313][SQL][BRANCH-1.6] Spark should not silently drop exceptions in file listing #14139

yhuai · 2016-07-11T20:04:05Z

What changes were proposed in this pull request?

Spark silently drops exceptions during file listing. This is a very bad behavior because it can mask legitimate errors and the resulting plan will silently have 0 rows. This patch changes it to not silently drop the errors.

After making partition discovery not silently drop exceptions, HiveMetastoreCatalog can trigger partition discovery on empty tables, which cause FileNotFoundExceptions (these Exceptions were dropped by partition discovery silently). To address this issue, this PR introduces two hacks to workaround the issues. These two hacks try to avoid of triggering partition discovery on empty tables in HiveMetastoreCatalog.

How was this patch tested?

Manually tested.

Note: This is a backport of #13987

rxin · 2016-07-11T20:05:54Z

Wouldn't this create a ton of warnings during table creation?

yhuai · 2016-07-11T20:13:12Z

I think there will be one warning when we create a table. Or maybe there is no warning during table creation because the refresh is called lazily.

SparkQA · 2016-07-11T20:14:31Z

Test build #62116 has finished for PR 14139 at commit 674999d.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

yhuai · 2016-07-11T20:15:47Z

test this please

SparkQA · 2016-07-11T20:23:46Z

Test build #62117 has finished for PR 14139 at commit 674999d.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

yhuai · 2016-07-11T20:27:35Z

Let me see if we can have a flag to determine if we want to swallow the FNF (like what https://github.com/apache/spark/pull/13987/files does).

SparkQA · 2016-07-11T22:37:20Z

Test build #62121 has finished for PR 14139 at commit 308aef5.

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

SparkQA · 2016-07-12T00:18:24Z

Test build #62123 has finished for PR 14139 at commit 6708e9b.

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

SparkQA · 2016-07-12T04:14:41Z

Test build #62136 has finished for PR 14139 at commit b98ee09.

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

SparkQA · 2016-07-12T04:43:22Z

Test build #62142 has finished for PR 14139 at commit 5aaa96b.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

yhuai · 2016-07-12T15:55:15Z

test this please

yhuai · 2016-07-12T15:55:23Z

test this please

yhuai · 2016-07-12T15:55:31Z

test this please

yhuai · 2016-07-12T15:59:39Z

tes this please

SparkQA · 2016-07-12T16:06:48Z

Test build #3186 has finished for PR 14139 at commit 5aaa96b.

This patch fails to build.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2016-07-12T17:22:28Z

Test build #3187 has finished for PR 14139 at commit 5aaa96b.

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

SparkQA · 2016-07-12T17:47:24Z

Test build #62175 has finished for PR 14139 at commit 5aaa96b.

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

yhuai · 2016-07-12T18:55:17Z

cc @marmbrus

marmbrus · 2016-07-12T19:00:24Z

LGTM

yhuai · 2016-07-13T06:20:14Z

let me take another look to see if there is a better change.

yhuai · 2016-07-13T23:43:50Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala

+            false
+        }
+      case _ => false
+    }


This function is equivalent with val resolvedRelation = dataSource.resolveRelation(checkPathExist = false) in 2.0 (https://github.com/apache/spark/blob/master/sql/core/src/main/scala/org/apache/spark/sql/execution/command/createDataSourceTables.scala#L427).

yhuai · 2016-07-13T23:52:28Z

@rxin I think this version is the minimal change. Since the partition discovery logic in inside HadoopFsRelation in 1.6 and the refresh is triggered by using lazy val, passing a flag down will introduce lots of changes.

SparkQA · 2016-07-14T01:15:15Z

Test build #62287 has finished for PR 14139 at commit 5d66df7.

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

SparkQA · 2016-07-14T01:20:23Z

Test build #62288 has finished for PR 14139 at commit 82d3711.

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

rxin · 2016-07-14T16:54:48Z

sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala

@@ -273,6 +273,20 @@ private[hive] class HiveMetastoreCatalog(val client: ClientInterface, hive: Hive
        serdeProperties = options)
    }

+    def hasPartitionColumns(relation: HadoopFsRelation): Boolean = {
+      try {
+        // Calling hadoopFsRelation.partitionColumns will trigger the refresh call of


I'd add to the comment that this is a hack for [SPARK-16313][SQL][BRANCH-1.6] Spark should not silently drop exceptions in file listing

rxin · 2016-07-14T16:59:16Z

LGTM other than that.

SparkQA · 2016-07-14T18:39:42Z

Test build #62330 has finished for PR 14139 at commit 072aa3f.

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

yhuai · 2016-07-14T18:59:16Z

Thank you! I am merging this PR to branch 1.6.

…ons in file listing ## What changes were proposed in this pull request? Spark silently drops exceptions during file listing. This is a very bad behavior because it can mask legitimate errors and the resulting plan will silently have 0 rows. This patch changes it to not silently drop the errors. After making partition discovery not silently drop exceptions, HiveMetastoreCatalog can trigger partition discovery on empty tables, which cause FileNotFoundExceptions (these Exceptions were dropped by partition discovery silently). To address this issue, this PR introduces two **hacks** to workaround the issues. These two hacks try to avoid of triggering partition discovery on empty tables in HiveMetastoreCatalog. ## How was this patch tested? Manually tested. **Note: This is a backport of #13987 Author: Yin Huai <yhuai@databricks.com> Closes #14139 from yhuai/SPARK-16313-branch-1.6.

…ons in file listing ## What changes were proposed in this pull request? Spark silently drops exceptions during file listing. This is a very bad behavior because it can mask legitimate errors and the resulting plan will silently have 0 rows. This patch changes it to not silently drop the errors. After making partition discovery not silently drop exceptions, HiveMetastoreCatalog can trigger partition discovery on empty tables, which cause FileNotFoundExceptions (these Exceptions were dropped by partition discovery silently). To address this issue, this PR introduces two **hacks** to workaround the issues. These two hacks try to avoid of triggering partition discovery on empty tables in HiveMetastoreCatalog. ## How was this patch tested? Manually tested. **Note: This is a backport of apache#13987 Author: Yin Huai <yhuai@databricks.com> Closes apache#14139 from yhuai/SPARK-16313-branch-1.6. (cherry picked from commit 6ea7d4b)

yhuai added 2 commits July 11, 2016 18:44

Spark should not silently drop exceptions in file listing

a0387ad

Fix tests

b98ee09

Comments and format

5aaa96b

More comments

5d66df7

yhuai reviewed Jul 13, 2016
View reviewed changes

Update

82d3711

rxin reviewed Jul 14, 2016
View reviewed changes

Make it clear that it is a hack

af5b7b3

Add pr link

072aa3f

yhuai closed this Jul 14, 2016

[SPARK-16313][SQL][BRANCH-1.6] Spark should not silently drop exceptions in file listing #14139

[SPARK-16313][SQL][BRANCH-1.6] Spark should not silently drop exceptions in file listing #14139

Uh oh!

Conversation

yhuai commented Jul 11, 2016 • 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!

rxin commented Jul 11, 2016

Uh oh!

yhuai commented Jul 11, 2016

Uh oh!

SparkQA commented Jul 11, 2016

Uh oh!

yhuai commented Jul 11, 2016

Uh oh!

SparkQA commented Jul 11, 2016

Uh oh!

yhuai commented Jul 11, 2016

Uh oh!

SparkQA commented Jul 11, 2016

Uh oh!

SparkQA commented Jul 12, 2016

Uh oh!

SparkQA commented Jul 12, 2016

Uh oh!

SparkQA commented Jul 12, 2016

Uh oh!

yhuai commented Jul 12, 2016

Uh oh!

yhuai commented Jul 12, 2016

Uh oh!

yhuai commented Jul 12, 2016

Uh oh!

yhuai commented Jul 12, 2016

Uh oh!

SparkQA commented Jul 12, 2016

Uh oh!

SparkQA commented Jul 12, 2016

Uh oh!

SparkQA commented Jul 12, 2016

Uh oh!

yhuai commented Jul 12, 2016

Uh oh!

marmbrus commented Jul 12, 2016

Uh oh!

yhuai commented Jul 13, 2016

Uh oh!

yhuai Jul 13, 2016

Choose a reason for hiding this comment

Uh oh!

yhuai commented Jul 13, 2016

Uh oh!

SparkQA commented Jul 14, 2016

Uh oh!

SparkQA commented Jul 14, 2016

Uh oh!

rxin Jul 14, 2016

Choose a reason for hiding this comment

Uh oh!

yhuai Jul 14, 2016

Choose a reason for hiding this comment

Uh oh!

rxin commented Jul 14, 2016

Uh oh!

SparkQA commented Jul 14, 2016

Uh oh!

yhuai commented Jul 14, 2016

Uh oh!

Uh oh!

yhuai commented Jul 11, 2016 •

edited

Loading