You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by kevinyu98 <gi...@git.apache.org> on 2018/11/21 19:48:52 UTC
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for resolu...
GitHub user kevinyu98 opened a pull request:
https://github.com/apache/spark/pull/23108
[Spark-25993][SQL][TEST]Add test cases for resolution of ORC table location
## What changes were proposed in this pull request?
Add these test cases for resolution of ORC table location reported by [SPARK-25993](https://issues.apache.org/jira/browse/SPARK-25993)
Update the
> sql-migration-guide-update
doc
## How was this patch tested?
This is a new test case.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/kevinyu98/spark spark-25993
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/23108.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #23108
----
commit 4e45ef90fba26b34bd4d9b575b6bf793d0500fdc
Author: Kevin Yu <qy...@...>
Date: 2018-11-21T16:28:41Z
add test case for orc table location
commit e238764f278883b05d4bc88243facf897d357e84
Author: Kevin Yu <qy...@...>
Date: 2018-11-21T19:43:47Z
doc the change in migration-guide
----
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5679/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99804/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99746/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5331/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99251 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99251/testReport)** for PR 23108 at commit [`d6e582b`](https://github.com/apache/spark/commit/d6e582b3ff33f767d41c9c7cf1710107d7901e0f).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5324/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99235 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99235/testReport)** for PR 23108 at commit [`e731746`](https://github.com/apache/spark/commit/e731746da4643d0283b3cd788d286aea62c96215).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for resolu...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r236835472
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcSourceSuite.scala ---
@@ -186,6 +186,54 @@ abstract class OrcSuite extends OrcTest with BeforeAndAfterAll {
}
}
+ protected def testORCTableLocation(isConvertMetastore: Boolean): Unit = {
+ val tableName1 = "spark_orc1"
+ val tableName2 = "spark_orc2"
+
+ withTempDir { dir =>
+ val someDF1 = Seq((1, 1, "orc1"), (2, 2, "orc2")).toDF("c1", "c2", "c3").repartition(1)
+ withTable(tableName1, tableName2) {
+ val dataDir = s"${dir.getCanonicalPath}/dir1/"
+ val parentDir = s"${dir.getCanonicalPath}/"
+ val wildCardDir = new File(s"${dir}/*").toURI
+ someDF1.write.orc(dataDir)
+ val parentDirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE $tableName1(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '${parentDir}'""".stripMargin
+ sql(parentDirStatement)
+ val parentDirSqlStatement = s"select * from ${tableName1}"
+ if (isConvertMetastore) {
+ checkAnswer(sql(parentDirSqlStatement), Nil)
+ } else {
+ checkAnswer(sql(parentDirSqlStatement),
+ (1 to 2).map(i => Row(i, i, s"orc$i")))
+ }
+
+ val wildCardStatement =
+ s"""
+ |CREATE EXTERNAL TABLE $tableName2(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '$wildCardDir'""".stripMargin
--- End diff --
@kevinyu98 . This works, but there is a side effect with this. I mean this creates additional directory who name is '*' literally.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5338/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r239364631
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveParquetSourceSuite.scala ---
@@ -222,4 +223,66 @@ class HiveParquetSourceSuite extends ParquetPartitioningTest {
assert(df4.columns === Array("str", "max_int"))
}
}
+
+ test("SPARK-25993 CREATE EXTERNAL TABLE with subdirectories") {
+ withTempPath { path =>
+ withTable("tbl1", "tbl2", "tbl3") {
+ val someDF1 = Seq((1, 1, "parq1"), (2, 2, "parq2")).
+ toDF("c1", "c2", "c3").repartition(1)
+ val dataDir = s"${path.getCanonicalPath}/l3/l2/l1/"
+ val parentDir = s"${path.getCanonicalPath}/l3/l2/"
+ val wildcardParentDir = new File(s"${path}/l3/l2/*").toURI
+ val wildcardL3Dir = new File(s"${path}/l3/*").toURI
+ someDF1.write.parquet(dataDir)
+ val parentDirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl1(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS parquet
+ |LOCATION '${parentDir}'""".stripMargin
+ sql(parentDirStatement)
+ val wildcardStatement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl2(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS parquet
+ |LOCATION '${wildcardParentDir}'""".stripMargin
+ sql(wildcardStatement)
+ val wildcardL3Statement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl3(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS parquet
+ |LOCATION '${wildcardL3Dir}'""".stripMargin
+ sql(wildcardL3Statement)
+
+ Seq("true", "false").foreach { parquetConversion =>
+ withSQLConf(HiveUtils.CONVERT_METASTORE_PARQUET.key -> parquetConversion) {
+ if (parquetConversion == "true") {
+ checkAnswer(sql("select * from tbl1"), Nil)
+ checkAnswer(sql("select * from tbl2"),
+ (1 to 2).map(i => Row(i, i, s"parq$i")))
+ checkAnswer(sql("select * from tbl3"), Nil)
+ } else {
+ Seq("select * from tbl1", "select * from tbl2", "select * from tbl3").foreach {
+ sqlStmt =>
+ try {
+ sql(sqlStmt)
+ } catch {
+ case e: IOException =>
+ assert(e.getMessage().contains("java.io.IOException: Not a file"))
+ }
--- End diff --
@kevinyu98 . Is this testing exceptions for the above all three SQLs? We use `intercept[IOException]` to test expected `Exception`s.
However, this is not a robust test case, because there is no `assert(false)` after `sql(sqlStmt)`. We need to check the individual query failure and success exactly for the specific configuration.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r239580991
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveParquetSourceSuite.scala ---
@@ -32,6 +32,7 @@ import org.apache.spark.util.Utils
class HiveParquetSourceSuite extends ParquetPartitioningTest {
import testImplicits._
import spark._
+ import java.io.IOException
--- End diff --
ok
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r239635890
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/orc/HiveOrcSourceSuite.scala ---
@@ -190,4 +192,103 @@ class HiveOrcSourceSuite extends OrcSuite with TestHiveSingleton {
}
}
}
+
+ test("SPARK-25993 CREATE EXTERNAL TABLE with subdirectories") {
+ Seq(true, false).foreach { convertMetastore =>
+ withSQLConf(HiveUtils.CONVERT_METASTORE_ORC.key -> s"$convertMetastore") {
+ withTempDir { dir =>
+ val dataDir = new File(s"${dir.getCanonicalPath}/l3/l2/l1/").toURI
+ val parentDir = s"${dir.getCanonicalPath}/l3/l2/"
+ val l3Dir = s"${dir.getCanonicalPath}/l3/"
+ val wildcardParentDir = new File(s"${dir}/l3/l2/*").toURI
+ val wildcardL3Dir = new File(s"${dir}/l3/*").toURI
+
+ try {
+ hiveClient.runSqlHive("USE default")
+ hiveClient.runSqlHive(
+ """
+ |CREATE EXTERNAL TABLE hive_orc(
+ | C1 INT,
+ | C2 INT,
+ | C3 STRING)
+ |STORED AS orc""".stripMargin)
+ // Hive throws an exception if I assign the location in the create table statement.
+ hiveClient.runSqlHive(
+ s"ALTER TABLE hive_orc SET LOCATION '$dataDir'")
+ hiveClient.runSqlHive(
+ """
+ |INSERT INTO TABLE hive_orc
+ |VALUES (1, 1, 'orc1'), (2, 2, 'orc2')""".stripMargin)
+
+ withTable("tbl1", "tbl2", "tbl3", "tbl4") {
+ val parentDirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl1(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '${parentDir}'""".stripMargin
+ sql(parentDirStatement)
+ val parentDirSqlStatement = s"select * from tbl1"
+ if (convertMetastore) {
+ checkAnswer(sql(parentDirSqlStatement), Nil)
+ } else {
+ checkAnswer(sql(parentDirSqlStatement),
+ (1 to 2).map(i => Row(i, i, s"orc$i")))
+ }
+
+ val l3DirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl2(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '${l3Dir}'""".stripMargin
+ sql(l3DirStatement)
+ val l3DirSqlStatement = s"select * from tbl2"
+ if (convertMetastore) {
+ checkAnswer(sql(l3DirSqlStatement), Nil)
+ } else {
+ checkAnswer(sql(l3DirSqlStatement),
+ (1 to 2).map(i => Row(i, i, s"orc$i")))
+ }
+
+ val wildcardStatement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl3(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '$wildcardParentDir'""".stripMargin
+ sql(wildcardStatement)
+ val wildcardSqlStatement = s"select * from tbl3"
+ if (convertMetastore) {
+ checkAnswer(sql(wildcardSqlStatement),
+ (1 to 2).map(i => Row(i, i, s"orc$i")))
+ } else {
+ checkAnswer(sql(wildcardSqlStatement), Nil)
+ }
+
+ val wildcardL3Statement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl4(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '$wildcardL3Dir'""".stripMargin
+ sql(wildcardL3Statement)
+ val wildcardL3SqlStatement = s"select * from tbl4"
+ checkAnswer(sql(wildcardL3SqlStatement), Nil)
+ }
+ } finally {
+ hiveClient.runSqlHive("DROP TABLE IF EXISTS hive_orc")
--- End diff --
@dongjoon-hyun at line 221, I put the `tbl1` ~ `tbl4` with the `withTable`, I think it will get dropped. I tried to run it couple time in intellij, it seems work fine. what do you think?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99688 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99688/testReport)** for PR 23108 at commit [`fe472c8`](https://github.com/apache/spark/commit/fe472c81a21700ff52c84808437b85d02d6871ed).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99747/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on the issue:
https://github.com/apache/spark/pull/23108
I fixed a typo in the testcase, retest please. Thanks.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r238944485
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcSourceSuite.scala ---
@@ -186,6 +186,82 @@ abstract class OrcSuite extends OrcTest with BeforeAndAfterAll {
}
}
+ protected def testORCTableLocation(isConvertMetastore: Boolean): Unit = {
--- End diff --
Since this test helper function is only used in `HiveOrcSourceSuite`, can we move this into `HiveOrcSourceSuite`?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r239266673
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcSourceSuite.scala ---
@@ -186,6 +186,54 @@ abstract class OrcSuite extends OrcTest with BeforeAndAfterAll {
}
}
+ protected def testORCTableLocation(isConvertMetastore: Boolean): Unit = {
+ val tableName1 = "spark_orc1"
+ val tableName2 = "spark_orc2"
+
+ withTempDir { dir =>
+ val someDF1 = Seq((1, 1, "orc1"), (2, 2, "orc2")).toDF("c1", "c2", "c3").repartition(1)
+ withTable(tableName1, tableName2) {
+ val dataDir = s"${dir.getCanonicalPath}/dir1/"
+ val parentDir = s"${dir.getCanonicalPath}/"
+ val wildCardDir = new File(s"${dir}/*").toURI
+ someDF1.write.orc(dataDir)
+ val parentDirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE $tableName1(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '${parentDir}'""".stripMargin
+ sql(parentDirStatement)
+ val parentDirSqlStatement = s"select * from ${tableName1}"
+ if (isConvertMetastore) {
+ checkAnswer(sql(parentDirSqlStatement), Nil)
+ } else {
+ checkAnswer(sql(parentDirSqlStatement),
+ (1 to 2).map(i => Row(i, i, s"orc$i")))
+ }
+
+ val wildCardStatement =
+ s"""
+ |CREATE EXTERNAL TABLE $tableName2(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '$wildCardDir'""".stripMargin
--- End diff --
done
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on the issue:
https://github.com/apache/spark/pull/23108
Retest please, I didn't change Pyspark code, and tested `--modules=pyspark-sql` in local machine, it is fine. Thanks
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99243 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99243/testReport)** for PR 23108 at commit [`e731746`](https://github.com/apache/spark/commit/e731746da4643d0283b3cd788d286aea62c96215).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r239577623
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/orc/HiveOrcSourceSuite.scala ---
@@ -73,9 +73,11 @@ class HiveOrcSourceSuite extends OrcSuite with TestHiveSingleton {
sql(
s"""
|CREATE TABLE $tableName
+
|USING org.apache.spark.sql.hive.orc
|OPTIONS (
- | PATH '${new File(orcTableAsDir.getAbsolutePath).toURI}'
+ | PATH '${new File(orcTableAsDir.getAbsolutePath
+ ).toURI}'
--- End diff --
good catch, I didn't notice my changes affect the formatting in the file. I have revert the change. Thanks
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r238943607
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -2370,4 +2370,51 @@ class HiveDDLSuite
))
}
}
+
+ test("SPARK-25993 Add test cases for resolution of Parquet table location") {
--- End diff --
Also, let's replace the test case name with `CREATE EXTERNAL TABLE with subdirectories`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for resolu...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r238524452
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcSourceSuite.scala ---
@@ -186,6 +186,54 @@ abstract class OrcSuite extends OrcTest with BeforeAndAfterAll {
}
}
+ protected def testORCTableLocation(isConvertMetastore: Boolean): Unit = {
+ val tableName1 = "spark_orc1"
+ val tableName2 = "spark_orc2"
+
+ withTempDir { dir =>
+ val someDF1 = Seq((1, 1, "orc1"), (2, 2, "orc2")).toDF("c1", "c2", "c3").repartition(1)
+ withTable(tableName1, tableName2) {
+ val dataDir = s"${dir.getCanonicalPath}/dir1/"
+ val parentDir = s"${dir.getCanonicalPath}/"
+ val wildCardDir = new File(s"${dir}/*").toURI
+ someDF1.write.orc(dataDir)
+ val parentDirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE $tableName1(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '${parentDir}'""".stripMargin
+ sql(parentDirStatement)
+ val parentDirSqlStatement = s"select * from ${tableName1}"
+ if (isConvertMetastore) {
+ checkAnswer(sql(parentDirSqlStatement), Nil)
+ } else {
+ checkAnswer(sql(parentDirSqlStatement),
+ (1 to 2).map(i => Row(i, i, s"orc$i")))
+ }
+
+ val wildCardStatement =
+ s"""
+ |CREATE EXTERNAL TABLE $tableName2(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '$wildCardDir'""".stripMargin
--- End diff --
Thanks, @kevinyu98 . Also, please update the PR title
```
- [Spark-25993][SQL][TEST]Add test cases for resolution of ORC table location
+ [SPARK-25993][SQL][TEST] Add test cases for CREATE EXTERNAL TABLE with subdirectories
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99799 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99799/testReport)** for PR 23108 at commit [`d851169`](https://github.com/apache/spark/commit/d851169803861e24c3c251dcf936b4bf11a9c964).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r239266747
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/orc/HiveOrcSourceSuite.scala ---
@@ -190,4 +190,12 @@ class HiveOrcSourceSuite extends OrcSuite with TestHiveSingleton {
}
}
}
+
+ test("SPARK-25993 Add test cases for resolution of ORC table location") {
--- End diff --
Changed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r238944097
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -2370,4 +2370,51 @@ class HiveDDLSuite
))
}
}
+
+ test("SPARK-25993 Add test cases for resolution of Parquet table location") {
+ withTempPath { path =>
+ val someDF1 = Seq((1, 1, "parq1"), (2, 2, "parq2")).toDF("c1", "c2", "c3").repartition(1)
+ withTable("tbl1", "tbl2", "tbl3") {
+ val dataDir = s"${path.getCanonicalPath}/l3/l2/l1/"
--- End diff --
indentation?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r240022467
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveParquetSourceSuite.scala ---
@@ -222,4 +222,61 @@ class HiveParquetSourceSuite extends ParquetPartitioningTest {
assert(df4.columns === Array("str", "max_int"))
}
}
+
+ test("SPARK-25993 CREATE EXTERNAL TABLE with subdirectories") {
--- End diff --
Please fix this first for the first and second review comments.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99688 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99688/testReport)** for PR 23108 at commit [`fe472c8`](https://github.com/apache/spark/commit/fe472c81a21700ff52c84808437b85d02d6871ed).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99747 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99747/testReport)** for PR 23108 at commit [`51d1d78`](https://github.com/apache/spark/commit/51d1d78d1e1c4f56f5f07dc18bc9fcbe9a00fbbf).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for resolu...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r235790826
--- Diff: docs/sql-migration-guide-upgrade.md ---
@@ -111,6 +111,8 @@ displayTitle: Spark SQL Upgrading Guide
- Since Spark 2.0, Spark converts Parquet Hive tables by default for better performance. Since Spark 2.4, Spark converts ORC Hive tables by default, too. It means Spark uses its own ORC support by default instead of Hive SerDe. As an example, `CREATE TABLE t(id int) STORED AS ORC` would be handled with Hive SerDe in Spark 2.3, and in Spark 2.4, it would be converted into Spark's ORC data source table and ORC vectorization would be applied. To set `false` to `spark.sql.hive.convertMetastoreOrc` restores the previous behavior.
+ - In version 2.3 and earlier, `spark.sql.hive.converMetastoreOrc` default is `false`, if you specify a directory in the `LOCATION` clause in the `CREATE EXTERNAL TABLE STORED AS ORC LOCATION` sql statement, Spark will use the Hive ORC reader to read the data into the table if the directory or sub-directory contains the matching data, if you specify the wild card(*), the Hive ORC reader will not be able to read the data, because it is treating the wild card as a directory. For example: ORC data is stored at `/tmp/orctab1/dir1/`, `create external table tab1(...) stored as orc location '/tmp/orctab1/'` will read the data into the table, `create external table tab2(...) stored as orc location '/tmp/orctab1/*' ` will not. Since Spark 2.4, `spark.sql.hive.convertMetaStoreOrc` default is `true`, Spark will use native ORC reader, it will read the data if you specify the wild card, but will not if you specify the parent directory. To set `false` to `spark.sql.hive.convertMetastoreOrc`
restores the previous behavior.
--- End diff --
sure.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/23108
Retest this please.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for resolu...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r237272454
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcSourceSuite.scala ---
@@ -186,6 +186,54 @@ abstract class OrcSuite extends OrcTest with BeforeAndAfterAll {
}
}
+ protected def testORCTableLocation(isConvertMetastore: Boolean): Unit = {
+ val tableName1 = "spark_orc1"
+ val tableName2 = "spark_orc2"
+
+ withTempDir { dir =>
+ val someDF1 = Seq((1, 1, "orc1"), (2, 2, "orc2")).toDF("c1", "c2", "c3").repartition(1)
+ withTable(tableName1, tableName2) {
+ val dataDir = s"${dir.getCanonicalPath}/dir1/"
+ val parentDir = s"${dir.getCanonicalPath}/"
+ val wildCardDir = new File(s"${dir}/*").toURI
+ someDF1.write.orc(dataDir)
+ val parentDirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE $tableName1(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '${parentDir}'""".stripMargin
+ sql(parentDirStatement)
+ val parentDirSqlStatement = s"select * from ${tableName1}"
+ if (isConvertMetastore) {
+ checkAnswer(sql(parentDirSqlStatement), Nil)
+ } else {
+ checkAnswer(sql(parentDirSqlStatement),
+ (1 to 2).map(i => Row(i, i, s"orc$i")))
+ }
+
+ val wildCardStatement =
+ s"""
+ |CREATE EXTERNAL TABLE $tableName2(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '$wildCardDir'""".stripMargin
--- End diff --
@dongjoon-hyun Hello Dongjoon, yes, you are right. It will create a directory with the name is '*', and it is the same behavior prior spark 2.4. I was just following the examples from the jira. Do you have any suggestions here? Thanks.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on the issue:
https://github.com/apache/spark/pull/23108
ok to test
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r239266775
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -2370,4 +2370,51 @@ class HiveDDLSuite
))
}
}
+
+ test("SPARK-25993 Add test cases for resolution of Parquet table location") {
+ withTempPath { path =>
+ val someDF1 = Seq((1, 1, "parq1"), (2, 2, "parq2")).toDF("c1", "c2", "c3").repartition(1)
--- End diff --
fixed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99235 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99235/testReport)** for PR 23108 at commit [`e731746`](https://github.com/apache/spark/commit/e731746da4643d0283b3cd788d286aea62c96215).
* This patch **fails due to an unknown error code, -9**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the issue:
https://github.com/apache/spark/pull/23108
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99622/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r238943270
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -2370,4 +2370,51 @@ class HiveDDLSuite
))
}
}
+
+ test("SPARK-25993 Add test cases for resolution of Parquet table location") {
--- End diff --
Maybe, `HiveParquetSourceSuite`? That's the similar one with `OrcSourceSuite`.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99804 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99804/testReport)** for PR 23108 at commit [`d851169`](https://github.com/apache/spark/commit/d851169803861e24c3c251dcf936b4bf11a9c964).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r239663711
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveParquetSourceSuite.scala ---
@@ -222,4 +223,66 @@ class HiveParquetSourceSuite extends ParquetPartitioningTest {
assert(df4.columns === Array("str", "max_int"))
}
}
+
+ test("SPARK-25993 CREATE EXTERNAL TABLE with subdirectories") {
+ withTempPath { path =>
+ withTable("tbl1", "tbl2", "tbl3") {
+ val someDF1 = Seq((1, 1, "parq1"), (2, 2, "parq2")).
+ toDF("c1", "c2", "c3").repartition(1)
+ val dataDir = s"${path.getCanonicalPath}/l3/l2/l1/"
+ val parentDir = s"${path.getCanonicalPath}/l3/l2/"
+ val wildcardParentDir = new File(s"${path}/l3/l2/*").toURI
+ val wildcardL3Dir = new File(s"${path}/l3/*").toURI
+ someDF1.write.parquet(dataDir)
+ val parentDirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl1(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS parquet
+ |LOCATION '${parentDir}'""".stripMargin
+ sql(parentDirStatement)
+ val wildcardStatement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl2(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS parquet
+ |LOCATION '${wildcardParentDir}'""".stripMargin
+ sql(wildcardStatement)
+ val wildcardL3Statement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl3(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS parquet
+ |LOCATION '${wildcardL3Dir}'""".stripMargin
+ sql(wildcardL3Statement)
+
+ Seq("true", "false").foreach { parquetConversion =>
+ withSQLConf(HiveUtils.CONVERT_METASTORE_PARQUET.key -> parquetConversion) {
+ if (parquetConversion == "true") {
+ checkAnswer(sql("select * from tbl1"), Nil)
+ checkAnswer(sql("select * from tbl2"),
+ (1 to 2).map(i => Row(i, i, s"parq$i")))
+ checkAnswer(sql("select * from tbl3"), Nil)
+ } else {
+ Seq("select * from tbl1", "select * from tbl2", "select * from tbl3").foreach {
+ sqlStmt =>
+ try {
+ sql(sqlStmt)
+ } catch {
+ case e: IOException =>
+ assert(e.getMessage().contains("java.io.IOException: Not a file"))
+ }
--- End diff --
you are right, I will make changes. Thanks.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r240014393
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/orc/HiveOrcSourceSuite.scala ---
@@ -190,4 +192,103 @@ class HiveOrcSourceSuite extends OrcSuite with TestHiveSingleton {
}
}
}
+
+ test("SPARK-25993 CREATE EXTERNAL TABLE with subdirectories") {
+ Seq(true, false).foreach { convertMetastore =>
+ withSQLConf(HiveUtils.CONVERT_METASTORE_ORC.key -> s"$convertMetastore") {
+ withTempDir { dir =>
+ val dataDir = new File(s"${dir.getCanonicalPath}/l3/l2/l1/").toURI
+ val parentDir = s"${dir.getCanonicalPath}/l3/l2/"
+ val l3Dir = s"${dir.getCanonicalPath}/l3/"
+ val wildcardParentDir = new File(s"${dir}/l3/l2/*").toURI
+ val wildcardL3Dir = new File(s"${dir}/l3/*").toURI
+
+ try {
+ hiveClient.runSqlHive("USE default")
+ hiveClient.runSqlHive(
+ """
+ |CREATE EXTERNAL TABLE hive_orc(
+ | C1 INT,
+ | C2 INT,
+ | C3 STRING)
+ |STORED AS orc""".stripMargin)
+ // Hive throws an exception if I assign the location in the create table statement.
+ hiveClient.runSqlHive(
+ s"ALTER TABLE hive_orc SET LOCATION '$dataDir'")
+ hiveClient.runSqlHive(
+ """
+ |INSERT INTO TABLE hive_orc
+ |VALUES (1, 1, 'orc1'), (2, 2, 'orc2')""".stripMargin)
+
+ withTable("tbl1", "tbl2", "tbl3", "tbl4") {
+ val parentDirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl1(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '${parentDir}'""".stripMargin
+ sql(parentDirStatement)
+ val parentDirSqlStatement = s"select * from tbl1"
+ if (convertMetastore) {
+ checkAnswer(sql(parentDirSqlStatement), Nil)
+ } else {
+ checkAnswer(sql(parentDirSqlStatement),
+ (1 to 2).map(i => Row(i, i, s"orc$i")))
+ }
+
+ val l3DirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl2(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '${l3Dir}'""".stripMargin
+ sql(l3DirStatement)
+ val l3DirSqlStatement = s"select * from tbl2"
+ if (convertMetastore) {
+ checkAnswer(sql(l3DirSqlStatement), Nil)
+ } else {
+ checkAnswer(sql(l3DirSqlStatement),
+ (1 to 2).map(i => Row(i, i, s"orc$i")))
+ }
+
+ val wildcardStatement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl3(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '$wildcardParentDir'""".stripMargin
+ sql(wildcardStatement)
+ val wildcardSqlStatement = s"select * from tbl3"
+ if (convertMetastore) {
+ checkAnswer(sql(wildcardSqlStatement),
+ (1 to 2).map(i => Row(i, i, s"orc$i")))
+ } else {
+ checkAnswer(sql(wildcardSqlStatement), Nil)
+ }
+
+ val wildcardL3Statement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl4(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '$wildcardL3Dir'""".stripMargin
+ sql(wildcardL3Statement)
+ val wildcardL3SqlStatement = s"select * from tbl4"
+ checkAnswer(sql(wildcardL3SqlStatement), Nil)
+ }
+ } finally {
+ hiveClient.runSqlHive("DROP TABLE IF EXISTS hive_orc")
--- End diff --
Got it. I missed that.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Can one of the admins verify this patch?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99799/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99179 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99179/testReport)** for PR 23108 at commit [`e238764`](https://github.com/apache/spark/commit/e238764f278883b05d4bc88243facf897d357e84).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r239365367
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/orc/HiveOrcSourceSuite.scala ---
@@ -190,4 +192,103 @@ class HiveOrcSourceSuite extends OrcSuite with TestHiveSingleton {
}
}
}
+
+ test("SPARK-25993 CREATE EXTERNAL TABLE with subdirectories") {
+ Seq(true, false).foreach { convertMetastore =>
+ withSQLConf(HiveUtils.CONVERT_METASTORE_ORC.key -> s"$convertMetastore") {
+ withTempDir { dir =>
+ val dataDir = new File(s"${dir.getCanonicalPath}/l3/l2/l1/").toURI
+ val parentDir = s"${dir.getCanonicalPath}/l3/l2/"
+ val l3Dir = s"${dir.getCanonicalPath}/l3/"
+ val wildcardParentDir = new File(s"${dir}/l3/l2/*").toURI
+ val wildcardL3Dir = new File(s"${dir}/l3/*").toURI
+
+ try {
+ hiveClient.runSqlHive("USE default")
+ hiveClient.runSqlHive(
+ """
+ |CREATE EXTERNAL TABLE hive_orc(
+ | C1 INT,
+ | C2 INT,
+ | C3 STRING)
+ |STORED AS orc""".stripMargin)
+ // Hive throws an exception if I assign the location in the create table statement.
+ hiveClient.runSqlHive(
+ s"ALTER TABLE hive_orc SET LOCATION '$dataDir'")
+ hiveClient.runSqlHive(
+ """
+ |INSERT INTO TABLE hive_orc
+ |VALUES (1, 1, 'orc1'), (2, 2, 'orc2')""".stripMargin)
+
+ withTable("tbl1", "tbl2", "tbl3", "tbl4") {
+ val parentDirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl1(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '${parentDir}'""".stripMargin
+ sql(parentDirStatement)
+ val parentDirSqlStatement = s"select * from tbl1"
+ if (convertMetastore) {
+ checkAnswer(sql(parentDirSqlStatement), Nil)
+ } else {
+ checkAnswer(sql(parentDirSqlStatement),
+ (1 to 2).map(i => Row(i, i, s"orc$i")))
+ }
+
+ val l3DirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl2(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '${l3Dir}'""".stripMargin
+ sql(l3DirStatement)
+ val l3DirSqlStatement = s"select * from tbl2"
+ if (convertMetastore) {
+ checkAnswer(sql(l3DirSqlStatement), Nil)
+ } else {
+ checkAnswer(sql(l3DirSqlStatement),
+ (1 to 2).map(i => Row(i, i, s"orc$i")))
+ }
+
+ val wildcardStatement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl3(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '$wildcardParentDir'""".stripMargin
+ sql(wildcardStatement)
+ val wildcardSqlStatement = s"select * from tbl3"
+ if (convertMetastore) {
+ checkAnswer(sql(wildcardSqlStatement),
+ (1 to 2).map(i => Row(i, i, s"orc$i")))
+ } else {
+ checkAnswer(sql(wildcardSqlStatement), Nil)
+ }
+
+ val wildcardL3Statement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl4(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '$wildcardL3Dir'""".stripMargin
+ sql(wildcardL3Statement)
+ val wildcardL3SqlStatement = s"select * from tbl4"
+ checkAnswer(sql(wildcardL3SqlStatement), Nil)
+ }
+ } finally {
+ hiveClient.runSqlHive("DROP TABLE IF EXISTS hive_orc")
--- End diff --
It seems that we need to clean up `tbl1` ~ `tbl4`, too.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5787/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r239266908
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcSourceSuite.scala ---
@@ -186,6 +186,82 @@ abstract class OrcSuite extends OrcTest with BeforeAndAfterAll {
}
}
+ protected def testORCTableLocation(isConvertMetastore: Boolean): Unit = {
--- End diff --
ok, I moved.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r238943983
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -2370,4 +2370,51 @@ class HiveDDLSuite
))
}
}
+
+ test("SPARK-25993 Add test cases for resolution of Parquet table location") {
--- End diff --
Also, for the full test coverage, can we have the following combination like ORC, too?
```
Seq(true, false).foreach { convertMetastore =>
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99235/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for resolu...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r237337683
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcSourceSuite.scala ---
@@ -186,6 +186,54 @@ abstract class OrcSuite extends OrcTest with BeforeAndAfterAll {
}
}
+ protected def testORCTableLocation(isConvertMetastore: Boolean): Unit = {
+ val tableName1 = "spark_orc1"
+ val tableName2 = "spark_orc2"
+
+ withTempDir { dir =>
+ val someDF1 = Seq((1, 1, "orc1"), (2, 2, "orc2")).toDF("c1", "c2", "c3").repartition(1)
+ withTable(tableName1, tableName2) {
+ val dataDir = s"${dir.getCanonicalPath}/dir1/"
+ val parentDir = s"${dir.getCanonicalPath}/"
+ val wildCardDir = new File(s"${dir}/*").toURI
+ someDF1.write.orc(dataDir)
+ val parentDirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE $tableName1(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '${parentDir}'""".stripMargin
+ sql(parentDirStatement)
+ val parentDirSqlStatement = s"select * from ${tableName1}"
+ if (isConvertMetastore) {
+ checkAnswer(sql(parentDirSqlStatement), Nil)
+ } else {
+ checkAnswer(sql(parentDirSqlStatement),
+ (1 to 2).map(i => Row(i, i, s"orc$i")))
+ }
+
+ val wildCardStatement =
+ s"""
+ |CREATE EXTERNAL TABLE $tableName2(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '$wildCardDir'""".stripMargin
--- End diff --
I have two suggestions.
1. Is this PR aiming only one-level subdirectories? Could you check the behavior on one, two, three level subdirectories in Parquet Hive tables first?
2. Since the test case looks general for both Parquet/ORC, please add a test case for Parquet while you are here.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99179 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99179/testReport)** for PR 23108 at commit [`e238764`](https://github.com/apache/spark/commit/e238764f278883b05d4bc88243facf897d357e84).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r239361792
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/orc/HiveOrcSourceSuite.scala ---
@@ -73,9 +73,11 @@ class HiveOrcSourceSuite extends OrcSuite with TestHiveSingleton {
sql(
s"""
|CREATE TABLE $tableName
+
|USING org.apache.spark.sql.hive.orc
|OPTIONS (
- | PATH '${new File(orcTableAsDir.getAbsolutePath).toURI}'
+ | PATH '${new File(orcTableAsDir.getAbsolutePath
+ ).toURI}'
--- End diff --
The above change in line 76 ~ 80 looks strange and irrelevant. Let's revert this change.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99799 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99799/testReport)** for PR 23108 at commit [`d851169`](https://github.com/apache/spark/commit/d851169803861e24c3c251dcf936b4bf11a9c964).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r238943694
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/orc/HiveOrcSourceSuite.scala ---
@@ -190,4 +190,12 @@ class HiveOrcSourceSuite extends OrcSuite with TestHiveSingleton {
}
}
}
+
+ test("SPARK-25993 Add test cases for resolution of ORC table location") {
--- End diff --
Please change this to `CREATE EXTERNAL TABLE with subdirectories`, too.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r239266791
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -2370,4 +2370,51 @@ class HiveDDLSuite
))
}
}
+
+ test("SPARK-25993 Add test cases for resolution of Parquet table location") {
+ withTempPath { path =>
+ val someDF1 = Seq((1, 1, "parq1"), (2, 2, "parq2")).toDF("c1", "c2", "c3").repartition(1)
+ withTable("tbl1", "tbl2", "tbl3") {
+ val dataDir = s"${path.getCanonicalPath}/l3/l2/l1/"
--- End diff --
fixed
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99747 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99747/testReport)** for PR 23108 at commit [`51d1d78`](https://github.com/apache/spark/commit/51d1d78d1e1c4f56f5f07dc18bc9fcbe9a00fbbf).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for resolu...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r235670505
--- Diff: docs/sql-migration-guide-upgrade.md ---
@@ -111,6 +111,8 @@ displayTitle: Spark SQL Upgrading Guide
- Since Spark 2.0, Spark converts Parquet Hive tables by default for better performance. Since Spark 2.4, Spark converts ORC Hive tables by default, too. It means Spark uses its own ORC support by default instead of Hive SerDe. As an example, `CREATE TABLE t(id int) STORED AS ORC` would be handled with Hive SerDe in Spark 2.3, and in Spark 2.4, it would be converted into Spark's ORC data source table and ORC vectorization would be applied. To set `false` to `spark.sql.hive.convertMetastoreOrc` restores the previous behavior.
+ - In version 2.3 and earlier, `spark.sql.hive.converMetastoreOrc` default is `false`, if you specify a directory in the `LOCATION` clause in the `CREATE EXTERNAL TABLE STORED AS ORC LOCATION` sql statement, Spark will use the Hive ORC reader to read the data into the table if the directory or sub-directory contains the matching data, if you specify the wild card(*), the Hive ORC reader will not be able to read the data, because it is treating the wild card as a directory. For example: ORC data is stored at `/tmp/orctab1/dir1/`, `create external table tab1(...) stored as orc location '/tmp/orctab1/'` will read the data into the table, `create external table tab2(...) stored as orc location '/tmp/orctab1/*' ` will not. Since Spark 2.4, `spark.sql.hive.convertMetaStoreOrc` default is `true`, Spark will use native ORC reader, it will read the data if you specify the wild card, but will not if you specify the parent directory. To set `false` to `spark.sql.hive.convertMetastoreOrc`
restores the previous behavior.
--- End diff --
Could you change `but will not if you specify the parent directory` more clearly with examples like the other sentence?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for resolu...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r236797263
--- Diff: docs/sql-migration-guide-upgrade.md ---
@@ -111,6 +111,8 @@ displayTitle: Spark SQL Upgrading Guide
- Since Spark 2.0, Spark converts Parquet Hive tables by default for better performance. Since Spark 2.4, Spark converts ORC Hive tables by default, too. It means Spark uses its own ORC support by default instead of Hive SerDe. As an example, `CREATE TABLE t(id int) STORED AS ORC` would be handled with Hive SerDe in Spark 2.3, and in Spark 2.4, it would be converted into Spark's ORC data source table and ORC vectorization would be applied. To set `false` to `spark.sql.hive.convertMetastoreOrc` restores the previous behavior.
+ - In version 2.3 and earlier, `spark.sql.hive.converMetastoreOrc` default is `false`, if you specify a directory in the `LOCATION` clause in the `CREATE EXTERNAL TABLE STORED AS ORC LOCATION` sql statement, Spark will use the Hive ORC reader to read the data into the table if the directory or sub-directory contains the matching data, if you specify the wild card(*), the Hive ORC reader will not be able to read the data, because it is treating the wild card as a directory. For example: ORC data is stored at `/tmp/orctab1/dir1/`, `create external table tab1(...) stored as orc location '/tmp/orctab1/'` will read the data into the table, `create external table tab2(...) stored as orc location '/tmp/orctab1/*' ` will not. Since Spark 2.4, `spark.sql.hive.convertMetaStoreOrc` default is `true`, Spark will use native ORC reader, if you specify the wild card, it will try to read the matching data from current directory and sub-directory, if you specify a directory which does not conta
ins the matching data, native ORC reader will not be able to read, even the data is in the sub-directory. For example: ORC data is stored at `/tmp/orctab1/dir1/`, `create external table tab3(...) stored as orc location '/tmp/orctab1/'` will not read the data from sub-directory into the table. To set `false` to `spark.sql.hive.convertMetastoreOrc` restores the previous behavior.
--- End diff --
`spark.sql.hive.converMetastoreOrc` -> `spark.sql.hive.convertMetastoreOrc`
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99804 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99804/testReport)** for PR 23108 at commit [`d851169`](https://github.com/apache/spark/commit/d851169803861e24c3c251dcf936b4bf11a9c964).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5786/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5841/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99179/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r238944067
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -2370,4 +2370,51 @@ class HiveDDLSuite
))
}
}
+
+ test("SPARK-25993 Add test cases for resolution of Parquet table location") {
+ withTempPath { path =>
+ val someDF1 = Seq((1, 1, "parq1"), (2, 2, "parq2")).toDF("c1", "c2", "c3").repartition(1)
--- End diff --
Indentation.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99243/
Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for resolu...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r238469695
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcSourceSuite.scala ---
@@ -186,6 +186,54 @@ abstract class OrcSuite extends OrcTest with BeforeAndAfterAll {
}
}
+ protected def testORCTableLocation(isConvertMetastore: Boolean): Unit = {
+ val tableName1 = "spark_orc1"
+ val tableName2 = "spark_orc2"
+
+ withTempDir { dir =>
+ val someDF1 = Seq((1, 1, "orc1"), (2, 2, "orc2")).toDF("c1", "c2", "c3").repartition(1)
+ withTable(tableName1, tableName2) {
+ val dataDir = s"${dir.getCanonicalPath}/dir1/"
+ val parentDir = s"${dir.getCanonicalPath}/"
+ val wildCardDir = new File(s"${dir}/*").toURI
+ someDF1.write.orc(dataDir)
+ val parentDirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE $tableName1(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '${parentDir}'""".stripMargin
+ sql(parentDirStatement)
+ val parentDirSqlStatement = s"select * from ${tableName1}"
+ if (isConvertMetastore) {
+ checkAnswer(sql(parentDirSqlStatement), Nil)
+ } else {
+ checkAnswer(sql(parentDirSqlStatement),
+ (1 to 2).map(i => Row(i, i, s"orc$i")))
+ }
+
+ val wildCardStatement =
+ s"""
+ |CREATE EXTERNAL TABLE $tableName2(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '$wildCardDir'""".stripMargin
--- End diff --
@dongjoon-hyun I didn't add three level subdirectores in this PR, should I ? I was thinking to add the three levels in the follow up PR. Let me know what you prefer. Thanks.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for resolu...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r235790938
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcQuerySuite.scala ---
@@ -597,6 +597,38 @@ abstract class OrcQueryTest extends OrcTest {
assert(m4.contains("Malformed ORC file"))
}
}
+
+ test("SPARK-25993 Add test cases for resolution of ORC table location") {
--- End diff --
ok, I will move the test case to there. Thanks.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by dilipbiswal <gi...@git.apache.org>.
Github user dilipbiswal commented on the issue:
https://github.com/apache/spark/pull/23108
retest this please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99746 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99746/testReport)** for PR 23108 at commit [`51d1d78`](https://github.com/apache/spark/commit/51d1d78d1e1c4f56f5f07dc18bc9fcbe9a00fbbf).
* This patch **fails PySpark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r239266816
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -2370,4 +2370,51 @@ class HiveDDLSuite
))
}
}
+
+ test("SPARK-25993 Add test cases for resolution of Parquet table location") {
+ withTempPath { path =>
+ val someDF1 = Seq((1, 1, "parq1"), (2, 2, "parq2")).toDF("c1", "c2", "c3").repartition(1)
+ withTable("tbl1", "tbl2", "tbl3") {
+ val dataDir = s"${path.getCanonicalPath}/l3/l2/l1/"
+ val parentDir = s"${path.getCanonicalPath}/l3/l2/"
+ val l3Dir = s"${path.getCanonicalPath}/l3/"
+ val wildcardParentDir = new File(s"${path}/l3/l2/*").toURI
+ val wildcardL3Dir = new File(s"${path}/l3/*").toURI
+ someDF1.write.parquet(dataDir)
+ val parentDirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl1(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS parquet
+ |LOCATION '${parentDir}'""".stripMargin
+ sql(parentDirStatement)
+ checkAnswer(sql("select * from tbl1"), Nil)
+
+ val wildcardStatement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl2(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS parquet
+ |LOCATION '${wildcardParentDir}'""".stripMargin
+ sql(wildcardStatement)
+ checkAnswer(sql("select * from tbl2"),
+ (1 to 2).map(i => Row(i, i, s"parq$i")))
+
+ val wildcardL3Statement =
+ s"""
--- End diff --
fixed
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r239365705
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/HiveParquetSourceSuite.scala ---
@@ -32,6 +32,7 @@ import org.apache.spark.util.Utils
class HiveParquetSourceSuite extends ParquetPartitioningTest {
import testImplicits._
import spark._
+ import java.io.IOException
--- End diff --
This had better go to line 21.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5278/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5836/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99746 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99746/testReport)** for PR 23108 at commit [`51d1d78`](https://github.com/apache/spark/commit/51d1d78d1e1c4f56f5f07dc18bc9fcbe9a00fbbf).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for resolu...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r237948799
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcSourceSuite.scala ---
@@ -186,6 +186,54 @@ abstract class OrcSuite extends OrcTest with BeforeAndAfterAll {
}
}
+ protected def testORCTableLocation(isConvertMetastore: Boolean): Unit = {
+ val tableName1 = "spark_orc1"
+ val tableName2 = "spark_orc2"
+
+ withTempDir { dir =>
+ val someDF1 = Seq((1, 1, "orc1"), (2, 2, "orc2")).toDF("c1", "c2", "c3").repartition(1)
+ withTable(tableName1, tableName2) {
+ val dataDir = s"${dir.getCanonicalPath}/dir1/"
+ val parentDir = s"${dir.getCanonicalPath}/"
+ val wildCardDir = new File(s"${dir}/*").toURI
+ someDF1.write.orc(dataDir)
+ val parentDirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE $tableName1(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '${parentDir}'""".stripMargin
+ sql(parentDirStatement)
+ val parentDirSqlStatement = s"select * from ${tableName1}"
+ if (isConvertMetastore) {
+ checkAnswer(sql(parentDirSqlStatement), Nil)
+ } else {
+ checkAnswer(sql(parentDirSqlStatement),
+ (1 to 2).map(i => Row(i, i, s"orc$i")))
+ }
+
+ val wildCardStatement =
+ s"""
+ |CREATE EXTERNAL TABLE $tableName2(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '$wildCardDir'""".stripMargin
--- End diff --
Thank you for investigating. I agree with you for (1). For the test case, please add three-level subdirectories. That will help us to improve Spark later. You may file another JIRA issue for that as a new feature JIRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test FAILed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99243 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99243/testReport)** for PR 23108 at commit [`e731746`](https://github.com/apache/spark/commit/e731746da4643d0283b3cd788d286aea62c96215).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99251 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99251/testReport)** for PR 23108 at commit [`d6e582b`](https://github.com/apache/spark/commit/d6e582b3ff33f767d41c9c7cf1710107d7901e0f).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for resolu...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r237272654
--- Diff: docs/sql-migration-guide-upgrade.md ---
@@ -111,6 +111,8 @@ displayTitle: Spark SQL Upgrading Guide
- Since Spark 2.0, Spark converts Parquet Hive tables by default for better performance. Since Spark 2.4, Spark converts ORC Hive tables by default, too. It means Spark uses its own ORC support by default instead of Hive SerDe. As an example, `CREATE TABLE t(id int) STORED AS ORC` would be handled with Hive SerDe in Spark 2.3, and in Spark 2.4, it would be converted into Spark's ORC data source table and ORC vectorization would be applied. To set `false` to `spark.sql.hive.convertMetastoreOrc` restores the previous behavior.
+ - In version 2.3 and earlier, `spark.sql.hive.converMetastoreOrc` default is `false`, if you specify a directory in the `LOCATION` clause in the `CREATE EXTERNAL TABLE STORED AS ORC LOCATION` sql statement, Spark will use the Hive ORC reader to read the data into the table if the directory or sub-directory contains the matching data, if you specify the wild card(*), the Hive ORC reader will not be able to read the data, because it is treating the wild card as a directory. For example: ORC data is stored at `/tmp/orctab1/dir1/`, `create external table tab1(...) stored as orc location '/tmp/orctab1/'` will read the data into the table, `create external table tab2(...) stored as orc location '/tmp/orctab1/*' ` will not. Since Spark 2.4, `spark.sql.hive.convertMetaStoreOrc` default is `true`, Spark will use native ORC reader, if you specify the wild card, it will try to read the matching data from current directory and sub-directory, if you specify a directory which does not conta
ins the matching data, native ORC reader will not be able to read, even the data is in the sub-directory. For example: ORC data is stored at `/tmp/orctab1/dir1/`, `create external table tab3(...) stored as orc location '/tmp/orctab1/'` will not read the data from sub-directory into the table. To set `false` to `spark.sql.hive.convertMetastoreOrc` restores the previous behavior.
--- End diff --
Thanks, I will make changes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for resolu...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r237691564
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcSourceSuite.scala ---
@@ -186,6 +186,54 @@ abstract class OrcSuite extends OrcTest with BeforeAndAfterAll {
}
}
+ protected def testORCTableLocation(isConvertMetastore: Boolean): Unit = {
+ val tableName1 = "spark_orc1"
+ val tableName2 = "spark_orc2"
+
+ withTempDir { dir =>
+ val someDF1 = Seq((1, 1, "orc1"), (2, 2, "orc2")).toDF("c1", "c2", "c3").repartition(1)
+ withTable(tableName1, tableName2) {
+ val dataDir = s"${dir.getCanonicalPath}/dir1/"
+ val parentDir = s"${dir.getCanonicalPath}/"
+ val wildCardDir = new File(s"${dir}/*").toURI
+ someDF1.write.orc(dataDir)
+ val parentDirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE $tableName1(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '${parentDir}'""".stripMargin
+ sql(parentDirStatement)
+ val parentDirSqlStatement = s"select * from ${tableName1}"
+ if (isConvertMetastore) {
+ checkAnswer(sql(parentDirSqlStatement), Nil)
+ } else {
+ checkAnswer(sql(parentDirSqlStatement),
+ (1 to 2).map(i => Row(i, i, s"orc$i")))
+ }
+
+ val wildCardStatement =
+ s"""
+ |CREATE EXTERNAL TABLE $tableName2(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '$wildCardDir'""".stripMargin
--- End diff --
@dongjoon-hyun Thanks for the suggestions. I tried with three level subdirectores for Parquet/ORC. Here is the result:
sql("set spark.sql.hive.convertMetastoreOrc=true")
three level directories
ORC:
- "/*" can read sub directory data, but not three level subdirectories
- "/" can only read current directory
Parquet:
- "/*" can read sub directory data, but not three level subdirectories
- "/" can only read current directory
sql("set spark.sql.hive.convertMetastoreOrc=false")
ORC:
- "/" can read three level subdirectories
- "/*" can't read any data
parquet:
- "/" can only read current directory
- "/*" can read sub directory data, but not three level subdirectories.
With sql("set spark.sql.hive.convertMetastoreOrc=true"), the ORC and Parquet behavior is consistent.
1. I think this PR is aiming only one-level subdirectores.
2. Sure, I will add one more for Parquet.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for resolu...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r236866087
--- Diff: docs/sql-migration-guide-upgrade.md ---
@@ -111,6 +111,8 @@ displayTitle: Spark SQL Upgrading Guide
- Since Spark 2.0, Spark converts Parquet Hive tables by default for better performance. Since Spark 2.4, Spark converts ORC Hive tables by default, too. It means Spark uses its own ORC support by default instead of Hive SerDe. As an example, `CREATE TABLE t(id int) STORED AS ORC` would be handled with Hive SerDe in Spark 2.3, and in Spark 2.4, it would be converted into Spark's ORC data source table and ORC vectorization would be applied. To set `false` to `spark.sql.hive.convertMetastoreOrc` restores the previous behavior.
+ - In version 2.3 and earlier, `spark.sql.hive.converMetastoreOrc` default is `false`, if you specify a directory in the `LOCATION` clause in the `CREATE EXTERNAL TABLE STORED AS ORC LOCATION` sql statement, Spark will use the Hive ORC reader to read the data into the table if the directory or sub-directory contains the matching data, if you specify the wild card(*), the Hive ORC reader will not be able to read the data, because it is treating the wild card as a directory. For example: ORC data is stored at `/tmp/orctab1/dir1/`, `create external table tab1(...) stored as orc location '/tmp/orctab1/'` will read the data into the table, `create external table tab2(...) stored as orc location '/tmp/orctab1/*' ` will not. Since Spark 2.4, `spark.sql.hive.convertMetaStoreOrc` default is `true`, Spark will use native ORC reader, if you specify the wild card, it will try to read the matching data from current directory and sub-directory, if you specify a directory which does not conta
ins the matching data, native ORC reader will not be able to read, even the data is in the sub-directory. For example: ORC data is stored at `/tmp/orctab1/dir1/`, `create external table tab3(...) stored as orc location '/tmp/orctab1/'` will not read the data from sub-directory into the table. To set `false` to `spark.sql.hive.convertMetastoreOrc` restores the previous behavior.
--- End diff --
I've read this again. In fact, this is not a new behavior for Spark users because Apache Spark uses Parquet as a default format since 2.0 and the default behavior of `STORED AS PARQUET` works like this.
In order to give the rich context to the users and to avoid irrelevant confusions, we had better merge this part into the above line (line 112). For example, I'd like to update line 112 like the following.
> applied. **In addition, this makes Spark's Hive table behavior more consistent over different formats. For example, for both ORC/Parquet Hive tables, `LOCATION '/table/*'` is required instead of `LOCATION '/table/'` to create an external table reading its direct sub-directories.** To set `false` to `spark.sql.hive.convertMetastoreOrc` restores the previous behavior.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/testing-k8s-prb-make-spark-distribution-unified/5740/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r239266720
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -2370,4 +2370,51 @@ class HiveDDLSuite
))
}
}
+
+ test("SPARK-25993 Add test cases for resolution of Parquet table location") {
--- End diff --
sure, changed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for resolu...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r235671486
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcQuerySuite.scala ---
@@ -597,6 +597,38 @@ abstract class OrcQueryTest extends OrcTest {
assert(m4.contains("Malformed ORC file"))
}
}
+
+ test("SPARK-25993 Add test cases for resolution of ORC table location") {
--- End diff --
`HiveOrcSourceSuite.scala` will be the better place. And, we had better have the following and cover both case behaviors; `true` and `false`.
```
Seq(true, false).foreach { convertMetastore =>
withSQLConf(HiveUtils.CONVERT_METASTORE_ORC.key -> s"$convertMetastore") {
```
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99688/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for resolu...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r238367919
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/datasources/orc/OrcSourceSuite.scala ---
@@ -186,6 +186,54 @@ abstract class OrcSuite extends OrcTest with BeforeAndAfterAll {
}
}
+ protected def testORCTableLocation(isConvertMetastore: Boolean): Unit = {
+ val tableName1 = "spark_orc1"
+ val tableName2 = "spark_orc2"
+
+ withTempDir { dir =>
+ val someDF1 = Seq((1, 1, "orc1"), (2, 2, "orc2")).toDF("c1", "c2", "c3").repartition(1)
+ withTable(tableName1, tableName2) {
+ val dataDir = s"${dir.getCanonicalPath}/dir1/"
+ val parentDir = s"${dir.getCanonicalPath}/"
+ val wildCardDir = new File(s"${dir}/*").toURI
+ someDF1.write.orc(dataDir)
+ val parentDirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE $tableName1(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '${parentDir}'""".stripMargin
+ sql(parentDirStatement)
+ val parentDirSqlStatement = s"select * from ${tableName1}"
+ if (isConvertMetastore) {
+ checkAnswer(sql(parentDirSqlStatement), Nil)
+ } else {
+ checkAnswer(sql(parentDirSqlStatement),
+ (1 to 2).map(i => Row(i, i, s"orc$i")))
+ }
+
+ val wildCardStatement =
+ s"""
+ |CREATE EXTERNAL TABLE $tableName2(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS orc
+ |LOCATION '$wildCardDir'""".stripMargin
--- End diff --
@dongjoon-hyun Sorry for the delay. My got some issues with my Intellij environment. Sure, I will add three level subdirectories for this PR. FYI, I also tried with `convertMetastoreParquet` for Parquet, the behavior is consistent.
sql("set spark.sql.hive.convertMetastoreParquet = true")
three level
Parquet:
-- "/" can only read current directory
-- "/*" can read sub directory data, but not three level subdirectories.
sql("set spark.sql.hive.convertMetastoreParquet = false")
-- "/" can only read current directory
-- "/*" can read sub directory data, but not three level subdirectories.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by kevinyu98 <gi...@git.apache.org>.
Github user kevinyu98 commented on the issue:
https://github.com/apache/spark/pull/23108
retest please
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE...
Posted by dongjoon-hyun <gi...@git.apache.org>.
Github user dongjoon-hyun commented on a diff in the pull request:
https://github.com/apache/spark/pull/23108#discussion_r238944132
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/hive/execution/HiveDDLSuite.scala ---
@@ -2370,4 +2370,51 @@ class HiveDDLSuite
))
}
}
+
+ test("SPARK-25993 Add test cases for resolution of Parquet table location") {
+ withTempPath { path =>
+ val someDF1 = Seq((1, 1, "parq1"), (2, 2, "parq2")).toDF("c1", "c2", "c3").repartition(1)
+ withTable("tbl1", "tbl2", "tbl3") {
+ val dataDir = s"${path.getCanonicalPath}/l3/l2/l1/"
+ val parentDir = s"${path.getCanonicalPath}/l3/l2/"
+ val l3Dir = s"${path.getCanonicalPath}/l3/"
+ val wildcardParentDir = new File(s"${path}/l3/l2/*").toURI
+ val wildcardL3Dir = new File(s"${path}/l3/*").toURI
+ someDF1.write.parquet(dataDir)
+ val parentDirStatement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl1(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS parquet
+ |LOCATION '${parentDir}'""".stripMargin
+ sql(parentDirStatement)
+ checkAnswer(sql("select * from tbl1"), Nil)
+
+ val wildcardStatement =
+ s"""
+ |CREATE EXTERNAL TABLE tbl2(
+ | c1 int,
+ | c2 int,
+ | c3 string)
+ |STORED AS parquet
+ |LOCATION '${wildcardParentDir}'""".stripMargin
+ sql(wildcardStatement)
+ checkAnswer(sql("select * from tbl2"),
+ (1 to 2).map(i => Row(i, i, s"parq$i")))
+
+ val wildcardL3Statement =
+ s"""
--- End diff --
indentation?
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99622 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99622/testReport)** for PR 23108 at commit [`d75b923`](https://github.com/apache/spark/commit/d75b923483a6ff7d74824f8327a77e7af1ca1022).
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/99251/
Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for resolution of...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/23108
**[Test build #99622 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/99622/testReport)** for PR 23108 at commit [`d75b923`](https://github.com/apache/spark/commit/d75b923483a6ff7d74824f8327a77e7af1ca1022).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark issue #23108: [Spark-25993][SQL][TEST]Add test cases for CREATE EXTERN...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/23108
Merged build finished. Test PASSed.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org