[SPARK-23305][SQL][TEST] Test `spark.sql.files.ignoreMissingFiles` for all file-based data sources #20479

dongjoon-hyun · 2018-02-01T18:26:29Z

What changes were proposed in this pull request?

Like Parquet, all file-based data source handles spark.sql.files.ignoreMissingFiles correctly. We had better have a test coverage for feature parity and in order to prevent future accidental regression for all data sources.

How was this patch tested?

Pass Jenkins with a newly added test case.

…t case for ORC

SparkQA · 2018-02-01T21:28:56Z

Test build #86940 has finished for PR 20479 at commit 7fdfc35.

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

HyukjinKwon · 2018-02-02T00:22:53Z

sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcQuerySuite.scala

+          new Path(basePath, "third").toString)
+
+        val fs = thirdPath.getFileSystem(spark.sparkContext.hadoopConfiguration)
+        fs.delete(thirdPath, true)


Shall we assert true?

HyukjinKwon

LGTM

dongjoon-hyun · 2018-02-02T01:24:13Z

Thank you for review and approval!

viirya · 2018-02-02T01:35:45Z

LGTM

SparkQA · 2018-02-02T04:21:48Z

Test build #86955 has finished for PR 20479 at commit c2891d0.

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

gatorsmile · 2018-02-02T04:43:17Z

sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcQuerySuite.scala

@@ -655,4 +655,35 @@ class OrcQuerySuite extends OrcQueryTest with SharedSQLContext {
      }
    }
  }
+
+  testQuietly("Enabling/disabling ignoreMissingFiles") {


Basically, this is copied from Parquet. To avoid duplicate codes, create a common base test class for parquet and orc? Then, we can deduplicate the codes?

+1. Which suite is proper for that base test class?

dongjoon-hyun · 2018-02-02T07:07:53Z

@gatorsmile . Can we do the refactoring later since we have still many things to do for RC3?
The main purpose is preventing any regression since Spark 2.3.0.

dongjoon-hyun · 2018-02-02T07:36:43Z

Ah, I moved both of them into FileBasedDataSourceSuite.

dongjoon-hyun · 2018-02-02T07:38:26Z

Thank you for review, @HyukjinKwon , @viirya , and @gatorsmile .
Now, there is no redundancy.

HyukjinKwon · 2018-02-02T07:43:37Z

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

@@ -92,4 +96,39 @@ class FileBasedDataSourceSuite extends QueryTest with SharedSQLContext {
      }
    }
  }
+
+  // Only ORC/Parquet support this.
+  Seq("orc", "parquet").foreach { format =>


Ah, yeah. This sounds better.

HyukjinKwon · 2018-02-02T07:56:02Z

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

@@ -92,4 +96,39 @@ class FileBasedDataSourceSuite extends QueryTest with SharedSQLContext {
      }
    }
  }
+
+  // Only ORC/Parquet support this.


Wait .. don't other sources support this option?

Ur, let me check back.

gatorsmile · 2018-02-02T08:03:22Z

For test-only PRs, we still can merge it to 2.3

SparkQA · 2018-02-02T08:05:02Z

Test build #86976 has finished for PR 20479 at commit 4512d72.

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

dongjoon-hyun · 2018-02-02T08:10:10Z

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

@@ -92,4 +96,37 @@ class FileBasedDataSourceSuite extends QueryTest with SharedSQLContext {
      }
    }
  }
