You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by chenghao-intel <gi...@git.apache.org> on 2015/02/12 10:29:46 UTC
[GitHub] spark pull request: [SQL] [Minor] Passdown the schema for Parquet ...
GitHub user chenghao-intel opened a pull request:
https://github.com/apache/spark/pull/4562
[SQL] [Minor] Passdown the schema for Parquet File in HiveContext
It's not allowed to be the empty directory for parquet, for example, it will failed when query the following
```
CREATE TABLE parquet_test (id int, str string) STORED AS PARQUET;
SELECT * FROM parquet_test;
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/chenghao-intel/spark parquet_error
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/4562.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 #4562
----
commit 33867c09248b22cca8456268af176f17206a4b74
Author: Cheng Hao <ha...@intel.com>
Date: 2015-02-12T09:19:27Z
passdown the schema for Parquet File in HiveContext
----
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:
https://github.com/apache/spark/pull/4562#issuecomment-74788801
@chenghao-intel can you close it? It is has been fixed by #4655.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4562#issuecomment-74615738
[Test build #27607 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27607/consoleFull) for PR 4562 at commit [`a04930b`](https://github.com/apache/spark/commit/a04930badb291e55ba4e6ba79ce781a89f827932).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/4562#discussion_r24791802
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/parquet/newParquet.scala ---
@@ -287,7 +287,11 @@ case class ParquetRelation2(
}
}
- parquetSchema = maybeSchema.getOrElse(readSchema())
+ try {
+ parquetSchema = readSchema().getOrElse(maybeSchema.get)
+ } catch {
+ case e => throw new SparkException(s"Failed to find schema for ${paths.mkString(",")}", e)
+ }
--- End diff --
How about this
```
parquetSchema = {
if (maybeSchema.isDefined) {
maybeSchema.get
} else {
(readSchema(), maybeMetastoreSchema) match {
case (Some(dataSchema), _) => dataSchema
case (None, Some(metastoreSchema)) => metastoreSchema
case (None, None) =>
throw new SparkException("Failed to get the schema.")
}
}
}
```
We first check if maybeSchema is defined. If not, we read the schema from existing data. If existing data does not exist, we are dealing with a newly created empty table and we will use maybeMetastoreSchema defined in the options.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/4562#discussion_r24791821
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/parquet/parquetSuites.scala ---
@@ -121,13 +121,50 @@ class ParquetDataSourceOnMetastoreSuite extends ParquetMetastoreSuiteBase {
override def beforeAll(): Unit = {
super.beforeAll()
+
+ sql(s"""
+ create table test_parquet
+ (
+ intField INT,
+ stringField STRING
+ )
+ ROW FORMAT SERDE 'org.apache.hadoop.hive.ql.io.parquet.serde.ParquetHiveSerDe'
+ STORED AS
+ INPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat'
+ OUTPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetOutputFormat'
+ """)
+
+ val rdd = sparkContext.parallelize((1 to 10).map(i => s"""{"a":$i, "b":"str${i}"}"""))
+ jsonRDD(rdd).registerTempTable("jt")
+ sql("""
+ create table test ROW FORMAT
+ | SERDE 'org.apache.hadoop.hive.ql.io.parquet.serde.ParquetHiveSerDe'
+ | STORED AS INPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat'
+ | OUTPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetOutputFormat'
+ | AS select * from jt""".stripMargin)
+
--- End diff --
Also add a test for `CREATE TABLE ... STORED AS PARQUET AS ...`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on the pull request:
https://github.com/apache/spark/pull/4562#issuecomment-74662087
Hey @chenghao-intel @yhuai, sorry I didn't notice this PR earlier, and I believe this issue has been fixed in #4563 ([here](https://github.com/apache/spark/pull/4563/files#diff-c69b9e667e93b7e4693812cc72abb65fR245)).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SQL] [Minor] Passdown the schema for Parquet ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4562#issuecomment-74203584
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27414/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by chenghao-intel <gi...@git.apache.org>.
Github user chenghao-intel commented on a diff in the pull request:
https://github.com/apache/spark/pull/4562#discussion_r24793945
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/parquet/newParquet.scala ---
@@ -287,7 +287,11 @@ case class ParquetRelation2(
}
}
- parquetSchema = maybeSchema.getOrElse(readSchema())
+ try {
+ parquetSchema = readSchema().getOrElse(maybeSchema.get)
+ } catch {
+ case e => throw new SparkException(s"Failed to find schema for ${paths.mkString(",")}", e)
+ }
--- End diff --
After reading the source code, I am wondering if the `maybeMetastoreSchema` is redundant, and it probably should be always converted into `maybeSchema` when creating the `ParquetRelation2` instance?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4562#issuecomment-74634826
[Test build #27615 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27615/consoleFull) for PR 4562 at commit [`36978d1`](https://github.com/apache/spark/commit/36978d1835ab6e0266ad3787b33056b573fd59e8).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SQL] [Minor] Passdown the schema for Parquet ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4562#issuecomment-74610745
[Test build #27607 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27607/consoleFull) for PR 4562 at commit [`a04930b`](https://github.com/apache/spark/commit/a04930badb291e55ba4e6ba79ce781a89f827932).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SQL] [Minor] Passdown the schema for Parquet ...
Posted by chenghao-intel <gi...@git.apache.org>.
Github user chenghao-intel commented on the pull request:
https://github.com/apache/spark/pull/4562#issuecomment-74213984
@liancheng
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4562#issuecomment-74615739
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27607/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SQL] [Minor] Passdown the schema for Parquet ...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4562#issuecomment-74047028
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27344/
Test FAILed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by chenghao-intel <gi...@git.apache.org>.
Github user chenghao-intel commented on a diff in the pull request:
https://github.com/apache/spark/pull/4562#discussion_r24793419
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/parquet/parquetSuites.scala ---
@@ -121,13 +121,50 @@ class ParquetDataSourceOnMetastoreSuite extends ParquetMetastoreSuiteBase {
override def beforeAll(): Unit = {
super.beforeAll()
+
+ sql(s"""
+ create table test_parquet
+ (
+ intField INT,
+ stringField STRING
+ )
+ ROW FORMAT SERDE 'org.apache.hadoop.hive.ql.io.parquet.serde.ParquetHiveSerDe'
+ STORED AS
+ INPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat'
+ OUTPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetOutputFormat'
+ """)
+
+ val rdd = sparkContext.parallelize((1 to 10).map(i => s"""{"a":$i, "b":"str${i}"}"""))
+ jsonRDD(rdd).registerTempTable("jt")
+ sql("""
+ create table test ROW FORMAT
+ | SERDE 'org.apache.hadoop.hive.ql.io.parquet.serde.ParquetHiveSerDe'
+ | STORED AS INPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat'
+ | OUTPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetOutputFormat'
+ | AS select * from jt""".stripMargin)
+
--- End diff --
`STORED AS PARQUET` is supported since Hive 0.13, the unit test may failed in Hive 0.12 if we do that.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4562#issuecomment-74627418
[Test build #27615 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27615/consoleFull) for PR 4562 at commit [`36978d1`](https://github.com/apache/spark/commit/36978d1835ab6e0266ad3787b33056b573fd59e8).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/4562#discussion_r24794112
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/parquet/newParquet.scala ---
@@ -287,7 +287,11 @@ case class ParquetRelation2(
}
}
- parquetSchema = maybeSchema.getOrElse(readSchema())
+ try {
+ parquetSchema = readSchema().getOrElse(maybeSchema.get)
+ } catch {
+ case e => throw new SparkException(s"Failed to find schema for ${paths.mkString(",")}", e)
+ }
--- End diff --
Based on Cheng's comment at https://github.com/apache/spark/blob/master/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala#L194, I think that it is better to keep `maybeMetastoreSchema` and we just fix the bug for now.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SQL] [Minor] Passdown the schema for Parquet ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4562#issuecomment-74203579
[Test build #27414 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27414/consoleFull) for PR 4562 at commit [`cbb5460`](https://github.com/apache/spark/commit/cbb5460e5fb4565f2d39e0ce71c00899ec7e8a7e).
* This patch **passes all tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/4562#discussion_r24791828
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala ---
@@ -208,14 +208,14 @@ private[hive] class HiveMetastoreCatalog(hive: HiveContext) extends Catalog with
ParquetRelation2(
paths,
Map(ParquetRelation2.METASTORE_SCHEMA -> metastoreSchema.json),
- None,
+ Some(metastoreSchema),
Some(partitionSpec))(hive))
} else {
val paths = Seq(metastoreRelation.hiveQlTable.getDataLocation.toString)
- LogicalRelation(
- ParquetRelation2(
+ LogicalRelation(ParquetRelation2(
paths,
- Map(ParquetRelation2.METASTORE_SCHEMA -> metastoreSchema.json))(hive))
+ Map(ParquetRelation2.METASTORE_SCHEMA -> metastoreSchema.json),
+ Some(metastoreSchema))(hive))
--- End diff --
OK, we can leave this file unchanged.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/4562#issuecomment-74634837
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/27615/
Test PASSed.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SQL] [Minor] Passdown the schema for Parquet ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4562#issuecomment-74047025
[Test build #27344 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27344/consoleFull) for PR 4562 at commit [`33867c0`](https://github.com/apache/spark/commit/33867c09248b22cca8456268af176f17206a4b74).
* This patch **fails Spark unit tests**.
* This patch merges cleanly.
* This patch adds no public classes.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SQL] [Minor] Passdown the schema for Parquet ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4562#issuecomment-74041731
[Test build #27344 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27344/consoleFull) for PR 4562 at commit [`33867c0`](https://github.com/apache/spark/commit/33867c09248b22cca8456268af176f17206a4b74).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/4562#discussion_r24793987
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/parquet/newParquet.scala ---
@@ -630,11 +635,12 @@ object ParquetRelation2 {
sqlContext.conf.isParquetBinaryAsString,
sqlContext.conf.isParquetINT96AsTimestamp))
}
- }.reduce { (left, right) =>
- try left.merge(right) catch { case e: Throwable =>
- throw new SparkException(s"Failed to merge incompatible schemas $left and $right", e)
- }
- }
+ }.foldLeft[StructType](null) {
--- End diff --
All right. Instead of putting a large code block in `Option`, how about use a temporary `val` and then use `Option` at the end of this method.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by chenghao-intel <gi...@git.apache.org>.
Github user chenghao-intel commented on a diff in the pull request:
https://github.com/apache/spark/pull/4562#discussion_r24797498
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/parquet/parquetSuites.scala ---
@@ -121,13 +121,50 @@ class ParquetDataSourceOnMetastoreSuite extends ParquetMetastoreSuiteBase {
override def beforeAll(): Unit = {
super.beforeAll()
+
+ sql(s"""
+ create table test_parquet
+ (
+ intField INT,
+ stringField STRING
+ )
+ ROW FORMAT SERDE 'org.apache.hadoop.hive.ql.io.parquet.serde.ParquetHiveSerDe'
+ STORED AS
+ INPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat'
+ OUTPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetOutputFormat'
+ """)
+
+ val rdd = sparkContext.parallelize((1 to 10).map(i => s"""{"a":$i, "b":"str${i}"}"""))
+ jsonRDD(rdd).registerTempTable("jt")
+ sql("""
+ create table test ROW FORMAT
+ | SERDE 'org.apache.hadoop.hive.ql.io.parquet.serde.ParquetHiveSerDe'
+ | STORED AS INPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat'
+ | OUTPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetOutputFormat'
+ | AS select * from jt""".stripMargin)
+
--- End diff --
Oh, i thought `STORED AS PARQUERT AS ..` is just the syntactic sugar. Unfortunately, all of the test suite are implemented in the sub project `sql`, but the `HiveShim` is in the subproject `hive` with `hive` package accessing visibility.
Let's put this test in another PR?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by chenghao-intel <gi...@git.apache.org>.
Github user chenghao-intel closed the pull request at:
https://github.com/apache/spark/pull/4562
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by chenghao-intel <gi...@git.apache.org>.
Github user chenghao-intel commented on the pull request:
https://github.com/apache/spark/pull/4562#issuecomment-74803640
Thank you @yhuai , I am closing this PR. :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SQL] [Minor] Passdown the schema for Parquet ...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/4562#issuecomment-74198792
[Test build #27414 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/27414/consoleFull) for PR 4562 at commit [`cbb5460`](https://github.com/apache/spark/commit/cbb5460e5fb4565f2d39e0ce71c00899ec7e8a7e).
* This patch merges cleanly.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by chenghao-intel <gi...@git.apache.org>.
Github user chenghao-intel commented on a diff in the pull request:
https://github.com/apache/spark/pull/4562#discussion_r24793692
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/parquet/newParquet.scala ---
@@ -630,11 +635,12 @@ object ParquetRelation2 {
sqlContext.conf.isParquetBinaryAsString,
sqlContext.conf.isParquetINT96AsTimestamp))
}
- }.reduce { (left, right) =>
- try left.merge(right) catch { case e: Throwable =>
- throw new SparkException(s"Failed to merge incompatible schemas $left and $right", e)
- }
- }
+ }.foldLeft[StructType](null) {
--- End diff --
Yeah, I was trying that also, but seems using `null` is more simple, as `Option` requires some more value extracting code.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/4562#discussion_r24791423
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala ---
@@ -208,14 +208,14 @@ private[hive] class HiveMetastoreCatalog(hive: HiveContext) extends Catalog with
ParquetRelation2(
paths,
Map(ParquetRelation2.METASTORE_SCHEMA -> metastoreSchema.json),
- None,
+ Some(metastoreSchema),
Some(partitionSpec))(hive))
} else {
val paths = Seq(metastoreRelation.hiveQlTable.getDataLocation.toString)
- LogicalRelation(
- ParquetRelation2(
+ LogicalRelation(ParquetRelation2(
paths,
- Map(ParquetRelation2.METASTORE_SCHEMA -> metastoreSchema.json))(hive))
+ Map(ParquetRelation2.METASTORE_SCHEMA -> metastoreSchema.json),
+ Some(metastoreSchema))(hive))
--- End diff --
Oh, but if we do create table, we have to pass the metastore schema.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/4562#discussion_r24793769
--- Diff: sql/hive/src/test/scala/org/apache/spark/sql/parquet/parquetSuites.scala ---
@@ -121,13 +121,50 @@ class ParquetDataSourceOnMetastoreSuite extends ParquetMetastoreSuiteBase {
override def beforeAll(): Unit = {
super.beforeAll()
+
+ sql(s"""
+ create table test_parquet
+ (
+ intField INT,
+ stringField STRING
+ )
+ ROW FORMAT SERDE 'org.apache.hadoop.hive.ql.io.parquet.serde.ParquetHiveSerDe'
+ STORED AS
+ INPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat'
+ OUTPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetOutputFormat'
+ """)
+
+ val rdd = sparkContext.parallelize((1 to 10).map(i => s"""{"a":$i, "b":"str${i}"}"""))
+ jsonRDD(rdd).registerTempTable("jt")
+ sql("""
+ create table test ROW FORMAT
+ | SERDE 'org.apache.hadoop.hive.ql.io.parquet.serde.ParquetHiveSerDe'
+ | STORED AS INPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetInputFormat'
+ | OUTPUTFORMAT 'org.apache.hadoop.hive.ql.io.parquet.MapredParquetOutputFormat'
+ | AS select * from jt""".stripMargin)
+
--- End diff --
How about we use `if (HiveShim.version =="0.13.1")` to check the Hive version like what we did in https://github.com/apache/spark/commit/e0490e271d078aa55d7c7583e2ba80337ed1b0c4.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by liancheng <gi...@git.apache.org>.
Github user liancheng commented on a diff in the pull request:
https://github.com/apache/spark/pull/4562#discussion_r24813338
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala ---
@@ -208,14 +208,14 @@ private[hive] class HiveMetastoreCatalog(hive: HiveContext) extends Catalog with
ParquetRelation2(
paths,
Map(ParquetRelation2.METASTORE_SCHEMA -> metastoreSchema.json),
- None,
+ Some(metastoreSchema),
Some(partitionSpec))(hive))
} else {
val paths = Seq(metastoreRelation.hiveQlTable.getDataLocation.toString)
- LogicalRelation(
- ParquetRelation2(
+ LogicalRelation(ParquetRelation2(
paths,
- Map(ParquetRelation2.METASTORE_SCHEMA -> metastoreSchema.json))(hive))
+ Map(ParquetRelation2.METASTORE_SCHEMA -> metastoreSchema.json),
+ Some(metastoreSchema))(hive))
--- End diff --
Yeah, evil case insensitivity...
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/4562#discussion_r24791099
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala ---
@@ -208,14 +208,14 @@ private[hive] class HiveMetastoreCatalog(hive: HiveContext) extends Catalog with
ParquetRelation2(
paths,
Map(ParquetRelation2.METASTORE_SCHEMA -> metastoreSchema.json),
- None,
+ Some(metastoreSchema),
Some(partitionSpec))(hive))
} else {
val paths = Seq(metastoreRelation.hiveQlTable.getDataLocation.toString)
- LogicalRelation(
- ParquetRelation2(
+ LogicalRelation(ParquetRelation2(
paths,
- Map(ParquetRelation2.METASTORE_SCHEMA -> metastoreSchema.json))(hive))
+ Map(ParquetRelation2.METASTORE_SCHEMA -> metastoreSchema.json),
+ Some(metastoreSchema))(hive))
--- End diff --
I think we cannot do it. See https://github.com/apache/spark/blob/master/sql/hive/src/main/scala/org/apache/spark/sql/hive/HiveMetastoreCatalog.scala#L194
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/4562#discussion_r24791812
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/parquet/newParquet.scala ---
@@ -630,11 +635,12 @@ object ParquetRelation2 {
sqlContext.conf.isParquetBinaryAsString,
sqlContext.conf.isParquetINT96AsTimestamp))
}
- }.reduce { (left, right) =>
- try left.merge(right) catch { case e: Throwable =>
- throw new SparkException(s"Failed to merge incompatible schemas $left and $right", e)
- }
- }
+ }.foldLeft[StructType](null) {
--- End diff --
How about using `None` instead of `null`?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
[GitHub] spark pull request: [SPARK-5852] [SQL] Passdown the schema for Par...
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/4562#discussion_r24793676
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/parquet/newParquet.scala ---
@@ -287,7 +287,11 @@ case class ParquetRelation2(
}
}
- parquetSchema = maybeSchema.getOrElse(readSchema())
+ try {
+ parquetSchema = readSchema().getOrElse(maybeSchema.get)
+ } catch {
+ case e => throw new SparkException(s"Failed to find schema for ${paths.mkString(",")}", e)
+ }
--- End diff --
Also, seems we do not need `try ... catch` at here.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastructure@apache.org or file a JIRA ticket
with INFRA.
---
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org