[SPARK-25993][SQL][TESTS] Add test cases for CREATE EXTERNAL TABLE with subdirectories #27130

kevinyu98 · 2020-01-08T07:01:18Z

What changes were proposed in this pull request?

This PR aims to add these test cases for resolution of ORC table location reported by SPARK-25993
also add corresponding test cases for Parquet table.

Why are the changes needed?

The current behavior is complex, this test case suites are designed to prevent the accidental behavior change. This pr is rebased on master, the original pr is 23108

Does this PR introduce any user-facing change?

No. This adds test cases only.

How was this patch tested?

This is a new test case.

HyukjinKwon · 2020-01-08T11:59:17Z

ok to test

SparkQA · 2020-01-08T14:29:16Z

Test build #116300 has finished for PR 27130 at commit 9722230.

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

dongjoon-hyun · 2020-01-08T17:42:01Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveParquetSourceSuite.scala

+                 |LOCATION '${s"${path.getCanonicalPath}"}'""".stripMargin
+            sql(topDirStatement)
+            if (parquetConversion == "true") {
+              checkAnswer(sql("select * from tbl1"), Nil)


Shall we capitalize the SQL statement like SELECT * FROM tbl1?

dongjoon-hyun · 2020-01-08T17:42:36Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveParquetSourceSuite.scala

+            if (parquetConversion == "true") {
+              checkAnswer(sql("select * from tbl1"), Nil)
+            } else {
+              intercept[IOException](sql("select * from tbl1").show())


We had better check the exception message.

dongjoon-hyun · 2020-01-08T17:43:35Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveParquetSourceSuite.scala

+                 |LOCATION '${s"${path.getCanonicalPath}/l1/"}'""".stripMargin
+            sql(l1DirStatement)
+            if (parquetConversion == "true") {
+              checkAnswer(sql("select * from tbl2"),


SELECT * FROM tbl2.

dongjoon-hyun · 2020-01-08T17:43:47Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveParquetSourceSuite.scala

+              checkAnswer(sql("select * from tbl2"),
+                (1 to 2).map(i => Row(i, i, s"parq$i")))
+            } else {
+              intercept[IOException](sql("select * from tbl2").show())


Please check the exception message.

the whole exception message is
Not a file: file:/Users/qianyangyu/IdeaProjects/spark/target/tmp/spark-abc8c1ad-4a3a-420f-b4fc-58d995be9bb0/l1, I will check the first part Not a file:.

dongjoon-hyun · 2020-01-08T17:44:01Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveParquetSourceSuite.scala

+                 |LOCATION '${s"${path.getCanonicalPath}/l1/l2/"}'""".stripMargin
+            sql(l2DirStatement)
+            if (parquetConversion == "true") {
+              checkAnswer(sql("select * from tbl3"),


SELECT * FROM tbl3.

dongjoon-hyun · 2020-01-08T17:44:22Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveParquetSourceSuite.scala

+              checkAnswer(sql("select * from tbl3"),
+                (3 to 4).map(i => Row(i, i, s"parq$i")))
+            } else {
+              intercept[IOException](sql("select * from tbl3").show())


Please check the exception message.

added the checking

dongjoon-hyun · 2020-01-08T17:47:56Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/orc/HiveOrcSourceSuite.scala

+        withTempDir { dir =>
+          try {
+            hiveClient.runSqlHive("USE default")
+            hiveClient.runSqlHive(


Do we need to use runSqlHive?

sure, I will change to sql.

dongjoon-hyun

Thank you for keeping working on this. Sorry for your long waiting. At this time, I hope we can merge your PR.

SparkQA · 2020-01-10T01:25:09Z

Test build #116421 has finished for PR 27130 at commit 9bc32ab.

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

dongjoon-hyun · 2020-01-10T07:50:57Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveParquetSourceSuite.scala

+            if (parquetConversion == "true") {
+              checkAnswer(sql("SELECT * FROM tbl1"), Nil)
+            } else {
+              val msg = intercept[IOException] {sql("SELECT * FROM tbl1").show()


sql("SELECT * FROM tbl1").show() seems to need to be in the next line.

thanks, changed.

dongjoon-hyun · 2020-01-10T07:51:21Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveParquetSourceSuite.scala

+            } else {
+              val msg = intercept[IOException] {sql("SELECT * FROM tbl1").show()
+              }.getMessage
+                assert(msg.contains("Not a file:"))


indentation?

dongjoon-hyun · 2020-01-10T07:52:06Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveParquetSourceSuite.scala

+            sql(l1DirStatement)
+            if (parquetConversion == "true") {
+              checkAnswer(sql("SELECT * FROM tbl2"),
+                (1 to 2).map(i => Row(i, i, s"parq$i")))


Can we merge 269 and 270 into one line here?

dongjoon-hyun · 2020-01-10T07:52:25Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveParquetSourceSuite.scala

+              checkAnswer(sql("SELECT * FROM tbl3"),
+                (3 to 4).map(i => Row(i, i, s"parq$i")))
+            } else {
+              val msg = intercept[IOException] {sql("SELECT * FROM tbl3").show()


sql("SELECT * FROM tbl3").show()?

moved to next line.

dongjoon-hyun · 2020-01-10T07:52:48Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveParquetSourceSuite.scala

+              checkAnswer(sql("SELECT * FROM tbl5"),
+                (1 to 4).map(i => Row(i, i, s"parq$i")))
+            } else {
+              val msg = intercept[IOException] {sql("SELECT * FROM tbl5").show()


dongjoon-hyun · 2020-01-10T07:58:08Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/orc/HiveOrcSourceSuite.scala

+            sql("USE default")
+            sql(
+              """
+                |CREATE EXTERNAL TABLE hive_orc(


I'm a little confused here.
@kevinyu98 . Do you want to get a table created by Hive here?
Usually, we use the table name, hive_orc, for that table. Please see https://github.com/apache/spark/pull/27130/files#diff-a8c26a35def87a13e6b59db19d9fb8a1R68 .

And, you still using hiveClient.runSqlHive at line 192. I'm wondering what is the test target in this PR~.

@dongjoon-hyun Thanks for pointing out this. I was using other test cases without thinking too much. I have changed the name. I also replaced the hiveClient.runSqlHive for the insert stmt.

SparkQA · 2020-01-12T08:05:02Z

Test build #116548 has finished for PR 27130 at commit 2cf3e26.

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

kevinyu98 · 2020-01-13T01:55:03Z

@dongjoon-hyun can we re-test it? I ran HiveParquetSourceSuite and HiveOrcSourceSuite locally, it works fine. thanks.

dilipbiswal · 2020-01-13T19:36:46Z

retest this please

SparkQA · 2020-01-13T21:54:01Z

Test build #116657 has finished for PR 27130 at commit 2cf3e26.

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

dongjoon-hyun

Hi, @kevinyu98 .
I agree with HiveOrcSourceSuite.scala. Could you update HiveParquetSourceSuite.scala like HiveOrcSourceSuite.scala? There is no need to be different.

kevinyu98 · 2020-01-15T18:16:56Z

@dongjoon-hyun sure, thanks.

dongjoon-hyun · 2020-01-17T06:29:32Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/orc/HiveOrcSourceSuite.scala

+              val l1DirSqlStatement = s"SELECT * FROM tbl2"
+              if (convertMetastore) {
+                checkAnswer(sql(l1DirSqlStatement),
+                  (1 to 2).map(i => Row(i, i, s"orc$i")))


Let's make line 250 and 251 as one line like HiveParquetSourceSuite.

dongjoon-hyun · 2020-01-17T06:29:43Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/orc/HiveOrcSourceSuite.scala

+                  (1 to 2).map(i => Row(i, i, s"orc$i")))
+              } else {
+                checkAnswer(sql(l1DirSqlStatement),
+                  (1 to 6).map(i => Row(i, i, s"orc$i")))


dongjoon-hyun · 2020-01-17T06:29:53Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/orc/HiveOrcSourceSuite.scala

+              val l2DirSqlStatement = s"SELECT * FROM tbl3"
+              if (convertMetastore) {
+                checkAnswer(sql(l2DirSqlStatement),
+                  (3 to 4).map(i => Row(i, i, s"orc$i")))


dongjoon-hyun · 2020-01-17T06:29:59Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/orc/HiveOrcSourceSuite.scala

+                  (3 to 4).map(i => Row(i, i, s"orc$i")))
+              } else {
+                checkAnswer(sql(l2DirSqlStatement),
+                  (3 to 6).map(i => Row(i, i, s"orc$i")))


dongjoon-hyun · 2020-01-17T06:30:07Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/orc/HiveOrcSourceSuite.scala

+              val wildcardTopDirSqlStatement = s"SELECT * FROM tbl4"
+              if (convertMetastore) {
+                checkAnswer(sql(wildcardTopDirSqlStatement),
+                  (1 to 2).map(i => Row(i, i, s"orc$i")))


dongjoon-hyun · 2020-01-17T06:30:17Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/orc/HiveOrcSourceSuite.scala

+              val wildcardL1DirSqlStatement = s"SELECT * FROM tbl5"
+              if (convertMetastore) {
+                checkAnswer(sql(wildcardL1DirSqlStatement),
+                  (1 to 4).map(i => Row(i, i, s"orc$i")))


dongjoon-hyun · 2020-01-17T06:30:24Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/orc/HiveOrcSourceSuite.scala

+              val wildcardL2SqlStatement = s"SELECT * FROM tbl6"
+              if (convertMetastore) {
+                checkAnswer(sql(wildcardL2SqlStatement),
+                  (3 to 6).map(i => Row(i, i, s"orc$i")))


dongjoon-hyun · 2020-01-17T06:32:05Z

sql/hive/src/test/scala/org/apache/spark/sql/hive/orc/HiveOrcSourceSuite.scala

+                checkAnswer(sql(wildcardL2SqlStatement), Nil)
+              }
+            }
+          } finally {


Shall we convert this try..finally with withTable like HiveParquetSourceSuite?

SparkQA · 2020-01-17T07:33:47Z

Test build #116894 has finished for PR 27130 at commit 0bb628f.

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

SparkQA · 2020-01-18T01:02:32Z

Test build #116967 has finished for PR 27130 at commit 39f271f.

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

dongjoon-hyun

+1, LGTM. Thank you, @kevinyu98 .
Merged to master.

rebase on spark 3.0

9722230

kevinyu98 mentioned this pull request Jan 8, 2020

[Spark-25993][SQL][TEST]Add test cases for CREATE EXTERNAL TABLE with subdirectories #23108

Closed

HyukjinKwon changed the title ~~[Spark-25993][SQL][TEST]Add test cases for CREATE EXTERNAL TABLE with subdirectories~~ [SPARK-25993][SQL][TESTS] Add test cases for CREATE EXTERNAL TABLE with subdirectories Jan 8, 2020

dongjoon-hyun added SQL TESTS labels Jan 8, 2020

dongjoon-hyun reviewed Jan 8, 2020

View reviewed changes

address comments

9bc32ab

dongjoon-hyun reviewed Jan 10, 2020

View reviewed changes

address comments

2cf3e26

dongjoon-hyun requested changes Jan 14, 2020

View reviewed changes

address comments

0bb628f

dongjoon-hyun reviewed Jan 17, 2020

View reviewed changes

address comments

39f271f

dongjoon-hyun approved these changes Jan 18, 2020

View reviewed changes

dongjoon-hyun closed this in 96a3445 Jan 18, 2020

[SPARK-25993][SQL][TESTS] Add test cases for CREATE EXTERNAL TABLE with subdirectories #27130

[SPARK-25993][SQL][TESTS] Add test cases for CREATE EXTERNAL TABLE with subdirectories #27130

Conversation

kevinyu98 commented Jan 8, 2020 • edited by dongjoon-hyun

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

HyukjinKwon commented Jan 8, 2020

SparkQA commented Jan 8, 2020

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

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

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

dongjoon-hyun left a comment

Choose a reason for hiding this comment

SparkQA commented Jan 10, 2020

dongjoon-hyun Jan 10, 2020 • edited

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

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

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

SparkQA commented Jan 12, 2020

kevinyu98 commented Jan 13, 2020

dilipbiswal commented Jan 13, 2020

SparkQA commented Jan 13, 2020

dongjoon-hyun left a comment

Choose a reason for hiding this comment

kevinyu98 commented Jan 15, 2020

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

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

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

SparkQA commented Jan 17, 2020

SparkQA commented Jan 18, 2020

dongjoon-hyun left a comment

Choose a reason for hiding this comment

kevinyu98 commented Jan 8, 2020 •

edited by dongjoon-hyun

dongjoon-hyun Jan 10, 2020 •

edited