+
+  allFileBasedDataSources.foreach { format =>


Thank you so much, @HyukjinKwon !

dongjoon-hyun · 2018-02-02T08:14:35Z

I updated the JIRA issue and PR descriptions to mention all file-based data sources.

SparkQA · 2018-02-02T11:18:16Z

Test build #86982 has finished for PR 20479 at commit 9a2f640.

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

dongjoon-hyun · 2018-02-03T02:19:01Z

Retest this please.

SparkQA · 2018-02-03T04:37:26Z

Test build #87019 has finished for PR 20479 at commit 9a2f640.

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

gatorsmile · 2018-02-03T04:40:30Z

retest this please

SparkQA · 2018-02-03T07:48:19Z

Test build #87022 has finished for PR 20479 at commit 9a2f640.

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

gatorsmile · 2018-02-03T08:03:21Z

Thanks! Merged to master/2.3

…r all file-based data sources ## What changes were proposed in this pull request? Like Parquet, all file-based data source handles `spark.sql.files.ignoreMissingFiles` correctly. We had better have a test coverage for feature parity and in order to prevent future accidental regression for all data sources. ## How was this patch tested? Pass Jenkins with a newly added test case. Author: Dongjoon Hyun <dongjoon@apache.org> Closes #20479 from dongjoon-hyun/SPARK-23305. (cherry picked from commit 522e0b1) Signed-off-by: gatorsmile <gatorsmile@gmail.com>

dongjoon-hyun · 2018-02-03T16:33:35Z

Thank you, @HyukjinKwon and @gatorsmile .

[SPARK-23305][SQL][TEST] Add spark.sql.files.ignoreMissingFiles tes…

7fdfc35

…t case for ORC

HyukjinKwon reviewed Feb 2, 2018

View reviewed changes

HyukjinKwon approved these changes Feb 2, 2018

View reviewed changes

Address comments.

c2891d0

gatorsmile reviewed Feb 2, 2018

View reviewed changes

Move to FileBasedDataSourceSuite

4512d72

HyukjinKwon reviewed Feb 2, 2018

View reviewed changes

Fix to test all.

9a2f640

dongjoon-hyun commented Feb 2, 2018

View reviewed changes

dongjoon-hyun changed the title ~~[SPARK-23305][SQL][TEST] Add spark.sql.files.ignoreMissingFiles test case for ORC~~ [SPARK-23305][SQL][TEST] Add spark.sql.files.ignoreMissingFiles test case Feb 2, 2018

dongjoon-hyun changed the title ~~[SPARK-23305][SQL][TEST] Add spark.sql.files.ignoreMissingFiles test case~~ [SPARK-23305][SQL][TEST] Test spark.sql.files.ignoreMissingFiles for all data sources Feb 2, 2018

dongjoon-hyun changed the title ~~[SPARK-23305][SQL][TEST] Test spark.sql.files.ignoreMissingFiles for all data sources~~ [SPARK-23305][SQL][TEST] Test spark.sql.files.ignoreMissingFiles for all file-based data sources Feb 2, 2018

asfgit closed this in 522e0b1 Feb 3, 2018

dongjoon-hyun deleted the SPARK-23305 branch February 3, 2018 16:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-23305][SQL][TEST] Test `spark.sql.files.ignoreMissingFiles` for all file-based data sources #20479

[SPARK-23305][SQL][TEST] Test `spark.sql.files.ignoreMissingFiles` for all file-based data sources #20479

dongjoon-hyun commented Feb 1, 2018 •

edited

Loading

SparkQA commented Feb 1, 2018

HyukjinKwon Feb 2, 2018

dongjoon-hyun Feb 2, 2018

HyukjinKwon left a comment

dongjoon-hyun commented Feb 2, 2018

viirya commented Feb 2, 2018

SparkQA commented Feb 2, 2018

gatorsmile Feb 2, 2018

dongjoon-hyun Feb 2, 2018

dongjoon-hyun commented Feb 2, 2018 •

edited

Loading

dongjoon-hyun commented Feb 2, 2018

dongjoon-hyun commented Feb 2, 2018

HyukjinKwon Feb 2, 2018

dongjoon-hyun Feb 2, 2018

HyukjinKwon Feb 2, 2018

dongjoon-hyun Feb 2, 2018

gatorsmile commented Feb 2, 2018

SparkQA commented Feb 2, 2018

dongjoon-hyun Feb 2, 2018

dongjoon-hyun commented Feb 2, 2018

SparkQA commented Feb 2, 2018

dongjoon-hyun commented Feb 3, 2018

SparkQA commented Feb 3, 2018

gatorsmile commented Feb 3, 2018

SparkQA commented Feb 3, 2018

gatorsmile commented Feb 3, 2018

dongjoon-hyun commented Feb 3, 2018

[SPARK-23305][SQL][TEST] Test spark.sql.files.ignoreMissingFiles for all file-based data sources #20479

[SPARK-23305][SQL][TEST] Test spark.sql.files.ignoreMissingFiles for all file-based data sources #20479

Conversation

dongjoon-hyun commented Feb 1, 2018 • edited Loading

What changes were proposed in this pull request?

How was this patch tested?

SparkQA commented Feb 1, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

HyukjinKwon left a comment

Choose a reason for hiding this comment

dongjoon-hyun commented Feb 2, 2018

viirya commented Feb 2, 2018

SparkQA commented Feb 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dongjoon-hyun commented Feb 2, 2018 • edited Loading

dongjoon-hyun commented Feb 2, 2018

dongjoon-hyun commented Feb 2, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

gatorsmile commented Feb 2, 2018

SparkQA commented Feb 2, 2018

Choose a reason for hiding this comment

dongjoon-hyun commented Feb 2, 2018

SparkQA commented Feb 2, 2018

dongjoon-hyun commented Feb 3, 2018

SparkQA commented Feb 3, 2018

gatorsmile commented Feb 3, 2018

SparkQA commented Feb 3, 2018

gatorsmile commented Feb 3, 2018

dongjoon-hyun commented Feb 3, 2018

[SPARK-23305][SQL][TEST] Test `spark.sql.files.ignoreMissingFiles` for all file-based data sources #20479

[SPARK-23305][SQL][TEST] Test `spark.sql.files.ignoreMissingFiles` for all file-based data sources #20479

dongjoon-hyun commented Feb 1, 2018 •

edited

Loading

dongjoon-hyun commented Feb 2, 2018 •

edited

Loading