You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by cloud-fan <gi...@git.apache.org> on 2017/01/20 06:10:56 UTC
[GitHub] spark pull request #16655: [SPARK-19305][SQL] partitioned table should alway...
GitHub user cloud-fan opened a pull request:
https://github.com/apache/spark/pull/16655
[SPARK-19305][SQL] partitioned table should always put partition columns at the end of table schema
## What changes were proposed in this pull request?
For data source tables, we will always reorder the specified table schema, or the query in CTAS, to put partition columns at the end. e.g. `CREATE TABLE t(a int, b int, c int, d int) USING parquet PARTITIONED BY (d, b)` will create a table with schema `<a, c, d, b>`
Hive serde tables don't have this problem before, because its CREATE TABLE syntax specifies data schema and partition schema individually.
However, after we unifed the CREATE TABLE syntax, Hive serde table also need to do the reorder. This PR puts the reorder logic in a analyzer rule, which works with both data source tables and Hive serde tables.
## How was this patch tested?
new regression test
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/cloud-fan/spark schema
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/16655.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 #16655
----
commit 9ec7d36a198560441e3c3e96fa59789bdd36751b
Author: Wenchen Fan <we...@databricks.com>
Date: 2017-01-20T06:10:36Z
partitioned table should always put partition columns at the end of table 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 issue #16655: [SPARK-19305][SQL] partitioned table should always put p...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the issue:
https://github.com/apache/spark/pull/16655
LGTM pending test
---
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 issue #16655: [SPARK-19305][SQL] partitioned table should always put p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16655
Merged build finished. 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 issue #16655: [SPARK-19305][SQL] partitioned table should always put p...
Posted by windpiger <gi...@git.apache.org>.
Github user windpiger commented on the issue:
https://github.com/apache/spark/pull/16655
LGTM, after this merged, I will contiune the work #16593 thanks~
---
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 issue #16655: [SPARK-19305][SQL] partitioned table should always put p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16655
**[Test build #71705 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71705/testReport)** for PR 16655 at commit [`9ec7d36`](https://github.com/apache/spark/commit/9ec7d36a198560441e3c3e96fa59789bdd36751b).
---
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 issue #16655: [SPARK-19305][SQL] partitioned table should always put p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16655
**[Test build #71714 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71714/testReport)** for PR 16655 at commit [`9ec7d36`](https://github.com/apache/spark/commit/9ec7d36a198560441e3c3e96fa59789bdd36751b).
---
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 issue #16655: [SPARK-19305][SQL] partitioned table should always put p...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/16655
retest this please
---
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 issue #16655: [SPARK-19305][SQL] partitioned table should always put p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16655
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71754/
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 issue #16655: [SPARK-19305][SQL] partitioned table should always put p...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/16655
cc @yhuai @gatorsmile @windpiger
---
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 issue #16655: [SPARK-19305][SQL] partitioned table should always put p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16655
Merged build finished. 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 #16655: [SPARK-19305][SQL] partitioned table should alway...
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/16655#discussion_r97189247
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala ---
@@ -199,31 +199,52 @@ case class AnalyzeCreateTable(sparkSession: SparkSession) extends Rule[LogicalPl
// * can't use all table columns as partition columns.
// * partition columns' type must be AtomicType.
// * sort columns' type must be orderable.
+ // * reorder table schema or output of query plan, to put partition columns at the end.
case c @ CreateTable(tableDesc, _, query) =>
- val analyzedQuery = query.map { q =>
- // Analyze the query in CTAS and then we can do the normalization and checking.
- val qe = sparkSession.sessionState.executePlan(q)
+ if (query.isDefined) {
+ val qe = sparkSession.sessionState.executePlan(query.get)
qe.assertAnalyzed()
- qe.analyzed
- }
- val schema = if (analyzedQuery.isDefined) {
- analyzedQuery.get.schema
- } else {
- tableDesc.schema
- }
+ val analyzedQuery = qe.analyzed
+
+ val normalizedTable = normalizeCatalogTable(analyzedQuery.schema, tableDesc)
+
+ val output = analyzedQuery.output
+ val partitionAttrs = normalizedTable.partitionColumnNames.map { partCol =>
+ output.find(_.name == partCol).get
+ }
+ val newOutput = output.filterNot(partitionAttrs.contains) ++ partitionAttrs
+ val reorderedQuery = if (newOutput == output) {
+ analyzedQuery
+ } else {
+ Project(newOutput, analyzedQuery)
+ }
- val columnNames = if (sparkSession.sessionState.conf.caseSensitiveAnalysis) {
- schema.map(_.name)
+ c.copy(tableDesc = normalizedTable, query = Some(reorderedQuery))
--- End diff --
How about adding one more check here?
```Scala
assert(normalizedTable.schema.isEmpty,
"Schema may not be specified in a Create Table As Select (CTAS) statement")
```
---
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 issue #16655: [SPARK-19305][SQL] partitioned table should always put p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16655
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71714/
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 issue #16655: [SPARK-19305][SQL] partitioned table should always put p...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on the issue:
https://github.com/apache/spark/pull/16655
thanks for the review, merging to master!
---
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 issue #16655: [SPARK-19305][SQL] partitioned table should always put p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16655
**[Test build #71754 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71754/testReport)** for PR 16655 at commit [`68f639e`](https://github.com/apache/spark/commit/68f639e468333faa9070cca639b3b491585b2e39).
* 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 issue #16655: [SPARK-19305][SQL] partitioned table should always put p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16655
**[Test build #71714 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71714/testReport)** for PR 16655 at commit [`9ec7d36`](https://github.com/apache/spark/commit/9ec7d36a198560441e3c3e96fa59789bdd36751b).
* 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 issue #16655: [SPARK-19305][SQL] partitioned table should always put p...
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the issue:
https://github.com/apache/spark/pull/16655
**[Test build #71754 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/71754/testReport)** for PR 16655 at commit [`68f639e`](https://github.com/apache/spark/commit/68f639e468333faa9070cca639b3b491585b2e39).
---
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 issue #16655: [SPARK-19305][SQL] partitioned table should always put p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16655
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/71705/
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 #16655: [SPARK-19305][SQL] partitioned table should alway...
Posted by cloud-fan <gi...@git.apache.org>.
Github user cloud-fan commented on a diff in the pull request:
https://github.com/apache/spark/pull/16655#discussion_r97189276
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/rules.scala ---
@@ -199,31 +199,52 @@ case class AnalyzeCreateTable(sparkSession: SparkSession) extends Rule[LogicalPl
// * can't use all table columns as partition columns.
// * partition columns' type must be AtomicType.
// * sort columns' type must be orderable.
+ // * reorder table schema or output of query plan, to put partition columns at the end.
case c @ CreateTable(tableDesc, _, query) =>
- val analyzedQuery = query.map { q =>
- // Analyze the query in CTAS and then we can do the normalization and checking.
- val qe = sparkSession.sessionState.executePlan(q)
+ if (query.isDefined) {
+ val qe = sparkSession.sessionState.executePlan(query.get)
qe.assertAnalyzed()
- qe.analyzed
- }
- val schema = if (analyzedQuery.isDefined) {
- analyzedQuery.get.schema
- } else {
- tableDesc.schema
- }
+ val analyzedQuery = qe.analyzed
+
+ val normalizedTable = normalizeCatalogTable(analyzedQuery.schema, tableDesc)
+
+ val output = analyzedQuery.output
+ val partitionAttrs = normalizedTable.partitionColumnNames.map { partCol =>
+ output.find(_.name == partCol).get
+ }
+ val newOutput = output.filterNot(partitionAttrs.contains) ++ partitionAttrs
+ val reorderedQuery = if (newOutput == output) {
+ analyzedQuery
+ } else {
+ Project(newOutput, analyzedQuery)
+ }
- val columnNames = if (sparkSession.sessionState.conf.caseSensitiveAnalysis) {
- schema.map(_.name)
+ c.copy(tableDesc = normalizedTable, query = Some(reorderedQuery))
--- End diff --
this should be guaranteed by the parser, but we can check it again 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
[GitHub] spark issue #16655: [SPARK-19305][SQL] partitioned table should always put p...
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the issue:
https://github.com/apache/spark/pull/16655
Merged build finished. 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 #16655: [SPARK-19305][SQL] partitioned table should alway...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/16655
---
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