You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by andrewor14 <gi...@git.apache.org> on 2016/04/09 08:56:20 UTC
[GitHub] spark pull request: [SPARK-14388][SQL][WIP] Implement CREATE TABLE
GitHub user andrewor14 opened a pull request:
https://github.com/apache/spark/pull/12271
[SPARK-14388][SQL][WIP] Implement CREATE TABLE
## What changes were proposed in this pull request?
This patch implements the `CREATE TABLE` command using the `SessionCatalog`. Previously we handled only `CTAS` and `CREATE TABLE ... USING`. This requires us to refactor `CatalogTable` to accept various fields (e.g. bucket and skew columns) and pass them to Hive.
## How was this patch tested?
Tests will come in a future commit.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/andrewor14/spark create-table-ddl
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/spark/pull/12271.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 #12271
----
commit 014c38e28e8f4545f926ef60ccb2ee4acae07b59
Author: Andrew Or <an...@databricks.com>
Date: 2016-04-08T21:32:39Z
Parse various parts of the CREATE TABLE command
We need to reconcile the differences between what's added here in
SparkSqlParser and HiveSqlParser. That will come in the next
commit.
This currently still fails tests, obviously because create table
is not implemented yet!
commit 15bb3b6c76e61d708538bee5d797981689ab6a8f
Author: Andrew Or <an...@databricks.com>
Date: 2016-04-01T21:20:37Z
Refactor CatalogTable column semantics
Before: CatalogTable has schema, partitionColumns and sortColumns.
There are no constraints between the 3. However, Hive will
complain if schema and partitionColumns overlap.
After: CatalogTable has schema, partitionColumnNames,
sortColumnNames, bucketColumnNames and skewColumnNames. All the
columns must be a subset of schema. This means splitting up
schema into (schema, partitionCols) before passing it to Hive.
This allows us to store the columns more uniformly. Otherwise
partition columns would be the odd one out. This commit also
fixes "alter table bucketing", which was incorrectly using
partition columns as bucket columns.
commit b6b4d293c2efeb537110ef56fa9ffdcad90c9bb0
Author: Andrew Or <an...@databricks.com>
Date: 2016-04-09T00:53:18Z
Implement CREATE TABLE in Hive parser
This involves reverting part of the changes in an earlier commit,
where we tried to implement the parsing logic in the general SQL
parser and introduced a bunch of case classes that we won't end
up using.
As of this commit the actual CREATE TABLE logic is not there yet.
It will come in a future commit.
commit 5e0fe03bfa655c6de854bc8adaa73186a17a0b0c
Author: Andrew Or <an...@databricks.com>
Date: 2016-04-09T06:52:20Z
Implement it
----
---
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-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59577033
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -675,34 +680,38 @@ private[hive] class HiveClientImpl(
private def toHiveTable(table: CatalogTable): HiveTable = {
val hiveTable = new HiveTable(table.database, table.identifier.table)
- // For EXTERNAL_TABLE/MANAGED_TABLE, we also need to set EXTERNAL field in
- // the table properties accodringly. Otherwise, if EXTERNAL_TABLE is the table type
- // but EXTERNAL field is not set, Hive metastore will change the type to
- // MANAGED_TABLE (see
- // metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L1095-L1105)
+ // For EXTERNAL_TABLE, we also need to set EXTERNAL field in the table properties.
+ // Otherwise, Hive metastore will change the table to a MANAGED_TABLE.
+ // (metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L1095-L1105)
hiveTable.setTableType(table.tableType match {
case CatalogTableType.EXTERNAL_TABLE =>
hiveTable.setProperty("EXTERNAL", "TRUE")
HiveTableType.EXTERNAL_TABLE
case CatalogTableType.MANAGED_TABLE =>
- hiveTable.setProperty("EXTERNAL", "FALSE")
HiveTableType.MANAGED_TABLE
case CatalogTableType.INDEX_TABLE => HiveTableType.INDEX_TABLE
case CatalogTableType.VIRTUAL_VIEW => HiveTableType.VIRTUAL_VIEW
})
- hiveTable.setFields(table.schema.map(toHiveColumn).asJava)
- hiveTable.setPartCols(table.partitionColumns.map(toHiveColumn).asJava)
+ // Note: In Hive the schema and partition columns must be disjoint sets
+ val (partCols, schema) = table.schema.map(toHiveColumn).partition { c =>
+ table.partitionColumnNames.contains(c.getName)
+ }
+ hiveTable.setFields(schema.asJava)
+ hiveTable.setPartCols(partCols.asJava)
// TODO: set sort columns here too
+ hiveTable.setBucketCols(table.bucketColumnNames.asJava)
hiveTable.setOwner(conf.getUser)
hiveTable.setNumBuckets(table.numBuckets)
hiveTable.setCreateTime((table.createTime / 1000).toInt)
hiveTable.setLastAccessTime((table.lastAccessTime / 1000).toInt)
table.storage.locationUri.foreach { loc => shim.setDataLocation(hiveTable, loc) }
table.storage.inputFormat.map(toInputFormat).foreach(hiveTable.setInputFormatClass)
table.storage.outputFormat.map(toOutputFormat).foreach(hiveTable.setOutputFormatClass)
- table.storage.serde.foreach(hiveTable.setSerializationLib)
+ hiveTable.setSerializationLib(
+ table.storage.serde.getOrElse("org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe"))
--- End diff --
I submitted PR to try it out (https://github.com/apache/spark/pull/12363). You can find my change in the last commit (https://github.com/apache/spark/pull/12363/commits/ab70cb751cce8ca2e0757b9fa523534207864328).
---
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-14388][SQL] Implement CREATE TABLE
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/spark/pull/12271
---
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-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209128481
**[Test build #55654 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55654/consoleFull)** for PR 12271 at commit [`efecac9`](https://github.com/apache/spark/commit/efecac9b01b3ff8be296234392f4a6c922fa2d25).
---
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209283002
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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59491556
--- Diff: sql/hive/compatibility/src/test/scala/org/apache/spark/sql/hive/execution/HiveCompatibilitySuite.scala ---
@@ -790,14 +821,13 @@ class HiveCompatibilitySuite extends HiveQueryFileTest with BeforeAndAfter {
"nullscript",
"optional_outer",
"orc_dictionary_threshold",
- "orc_empty_files",
"order",
"order2",
"outer_join_ppr",
"parallel",
"parenthesis_star_by",
"part_inherit_tbl_props",
- "part_inherit_tbl_props_empty",
+ //"part_inherit_tbl_props_empty", // TODO: results don't match
--- End diff --
This one and others that are commented out below are caused by differences of describe table. There are mainly three differences. First, for managed tables, we will EXTERNAL set to false in table properties. For tables that are not bucketed, we will have numBuckets set to 0 instead of -1. For tables that does not specify the file format, the output format will be org.apache.hadoop.hive.ql.io.IgnoreKeyTextOutputFormat instead of org.apache.hadoop.hive.ql.io.HiveIgnoreKeyTextOutputFormat (For Hive, it somehow replaced even if the explain output still shows IgnoreKeyTextOutputFormat). We do not need to block this PR for any of them.
---
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-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209115138
**[Test build #55644 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55644/consoleFull)** for PR 12271 at commit [`250f402`](https://github.com/apache/spark/commit/250f402372e9826865749f3b81cd96a7cdaff657).
* 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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209247087
**[Test build #55692 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55692/consoleFull)** for PR 12271 at commit [`59edce3`](https://github.com/apache/spark/commit/59edce332f87b07bdfb07e2e385431b2b123e1b0).
---
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-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59421647
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -121,84 +123,114 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
}
/**
- * Create a [[CatalogStorageFormat]]. This is part of the [[CreateTableAsSelect]] command.
+ * Create a [[CatalogStorageFormat]] for creating tables.
*/
override def visitCreateFileFormat(
ctx: CreateFileFormatContext): CatalogStorageFormat = withOrigin(ctx) {
- if (ctx.storageHandler == null) {
- typedVisit[CatalogStorageFormat](ctx.fileFormat)
- } else {
- visitStorageHandler(ctx.storageHandler)
+ (ctx.fileFormat, ctx.storageHandler) match {
+ // Expected format: INPUTFORMAT input_format OUTPUTFORMAT output_format
+ case (c: TableFileFormatContext, null) =>
+ visitTableFileFormat(c)
+ // Expected format: SEQUENCEFILE | TEXTFILE | RCFILE | ORC | PARQUET | AVRO
+ case (c: GenericFileFormatContext, null) =>
+ visitGenericFileFormat(c)
+ case (null, storageHandler) =>
+ throw new ParseException("Operation not allowed: ... STORED BY storage_handler ...", ctx)
+ case _ =>
+ throw new ParseException("expected either STORED AS or STORED BY, not both", ctx)
}
}
/**
- * Create a [[CreateTableAsSelect]] command.
+ * Create a table, returning either a [[CreateTable]] or a [[CreateTableAsSelect]].
+ *
+ * This is not used to create datasource tables, which is handled through
+ * "CREATE TABLE ... USING ...".
+ *
+ * Note: several features are currently not supported - temporary tables, bucketing,
+ * skewed columns and storage handlers (STORED BY).
+ *
+ * Expected format:
+ * {{{
+ * CREATE [TEMPORARY] [EXTERNAL] TABLE [IF NOT EXISTS] [db_name.]table_name
+ * [(col1 data_type [COMMENT col_comment], ...)]
+ * [COMMENT table_comment]
+ * [PARTITIONED BY (col3 data_type [COMMENT col_comment], ...)]
+ * [CLUSTERED BY (col1, ...) [SORTED BY (col1 [ASC|DESC], ...)] INTO num_buckets BUCKETS]
+ * [SKEWED BY (col1, col2, ...) ON ((col_value, col_value, ...), ...) [STORED AS DIRECTORIES]]
--- End diff --
Remove this line while resolving the conflicts?
---
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-14388][SQL] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209536129
OK, I cherry-picked your changes and updated the comment.
---
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-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-208623925
**[Test build #55552 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55552/consoleFull)** for PR 12271 at commit [`2e95ecf`](https://github.com/apache/spark/commit/2e95ecf790dc5d5b12b6ec72c0bd2b4bca99b17d).
---
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-14388][SQL] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59426592
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -134,82 +136,117 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
}
/**
- * Create a [[CatalogStorageFormat]]. This is part of the [[CreateTableAsSelect]] command.
+ * Create a [[CatalogStorageFormat]] for creating tables.
*/
override def visitCreateFileFormat(
ctx: CreateFileFormatContext): CatalogStorageFormat = withOrigin(ctx) {
- if (ctx.storageHandler == null) {
- typedVisit[CatalogStorageFormat](ctx.fileFormat)
- } else {
- visitStorageHandler(ctx.storageHandler)
+ (ctx.fileFormat, ctx.storageHandler) match {
+ case (fileFormat, null) if fileFormat != null =>
+ fileFormat match {
+ // Expected format: INPUTFORMAT input_format OUTPUTFORMAT output_format
+ case c: TableFileFormatContext => visitTableFileFormat(c)
+ // Expected format: SEQUENCEFILE | TEXTFILE | RCFILE | ORC | PARQUET | AVRO
+ case c: GenericFileFormatContext => visitGenericFileFormat(c)
+ }
+ case (null, storageHandler) if storageHandler != null =>
+ throw new ParseException("Operation not allowed: ... STORED BY storage_handler ...", ctx)
+ case (null, null) =>
+ throw new ParseException("expected one of STORED AS or STORED BY", ctx)
+ case _ =>
+ throw new ParseException("expected either STORED AS or STORED BY, not both", ctx)
}
}
/**
- * Create a [[CreateTableAsSelect]] command.
+ * Create a table. TODO: expand this comment!
+ *
+ * For example:
+ * {{{
+ * CREATE [TEMPORARY] [EXTERNAL] TABLE [IF NOT EXISTS] [db_name.]table_name
+ * [(col1 data_type [COMMENT col_comment], ...)]
+ * [COMMENT table_comment]
+ * [PARTITIONED BY (col3 data_type [COMMENT col_comment], ...)]
+ * [CLUSTERED BY (col1, ...) [SORTED BY (col1 [ASC|DESC], ...)] INTO num_buckets BUCKETS]
+ * [SKEWED BY (col1, col2, ...) ON ((col_value, col_value, ...), ...)
+ * [STORED AS DIRECTORIES]
+ * [ROW FORMAT row_format]
+ * [STORED AS file_format | STORED BY storage_handler_class [WITH SERDEPROPERTIES (...)]]
+ * [LOCATION path]
+ * [TBLPROPERTIES (property_name=property_value, ...)]
+ * [AS select_statement];
+ * }}}
*/
override def visitCreateTable(ctx: CreateTableContext): LogicalPlan = {
- if (ctx.query == null) {
- HiveNativeCommand(command(ctx))
+ val (name, temp, ifNotExists, external) = visitCreateTableHeader(ctx.createTableHeader)
+ // TODO: implement temporary tables
+ if (temp) {
+ throw new AnalysisException(
--- End diff --
(I actually fixed this in this patch, but we still need to fix it in all the other places in SPARK-14414)
---
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-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209282220
**[Test build #55693 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55693/consoleFull)** for PR 12271 at commit [`a60e66a`](https://github.com/apache/spark/commit/a60e66a71dec96973043ec62e2b6d4213c5add2c).
* 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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209087617
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: [SPARK-14388][SQL][WIP] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-207731894
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55425/
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209087621
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55634/
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-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209254390
**[Test build #55696 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55696/consoleFull)** for PR 12271 at commit [`02738fe`](https://github.com/apache/spark/commit/02738fed54ba0eebc2c8d887430f0bef34213c68).
---
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209572487
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: [SPARK-14388][SQL][WIP] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-207730021
**[Test build #55425 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55425/consoleFull)** for PR 12271 at commit [`5e0fe03`](https://github.com/apache/spark/commit/5e0fe03bfa655c6de854bc8adaa73186a17a0b0c).
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59111469
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/ddl.scala ---
@@ -176,29 +176,6 @@ case class DescribeDatabase(
}
/**
- * A command that renames a table/view.
- *
- * The syntax of this command is:
- * {{{
- * ALTER TABLE table1 RENAME TO table2;
- * ALTER VIEW view1 RENAME TO view2;
- * }}}
- */
-case class AlterTableRename(
--- End diff --
moved to `tables.scala`. I just moved this one command for now to avoid inflating the diff too much.
---
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-208634664
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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59491102
--- Diff: sql/hive/compatibility/src/test/scala/org/apache/spark/sql/hive/execution/HiveCompatibilitySuite.scala ---
@@ -509,7 +555,7 @@ class HiveCompatibilitySuite extends HiveQueryFileTest with BeforeAndAfter {
"date_comparison",
"date_join1",
"date_serde",
- "decimal_1",
+ //"decimal_1", // TODO: cannot parse column decimal(5)
--- End diff --
Yea. We should support decimal(5). At here, the scale is 0.
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59137814
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -134,82 +136,117 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
}
/**
- * Create a [[CatalogStorageFormat]]. This is part of the [[CreateTableAsSelect]] command.
+ * Create a [[CatalogStorageFormat]] for creating tables.
*/
override def visitCreateFileFormat(
ctx: CreateFileFormatContext): CatalogStorageFormat = withOrigin(ctx) {
- if (ctx.storageHandler == null) {
- typedVisit[CatalogStorageFormat](ctx.fileFormat)
- } else {
- visitStorageHandler(ctx.storageHandler)
+ (ctx.fileFormat, ctx.storageHandler) match {
+ case (fileFormat, null) if fileFormat != null =>
+ fileFormat match {
+ // Expected format: INPUTFORMAT input_format OUTPUTFORMAT output_format
+ case c: TableFileFormatContext => visitTableFileFormat(c)
+ // Expected format: SEQUENCEFILE | TEXTFILE | RCFILE | ORC | PARQUET | AVRO
+ case c: GenericFileFormatContext => visitGenericFileFormat(c)
+ }
+ case (null, storageHandler) if storageHandler != null =>
+ throw new ParseException("Operation not allowed: ... STORED BY storage_handler ...", ctx)
+ case (null, null) =>
--- End diff --
sure
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59138476
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
@@ -218,14 +219,35 @@ case class CatalogTable(
tableType: CatalogTableType,
storage: CatalogStorageFormat,
schema: Seq[CatalogColumn],
- partitionColumns: Seq[CatalogColumn] = Seq.empty,
- sortColumns: Seq[CatalogColumn] = Seq.empty,
+ partitionColumnNames: Seq[String] = Seq.empty,
+ sortColumnNames: Seq[String] = Seq.empty,
--- End diff --
I think we do not support SORTED BY and CLUSTERED BY 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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59445656
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -121,84 +123,114 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
}
/**
- * Create a [[CatalogStorageFormat]]. This is part of the [[CreateTableAsSelect]] command.
+ * Create a [[CatalogStorageFormat]] for creating tables.
*/
override def visitCreateFileFormat(
ctx: CreateFileFormatContext): CatalogStorageFormat = withOrigin(ctx) {
- if (ctx.storageHandler == null) {
- typedVisit[CatalogStorageFormat](ctx.fileFormat)
- } else {
- visitStorageHandler(ctx.storageHandler)
+ (ctx.fileFormat, ctx.storageHandler) match {
+ // Expected format: INPUTFORMAT input_format OUTPUTFORMAT output_format
+ case (c: TableFileFormatContext, null) =>
+ visitTableFileFormat(c)
+ // Expected format: SEQUENCEFILE | TEXTFILE | RCFILE | ORC | PARQUET | AVRO
+ case (c: GenericFileFormatContext, null) =>
+ visitGenericFileFormat(c)
+ case (null, storageHandler) =>
+ throw new ParseException("Operation not allowed: ... STORED BY storage_handler ...", ctx)
+ case _ =>
+ throw new ParseException("expected either STORED AS or STORED BY, not both", ctx)
}
}
/**
- * Create a [[CreateTableAsSelect]] command.
+ * Create a table, returning either a [[CreateTable]] or a [[CreateTableAsSelect]].
+ *
+ * This is not used to create datasource tables, which is handled through
+ * "CREATE TABLE ... USING ...".
+ *
+ * Note: several features are currently not supported - temporary tables, bucketing,
+ * skewed columns and storage handlers (STORED BY).
+ *
+ * Expected format:
+ * {{{
+ * CREATE [TEMPORARY] [EXTERNAL] TABLE [IF NOT EXISTS] [db_name.]table_name
+ * [(col1 data_type [COMMENT col_comment], ...)]
+ * [COMMENT table_comment]
+ * [PARTITIONED BY (col3 data_type [COMMENT col_comment], ...)]
+ * [CLUSTERED BY (col1, ...) [SORTED BY (col1 [ASC|DESC], ...)] INTO num_buckets BUCKETS]
+ * [SKEWED BY (col1, col2, ...) ON ((col_value, col_value, ...), ...) [STORED AS DIRECTORIES]]
+ * [ROW FORMAT row_format]
+ * [STORED AS file_format | STORED BY storage_handler_class [WITH SERDEPROPERTIES (...)]]
+ * [LOCATION path]
+ * [TBLPROPERTIES (property_name=property_value, ...)]
+ * [AS select_statement];
+ * }}}
*/
- override def visitCreateTable(ctx: CreateTableContext): LogicalPlan = {
- if (ctx.query == null) {
- HiveNativeCommand(command(ctx))
+ override def visitCreateTable(ctx: CreateTableContext): LogicalPlan = withOrigin(ctx) {
+ val (name, temp, ifNotExists, external) = visitCreateTableHeader(ctx.createTableHeader)
+ // TODO: implement temporary tables
+ if (temp) {
+ throw new ParseException(
+ "CREATE TEMPORARY TABLE is not supported yet. " +
+ "Please use registerTempTable as an alternative.", ctx)
+ }
+ if (ctx.skewSpec != null) {
+ throw new ParseException("Operation not allowed: CREATE TABLE ... SKEWED BY ...", ctx)
+ }
+ if (ctx.bucketSpec != null) {
+ throw new ParseException("Operation not allowed: CREATE TABLE ... CLUSTERED BY ...", ctx)
+ }
+ val tableType = if (external) {
+ CatalogTableType.EXTERNAL_TABLE
} else {
- // Get the table header.
- val (table, temp, ifNotExists, external) = visitCreateTableHeader(ctx.createTableHeader)
- val tableType = if (external) {
- CatalogTableType.EXTERNAL_TABLE
- } else {
- CatalogTableType.MANAGED_TABLE
- }
-
- // Unsupported clauses.
- if (temp) {
- throw new ParseException(s"Unsupported operation: TEMPORARY clause.", ctx)
- }
- if (ctx.bucketSpec != null) {
- // TODO add this - we need cluster columns in the CatalogTable for this to work.
- throw new ParseException("Unsupported operation: " +
- "CLUSTERED BY ... [ORDERED BY ...] INTO ... BUCKETS clause.", ctx)
- }
- if (ctx.skewSpec != null) {
- throw new ParseException("Operation not allowed: " +
- "SKEWED BY ... ON ... [STORED AS DIRECTORIES] clause.", ctx)
- }
-
- // Create the schema.
- val schema = Option(ctx.columns).toSeq.flatMap(visitCatalogColumns(_, _.toLowerCase))
-
- // Get the column by which the table is partitioned.
- val partitionCols = Option(ctx.partitionColumns).toSeq.flatMap(visitCatalogColumns(_))
-
- // Create the storage.
- def format(fmt: ParserRuleContext): CatalogStorageFormat = {
- Option(fmt).map(typedVisit[CatalogStorageFormat]).getOrElse(EmptyStorageFormat)
- }
- // Default storage.
+ CatalogTableType.MANAGED_TABLE
+ }
+ val comment = Option(ctx.STRING).map(string)
+ val partitionCols = Option(ctx.partitionColumns).toSeq.flatMap(visitCatalogColumns(_))
+ val cols = Option(ctx.columns).toSeq.flatMap(visitCatalogColumns(_, _.toLowerCase))
+ val properties = Option(ctx.tablePropertyList).map(visitTablePropertyList).getOrElse(Map.empty)
+ val selectQuery = Option(ctx.query).map(plan)
+
+ // Note: Hive requires partition columns to be distinct from the schema, so we need
+ // to include the partition columns here explicitly
+ val schema = cols ++ partitionCols
+
+ // Storage format
+ val defaultStorage: CatalogStorageFormat = {
val defaultStorageType = hiveConf.getVar(HiveConf.ConfVars.HIVEDEFAULTFILEFORMAT)
- val hiveSerDe = HiveSerDe.sourceToSerDe(defaultStorageType, hiveConf).getOrElse {
- HiveSerDe(
- inputFormat = Option("org.apache.hadoop.mapred.TextInputFormat"),
- outputFormat = Option("org.apache.hadoop.hive.ql.io.IgnoreKeyTextOutputFormat"))
- }
- // Defined storage.
- val fileStorage = format(ctx.createFileFormat)
- val rowStorage = format(ctx.rowFormat)
- val storage = CatalogStorageFormat(
- Option(ctx.locationSpec).map(visitLocationSpec),
- fileStorage.inputFormat.orElse(hiveSerDe.inputFormat),
- fileStorage.outputFormat.orElse(hiveSerDe.outputFormat),
- rowStorage.serde.orElse(hiveSerDe.serde).orElse(fileStorage.serde),
- rowStorage.serdeProperties ++ fileStorage.serdeProperties
- )
+ val defaultHiveSerde = HiveSerDe.sourceToSerDe(defaultStorageType, hiveConf)
+ CatalogStorageFormat(
+ locationUri = None,
+ inputFormat = defaultHiveSerde.flatMap(_.inputFormat)
+ .orElse(Option("org.apache.hadoop.mapred.TextInputFormat")),
+ outputFormat = defaultHiveSerde.flatMap(_.outputFormat)
+ .orElse(Option("org.apache.hadoop.hive.ql.io.IgnoreKeyTextOutputFormat")),
+ serde = defaultHiveSerde.flatMap(_.serde),
+ serdeProperties = Map())
+ }
+ val fileStorage = Option(ctx.createFileFormat).map(visitCreateFileFormat)
+ .getOrElse(EmptyStorageFormat)
+ val rowStorage = Option(ctx.rowFormat).map(visitRowFormat).getOrElse(EmptyStorageFormat)
+ val location = Option(ctx.locationSpec).map(visitLocationSpec)
+ val storage = CatalogStorageFormat(
+ locationUri = location,
+ inputFormat = fileStorage.inputFormat.orElse(defaultStorage.inputFormat),
+ outputFormat = fileStorage.outputFormat.orElse(defaultStorage.outputFormat),
+ serde = rowStorage.serde.orElse(fileStorage.serde).orElse(defaultStorage.serde),
+ serdeProperties = rowStorage.serdeProperties ++ fileStorage.serdeProperties)
+
+ // TODO support the sql text - have a proper location for this!
--- End diff --
In the old situation we were storing the `comment` into the `viewText` column which didn't seem right. I think we can remove this one now that we actually have a comment field.
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59111501
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -271,8 +271,7 @@ createFileFormat
;
fileFormat
- : INPUTFORMAT inFmt=STRING OUTPUTFORMAT outFmt=STRING (SERDE serdeCls=STRING)?
- (INPUTDRIVER inDriver=STRING OUTPUTDRIVER outDriver=STRING)? #tableFileFormat
--- End diff --
@hvanhovell I deleted this because Hive doesn't support it. Why was this added in the first place? Is there any supporting documentation for this somewhere?
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59132734
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -134,82 +136,117 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
}
/**
- * Create a [[CatalogStorageFormat]]. This is part of the [[CreateTableAsSelect]] command.
+ * Create a [[CatalogStorageFormat]] for creating tables.
*/
override def visitCreateFileFormat(
ctx: CreateFileFormatContext): CatalogStorageFormat = withOrigin(ctx) {
- if (ctx.storageHandler == null) {
- typedVisit[CatalogStorageFormat](ctx.fileFormat)
- } else {
- visitStorageHandler(ctx.storageHandler)
+ (ctx.fileFormat, ctx.storageHandler) match {
+ case (fileFormat, null) if fileFormat != null =>
+ fileFormat match {
+ // Expected format: INPUTFORMAT input_format OUTPUTFORMAT output_format
+ case c: TableFileFormatContext => visitTableFileFormat(c)
+ // Expected format: SEQUENCEFILE | TEXTFILE | RCFILE | ORC | PARQUET | AVRO
+ case c: GenericFileFormatContext => visitGenericFileFormat(c)
+ }
+ case (null, storageHandler) if storageHandler != null =>
+ throw new ParseException("Operation not allowed: ... STORED BY storage_handler ...", ctx)
+ case (null, null) =>
+ throw new ParseException("expected one of STORED AS or STORED BY", ctx)
+ case _ =>
+ throw new ParseException("expected either STORED AS or STORED BY, not both", ctx)
}
}
/**
- * Create a [[CreateTableAsSelect]] command.
+ * Create a table. TODO: expand this comment!
+ *
+ * For example:
+ * {{{
+ * CREATE [TEMPORARY] [EXTERNAL] TABLE [IF NOT EXISTS] [db_name.]table_name
+ * [(col1 data_type [COMMENT col_comment], ...)]
+ * [COMMENT table_comment]
+ * [PARTITIONED BY (col3 data_type [COMMENT col_comment], ...)]
+ * [CLUSTERED BY (col1, ...) [SORTED BY (col1 [ASC|DESC], ...)] INTO num_buckets BUCKETS]
+ * [SKEWED BY (col1, col2, ...) ON ((col_value, col_value, ...), ...)
+ * [STORED AS DIRECTORIES]
+ * [ROW FORMAT row_format]
+ * [STORED AS file_format | STORED BY storage_handler_class [WITH SERDEPROPERTIES (...)]]
+ * [LOCATION path]
+ * [TBLPROPERTIES (property_name=property_value, ...)]
+ * [AS select_statement];
+ * }}}
*/
override def visitCreateTable(ctx: CreateTableContext): LogicalPlan = {
- if (ctx.query == null) {
- HiveNativeCommand(command(ctx))
+ val (name, temp, ifNotExists, external) = visitCreateTableHeader(ctx.createTableHeader)
+ // TODO: implement temporary tables
+ if (temp) {
+ throw new AnalysisException(
--- End diff --
I would throw a ParseException here. The advantage is that the error reporting of these exceptions is a bit better.
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by xwu0226 <gi...@git.apache.org>.
Github user xwu0226 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59137396
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
@@ -218,14 +219,35 @@ case class CatalogTable(
tableType: CatalogTableType,
storage: CatalogStorageFormat,
schema: Seq[CatalogColumn],
- partitionColumns: Seq[CatalogColumn] = Seq.empty,
- sortColumns: Seq[CatalogColumn] = Seq.empty,
+ partitionColumnNames: Seq[String] = Seq.empty,
+ sortColumnNames: Seq[String] = Seq.empty,
--- End diff --
How do we maintain the ordering of the sort columns here? For the clause SORTED BY (col_name [ASC|DESC], ...), within CLUSTERED BY clause.
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-207750220
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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209031556
Thanks for the reviews. Latest commit should pass tests.
---
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209115335
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55644/
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59132631
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -397,11 +429,28 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
}
/**
- * Storage Handlers are currently not supported in the statements we support (CTAS).
+ * Create a [[RowFormat]] used for creating tables.
+ *
+ * Example format:
+ * {{{
+ * SERDE serde_name [WITH SERDEPROPERTIES (k1=v1, k2=v2, ...)]
+ * }}}
+ *
+ * OR
+ *
+ * {{{
+ * DELIMITED [FIELDS TERMINATED BY char [ESCAPED BY char]]
+ * [COLLECTION ITEMS TERMINATED BY char]
+ * [MAP KEYS TERMINATED BY char]
+ * [LINES TERMINATED BY char]
+ * [NULL DEFINED AS char]
+ * }}}
*/
- override def visitStorageHandler(
- ctx: StorageHandlerContext): CatalogStorageFormat = withOrigin(ctx) {
- throw new ParseException("Storage Handlers are currently unsupported.", ctx)
+ private def visitRowFormat(ctx: RowFormatContext): CatalogStorageFormat = withOrigin(ctx) {
+ ctx match {
--- End diff --
Minor: You could also call `ctx.accept(this)` here and cast it, but I get that this is more typed and less opaque.
---
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209284620
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55696/
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-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59421524
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
@@ -218,14 +219,35 @@ case class CatalogTable(
tableType: CatalogTableType,
storage: CatalogStorageFormat,
schema: Seq[CatalogColumn],
- partitionColumns: Seq[CatalogColumn] = Seq.empty,
- sortColumns: Seq[CatalogColumn] = Seq.empty,
+ partitionColumnNames: Seq[String] = Seq.empty,
+ sortColumnNames: Seq[String] = Seq.empty,
+ bucketColumnNames: Seq[String] = Seq.empty,
+ // e.g. (date, country)
+ skewColumnNames: Seq[String] = Seq.empty,
+ // e.g. ('2008-08-08', 'us), ('2009-09-09', 'uk')
+ skewColumnValues: Seq[Seq[String]] = Seq.empty,
numBuckets: Int = 0,
createTime: Long = System.currentTimeMillis,
lastAccessTime: Long = System.currentTimeMillis,
properties: Map[String, String] = Map.empty,
viewOriginalText: Option[String] = None,
- viewText: Option[String] = None) {
+ viewText: Option[String] = None,
+ comment: Option[String] = None) {
--- End diff --
ok
---
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209135567
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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59574261
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -675,34 +680,38 @@ private[hive] class HiveClientImpl(
private def toHiveTable(table: CatalogTable): HiveTable = {
val hiveTable = new HiveTable(table.database, table.identifier.table)
- // For EXTERNAL_TABLE/MANAGED_TABLE, we also need to set EXTERNAL field in
- // the table properties accodringly. Otherwise, if EXTERNAL_TABLE is the table type
- // but EXTERNAL field is not set, Hive metastore will change the type to
- // MANAGED_TABLE (see
- // metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L1095-L1105)
+ // For EXTERNAL_TABLE, we also need to set EXTERNAL field in the table properties.
+ // Otherwise, Hive metastore will change the table to a MANAGED_TABLE.
+ // (metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L1095-L1105)
hiveTable.setTableType(table.tableType match {
case CatalogTableType.EXTERNAL_TABLE =>
hiveTable.setProperty("EXTERNAL", "TRUE")
HiveTableType.EXTERNAL_TABLE
case CatalogTableType.MANAGED_TABLE =>
- hiveTable.setProperty("EXTERNAL", "FALSE")
HiveTableType.MANAGED_TABLE
case CatalogTableType.INDEX_TABLE => HiveTableType.INDEX_TABLE
case CatalogTableType.VIRTUAL_VIEW => HiveTableType.VIRTUAL_VIEW
})
- hiveTable.setFields(table.schema.map(toHiveColumn).asJava)
- hiveTable.setPartCols(table.partitionColumns.map(toHiveColumn).asJava)
+ // Note: In Hive the schema and partition columns must be disjoint sets
+ val (partCols, schema) = table.schema.map(toHiveColumn).partition { c =>
+ table.partitionColumnNames.contains(c.getName)
+ }
+ hiveTable.setFields(schema.asJava)
+ hiveTable.setPartCols(partCols.asJava)
// TODO: set sort columns here too
+ hiveTable.setBucketCols(table.bucketColumnNames.asJava)
hiveTable.setOwner(conf.getUser)
hiveTable.setNumBuckets(table.numBuckets)
hiveTable.setCreateTime((table.createTime / 1000).toInt)
hiveTable.setLastAccessTime((table.lastAccessTime / 1000).toInt)
table.storage.locationUri.foreach { loc => shim.setDataLocation(hiveTable, loc) }
table.storage.inputFormat.map(toInputFormat).foreach(hiveTable.setInputFormatClass)
table.storage.outputFormat.map(toOutputFormat).foreach(hiveTable.setOutputFormatClass)
- table.storage.serde.foreach(hiveTable.setSerializationLib)
+ hiveTable.setSerializationLib(
+ table.storage.serde.getOrElse("org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe"))
--- End diff --
For now, if the schema does not have any field, can we set it to the following one?
```
[# col_name data_type comment ]
[ ]
[col array<string> from deserializer ]
```
So, we try to preserve the existing (and weird) behavior.
---
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-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209194377
**[Test build #55674 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55674/consoleFull)** for PR 12271 at commit [`8e273fd`](https://github.com/apache/spark/commit/8e273fdc4f95d08cb6d09f4641472861587a3a01).
* 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: [SPARK-14388][SQL][WIP] Implement CREATE TABLE
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59132476
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -134,82 +136,117 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
}
/**
- * Create a [[CatalogStorageFormat]]. This is part of the [[CreateTableAsSelect]] command.
+ * Create a [[CatalogStorageFormat]] for creating tables.
*/
override def visitCreateFileFormat(
ctx: CreateFileFormatContext): CatalogStorageFormat = withOrigin(ctx) {
- if (ctx.storageHandler == null) {
- typedVisit[CatalogStorageFormat](ctx.fileFormat)
- } else {
- visitStorageHandler(ctx.storageHandler)
+ (ctx.fileFormat, ctx.storageHandler) match {
+ case (fileFormat, null) if fileFormat != null =>
+ fileFormat match {
+ // Expected format: INPUTFORMAT input_format OUTPUTFORMAT output_format
+ case c: TableFileFormatContext => visitTableFileFormat(c)
+ // Expected format: SEQUENCEFILE | TEXTFILE | RCFILE | ORC | PARQUET | AVRO
+ case c: GenericFileFormatContext => visitGenericFileFormat(c)
+ }
+ case (null, storageHandler) if storageHandler != null =>
+ throw new ParseException("Operation not allowed: ... STORED BY storage_handler ...", ctx)
+ case (null, null) =>
--- End diff --
I don't think this is possible. If they are both empty, the `CreateFileFormatContext` should be null to begin with.
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59123869
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
@@ -218,14 +219,35 @@ case class CatalogTable(
tableType: CatalogTableType,
storage: CatalogStorageFormat,
schema: Seq[CatalogColumn],
- partitionColumns: Seq[CatalogColumn] = Seq.empty,
- sortColumns: Seq[CatalogColumn] = Seq.empty,
+ partitionColumnNames: Seq[String] = Seq.empty,
+ sortColumnNames: Seq[String] = Seq.empty,
+ bucketColumnNames: Seq[String] = Seq.empty,
+ // e.g. (date, country)
+ skewColumnNames: Seq[String] = Seq.empty,
+ // e.g. ('2008-08-08', 'us), ('2009-09-09', 'uk')
+ skewColumnValues: Seq[Seq[String]] = Seq.empty,
numBuckets: Int = 0,
createTime: Long = System.currentTimeMillis,
lastAccessTime: Long = System.currentTimeMillis,
properties: Map[String, String] = Map.empty,
viewOriginalText: Option[String] = None,
- viewText: Option[String] = None) {
+ viewText: Option[String] = None,
+ comment: Option[String] = None) {
+
+ // Verify that the provided columns are part of the schema
+ private val colNames = schema.map(_.name).toSet
+ private def requireSubsetOfSchema(cols: Seq[String], colType: String): Unit = {
+ require(cols.toSet.subsetOf(colNames), s"$colType columns (${cols.mkString(", ")}) " +
+ s"must be a subset of schema (${colNames.mkString(", ")}) in table '$identifier'")
+ }
+ requireSubsetOfSchema(partitionColumnNames, "partition")
--- End diff --
no because we put the partition columns in the schema when we parse it. There's a comment in `HiveSqlParser` that describes this.
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59132489
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -134,82 +136,117 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
}
/**
- * Create a [[CatalogStorageFormat]]. This is part of the [[CreateTableAsSelect]] command.
+ * Create a [[CatalogStorageFormat]] for creating tables.
*/
override def visitCreateFileFormat(
ctx: CreateFileFormatContext): CatalogStorageFormat = withOrigin(ctx) {
- if (ctx.storageHandler == null) {
- typedVisit[CatalogStorageFormat](ctx.fileFormat)
- } else {
- visitStorageHandler(ctx.storageHandler)
+ (ctx.fileFormat, ctx.storageHandler) match {
--- End diff --
The parser either allows a fileFormat or a storageHandler here (not both). So we could simplify this.
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59132688
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -134,82 +136,117 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
}
/**
- * Create a [[CatalogStorageFormat]]. This is part of the [[CreateTableAsSelect]] command.
+ * Create a [[CatalogStorageFormat]] for creating tables.
*/
override def visitCreateFileFormat(
ctx: CreateFileFormatContext): CatalogStorageFormat = withOrigin(ctx) {
- if (ctx.storageHandler == null) {
- typedVisit[CatalogStorageFormat](ctx.fileFormat)
- } else {
- visitStorageHandler(ctx.storageHandler)
+ (ctx.fileFormat, ctx.storageHandler) match {
+ case (fileFormat, null) if fileFormat != null =>
+ fileFormat match {
+ // Expected format: INPUTFORMAT input_format OUTPUTFORMAT output_format
+ case c: TableFileFormatContext => visitTableFileFormat(c)
+ // Expected format: SEQUENCEFILE | TEXTFILE | RCFILE | ORC | PARQUET | AVRO
+ case c: GenericFileFormatContext => visitGenericFileFormat(c)
+ }
+ case (null, storageHandler) if storageHandler != null =>
+ throw new ParseException("Operation not allowed: ... STORED BY storage_handler ...", ctx)
+ case (null, null) =>
+ throw new ParseException("expected one of STORED AS or STORED BY", ctx)
+ case _ =>
+ throw new ParseException("expected either STORED AS or STORED BY, not both", ctx)
}
}
/**
- * Create a [[CreateTableAsSelect]] command.
+ * Create a table. TODO: expand this comment!
+ *
+ * For example:
+ * {{{
+ * CREATE [TEMPORARY] [EXTERNAL] TABLE [IF NOT EXISTS] [db_name.]table_name
+ * [(col1 data_type [COMMENT col_comment], ...)]
+ * [COMMENT table_comment]
+ * [PARTITIONED BY (col3 data_type [COMMENT col_comment], ...)]
+ * [CLUSTERED BY (col1, ...) [SORTED BY (col1 [ASC|DESC], ...)] INTO num_buckets BUCKETS]
+ * [SKEWED BY (col1, col2, ...) ON ((col_value, col_value, ...), ...)
+ * [STORED AS DIRECTORIES]
+ * [ROW FORMAT row_format]
+ * [STORED AS file_format | STORED BY storage_handler_class [WITH SERDEPROPERTIES (...)]]
+ * [LOCATION path]
+ * [TBLPROPERTIES (property_name=property_value, ...)]
+ * [AS select_statement];
+ * }}}
*/
override def visitCreateTable(ctx: CreateTableContext): LogicalPlan = {
--- End diff --
Could you add withOrigin here? I apparently forgot to...
---
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-14388][SQL] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209172015
I've ignored some tests in `HiveCompatibilitySuite` for now. I'll have a look at them shortly. I just wanted to see if the rest of the tests will pass.
---
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-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209283962
**[Test build #55696 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55696/consoleFull)** for PR 12271 at commit [`02738fe`](https://github.com/apache/spark/commit/02738fed54ba0eebc2c8d887430f0bef34213c68).
* 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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209270685
**[Test build #55692 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55692/consoleFull)** for PR 12271 at commit [`59edce3`](https://github.com/apache/spark/commit/59edce332f87b07bdfb07e2e385431b2b123e1b0).
* 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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59580227
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -675,34 +680,38 @@ private[hive] class HiveClientImpl(
private def toHiveTable(table: CatalogTable): HiveTable = {
val hiveTable = new HiveTable(table.database, table.identifier.table)
- // For EXTERNAL_TABLE/MANAGED_TABLE, we also need to set EXTERNAL field in
- // the table properties accodringly. Otherwise, if EXTERNAL_TABLE is the table type
- // but EXTERNAL field is not set, Hive metastore will change the type to
- // MANAGED_TABLE (see
- // metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L1095-L1105)
+ // For EXTERNAL_TABLE, we also need to set EXTERNAL field in the table properties.
+ // Otherwise, Hive metastore will change the table to a MANAGED_TABLE.
+ // (metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L1095-L1105)
hiveTable.setTableType(table.tableType match {
case CatalogTableType.EXTERNAL_TABLE =>
hiveTable.setProperty("EXTERNAL", "TRUE")
HiveTableType.EXTERNAL_TABLE
case CatalogTableType.MANAGED_TABLE =>
- hiveTable.setProperty("EXTERNAL", "FALSE")
HiveTableType.MANAGED_TABLE
case CatalogTableType.INDEX_TABLE => HiveTableType.INDEX_TABLE
case CatalogTableType.VIRTUAL_VIEW => HiveTableType.VIRTUAL_VIEW
})
- hiveTable.setFields(table.schema.map(toHiveColumn).asJava)
- hiveTable.setPartCols(table.partitionColumns.map(toHiveColumn).asJava)
+ // Note: In Hive the schema and partition columns must be disjoint sets
+ val (partCols, schema) = table.schema.map(toHiveColumn).partition { c =>
+ table.partitionColumnNames.contains(c.getName)
+ }
+ hiveTable.setFields(schema.asJava)
+ hiveTable.setPartCols(partCols.asJava)
// TODO: set sort columns here too
+ hiveTable.setBucketCols(table.bucketColumnNames.asJava)
hiveTable.setOwner(conf.getUser)
hiveTable.setNumBuckets(table.numBuckets)
hiveTable.setCreateTime((table.createTime / 1000).toInt)
hiveTable.setLastAccessTime((table.lastAccessTime / 1000).toInt)
table.storage.locationUri.foreach { loc => shim.setDataLocation(hiveTable, loc) }
table.storage.inputFormat.map(toInputFormat).foreach(hiveTable.setInputFormatClass)
table.storage.outputFormat.map(toOutputFormat).foreach(hiveTable.setOutputFormatClass)
- table.storage.serde.foreach(hiveTable.setSerializationLib)
+ hiveTable.setSerializationLib(
+ table.storage.serde.getOrElse("org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe"))
--- End diff --
why are there all these undocumented implicit behaviors in Hive :(
---
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-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209058214
**[Test build #55628 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55628/consoleFull)** for PR 12271 at commit [`250f402`](https://github.com/apache/spark/commit/250f402372e9826865749f3b81cd96a7cdaff657).
* 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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209059940
test 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 pull request: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209058447
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55628/
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-14388][SQL] Implement CREATE TABLE
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209086376
LGTM - pending jenkins
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59118178
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -271,8 +271,7 @@ createFileFormat
;
fileFormat
- : INPUTFORMAT inFmt=STRING OUTPUTFORMAT outFmt=STRING (SERDE serdeCls=STRING)?
- (INPUTDRIVER inDriver=STRING OUTPUTDRIVER outDriver=STRING)? #tableFileFormat
--- End diff --
Let's remove it. I have never seen this before.
---
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-14388][SQL] Implement CREATE TABLE
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59501698
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -435,13 +480,15 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
/**
* Create a sequence of [[CatalogColumn]]s from a column list
*/
- private def visitCatalogColumns(
- ctx: ColTypeListContext,
- formatter: String => String = identity): Seq[CatalogColumn] = withOrigin(ctx) {
+ private def visitCatalogColumns(ctx: ColTypeListContext): Seq[CatalogColumn] = withOrigin(ctx) {
ctx.colType.asScala.map { col =>
CatalogColumn(
- formatter(col.identifier.getText),
- col.dataType.getText.toLowerCase, // TODO validate this?
+ col.identifier.getText.toLowerCase,
+ // Note: for types like "STRUCT<myFirstName: STRING, myLastName: STRING>" we can't
+ // just convert the whole type string to lower case, otherwise the struct field names
+ // will no longer be case sensitive. Instead, we rely on our parser to get the proper
+ // case before passing it to Hive.
+ CatalystSqlParser.parseDataType(col.dataType.getText).simpleString,
--- End diff --
MINOR/NIT: The DataType parsing is done in the `AstBuilder`, so we really don't need to parse this string again. You could use (magical/evil) `typedVisit[DataType](col.dataType)` 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 pull request: [SPARK-14388][SQL][WIP] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59123872
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -665,9 +672,17 @@ private[hive] class HiveClientImpl(
case CatalogTableType.INDEX_TABLE => HiveTableType.INDEX_TABLE
case CatalogTableType.VIRTUAL_VIEW => HiveTableType.VIRTUAL_VIEW
})
- hiveTable.setFields(table.schema.map(toHiveColumn).asJava)
- hiveTable.setPartCols(table.partitionColumns.map(toHiveColumn).asJava)
+ // TODO: pass the comment to Hive somehow
--- End diff --
I see. That's what I suspected
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by xwu0226 <gi...@git.apache.org>.
Github user xwu0226 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59141311
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
@@ -218,14 +219,35 @@ case class CatalogTable(
tableType: CatalogTableType,
storage: CatalogStorageFormat,
schema: Seq[CatalogColumn],
- partitionColumns: Seq[CatalogColumn] = Seq.empty,
- sortColumns: Seq[CatalogColumn] = Seq.empty,
+ partitionColumnNames: Seq[String] = Seq.empty,
+ sortColumnNames: Seq[String] = Seq.empty,
+ bucketColumnNames: Seq[String] = Seq.empty,
+ // e.g. (date, country)
+ skewColumnNames: Seq[String] = Seq.empty,
+ // e.g. ('2008-08-08', 'us), ('2009-09-09', 'uk')
+ skewColumnValues: Seq[Seq[String]] = Seq.empty,
numBuckets: Int = 0,
createTime: Long = System.currentTimeMillis,
lastAccessTime: Long = System.currentTimeMillis,
properties: Map[String, String] = Map.empty,
viewOriginalText: Option[String] = None,
- viewText: Option[String] = None) {
+ viewText: Option[String] = None,
+ comment: Option[String] = None) {
--- End diff --
Does this "comment" mean the comment for the table, as defined in the syntax `create table t1 (c1 int) COMMENT 'abc'`? If it is, shall it be put into `properties`, it seems Hive puts this table comment into TBLPROPERTIES. so it is consistent when creating hive metastore table from CatalogTable object?
---
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-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209153212
**[Test build #55666 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55666/consoleFull)** for PR 12271 at commit [`8dc554a`](https://github.com/apache/spark/commit/8dc554a38c9989fc43b119645bfe5c8ceb7b6cdb).
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-207731891
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: [SPARK-14388][SQL][WIP] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-207730222
**[Test build #55426 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55426/consoleFull)** for PR 12271 at commit [`f7501d9`](https://github.com/apache/spark/commit/f7501d9ebc5c4f08374788a937de6a56689258b8).
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59139248
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
@@ -218,14 +219,35 @@ case class CatalogTable(
tableType: CatalogTableType,
storage: CatalogStorageFormat,
schema: Seq[CatalogColumn],
- partitionColumns: Seq[CatalogColumn] = Seq.empty,
- sortColumns: Seq[CatalogColumn] = Seq.empty,
+ partitionColumnNames: Seq[String] = Seq.empty,
+ sortColumnNames: Seq[String] = Seq.empty,
+ bucketColumnNames: Seq[String] = Seq.empty,
+ // e.g. (date, country)
+ skewColumnNames: Seq[String] = Seq.empty,
+ // e.g. ('2008-08-08', 'us), ('2009-09-09', 'uk')
+ skewColumnValues: Seq[Seq[String]] = Seq.empty,
numBuckets: Int = 0,
createTime: Long = System.currentTimeMillis,
lastAccessTime: Long = System.currentTimeMillis,
properties: Map[String, String] = Map.empty,
viewOriginalText: Option[String] = None,
- viewText: Option[String] = None) {
+ viewText: Option[String] = None,
+ comment: Option[String] = None) {
--- End diff --
Maybe renaming it to `description` (be consistent with `CatalogDatabase`)?
---
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-14388][SQL] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-208665982
I'll have a look at the failing tests in a couple of hours.
---
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209283010
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55693/
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59280363
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
@@ -218,14 +219,35 @@ case class CatalogTable(
tableType: CatalogTableType,
storage: CatalogStorageFormat,
schema: Seq[CatalogColumn],
- partitionColumns: Seq[CatalogColumn] = Seq.empty,
- sortColumns: Seq[CatalogColumn] = Seq.empty,
+ partitionColumnNames: Seq[String] = Seq.empty,
+ sortColumnNames: Seq[String] = Seq.empty,
+ bucketColumnNames: Seq[String] = Seq.empty,
+ // e.g. (date, country)
+ skewColumnNames: Seq[String] = Seq.empty,
+ // e.g. ('2008-08-08', 'us), ('2009-09-09', 'uk')
+ skewColumnValues: Seq[Seq[String]] = Seq.empty,
numBuckets: Int = 0,
createTime: Long = System.currentTimeMillis,
lastAccessTime: Long = System.currentTimeMillis,
properties: Map[String, String] = Map.empty,
viewOriginalText: Option[String] = None,
- viewText: Option[String] = None) {
+ viewText: Option[String] = None,
+ comment: Option[String] = None) {
--- End diff --
I actually prefer to keep it as comment because it's `COMMENT` in the query and `"comment"` in Hive's table properties
---
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-14388][SQL] Implement CREATE TABLE
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59501466
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -488,7 +488,7 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
// just convert the whole type string to lower case, otherwise the struct field names
// will no longer be case sensitive. Instead, we rely on our parser to get the proper
// case before passing it to Hive.
- HiveMetastoreTypes.toDataType(col.dataType.getText).simpleString,
+ CatalystSqlParser.parseDataType(col.dataType.getText).simpleString,
--- End diff --
MINOR/NIT: The DataType parsing is done in the `AstBuilder`, so we really don't need to parse this string again. You could use (magical/evil) `typedVisit[DataType](col.dataType)` 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 pull request: [SPARK-14388][SQL][WIP] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59300596
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -134,82 +136,117 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
}
/**
- * Create a [[CatalogStorageFormat]]. This is part of the [[CreateTableAsSelect]] command.
+ * Create a [[CatalogStorageFormat]] for creating tables.
*/
override def visitCreateFileFormat(
ctx: CreateFileFormatContext): CatalogStorageFormat = withOrigin(ctx) {
- if (ctx.storageHandler == null) {
- typedVisit[CatalogStorageFormat](ctx.fileFormat)
- } else {
- visitStorageHandler(ctx.storageHandler)
+ (ctx.fileFormat, ctx.storageHandler) match {
+ case (fileFormat, null) if fileFormat != null =>
+ fileFormat match {
+ // Expected format: INPUTFORMAT input_format OUTPUTFORMAT output_format
+ case c: TableFileFormatContext => visitTableFileFormat(c)
+ // Expected format: SEQUENCEFILE | TEXTFILE | RCFILE | ORC | PARQUET | AVRO
+ case c: GenericFileFormatContext => visitGenericFileFormat(c)
+ }
+ case (null, storageHandler) if storageHandler != null =>
+ throw new ParseException("Operation not allowed: ... STORED BY storage_handler ...", ctx)
+ case (null, null) =>
+ throw new ParseException("expected one of STORED AS or STORED BY", ctx)
+ case _ =>
+ throw new ParseException("expected either STORED AS or STORED BY, not both", ctx)
}
}
/**
- * Create a [[CreateTableAsSelect]] command.
+ * Create a table. TODO: expand this comment!
+ *
+ * For example:
+ * {{{
+ * CREATE [TEMPORARY] [EXTERNAL] TABLE [IF NOT EXISTS] [db_name.]table_name
+ * [(col1 data_type [COMMENT col_comment], ...)]
+ * [COMMENT table_comment]
+ * [PARTITIONED BY (col3 data_type [COMMENT col_comment], ...)]
+ * [CLUSTERED BY (col1, ...) [SORTED BY (col1 [ASC|DESC], ...)] INTO num_buckets BUCKETS]
+ * [SKEWED BY (col1, col2, ...) ON ((col_value, col_value, ...), ...)
+ * [STORED AS DIRECTORIES]
+ * [ROW FORMAT row_format]
+ * [STORED AS file_format | STORED BY storage_handler_class [WITH SERDEPROPERTIES (...)]]
+ * [LOCATION path]
+ * [TBLPROPERTIES (property_name=property_value, ...)]
+ * [AS select_statement];
+ * }}}
*/
override def visitCreateTable(ctx: CreateTableContext): LogicalPlan = {
--- End diff --
ok
---
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209169942
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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209194478
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55674/
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-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209032675
**[Test build #55628 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55628/consoleFull)** for PR 12271 at commit [`250f402`](https://github.com/apache/spark/commit/250f402372e9826865749f3b81cd96a7cdaff657).
---
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-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209092173
**[Test build #55644 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55644/consoleFull)** for PR 12271 at commit [`250f402`](https://github.com/apache/spark/commit/250f402372e9826865749f3b81cd96a7cdaff657).
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59137836
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -397,11 +429,28 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
}
/**
- * Storage Handlers are currently not supported in the statements we support (CTAS).
+ * Create a [[RowFormat]] used for creating tables.
+ *
+ * Example format:
+ * {{{
+ * SERDE serde_name [WITH SERDEPROPERTIES (k1=v1, k2=v2, ...)]
+ * }}}
+ *
+ * OR
+ *
+ * {{{
+ * DELIMITED [FIELDS TERMINATED BY char [ESCAPED BY char]]
+ * [COLLECTION ITEMS TERMINATED BY char]
+ * [MAP KEYS TERMINATED BY char]
+ * [LINES TERMINATED BY char]
+ * [NULL DEFINED AS char]
+ * }}}
*/
- override def visitStorageHandler(
- ctx: StorageHandlerContext): CatalogStorageFormat = withOrigin(ctx) {
- throw new ParseException("Storage Handlers are currently unsupported.", ctx)
+ private def visitRowFormat(ctx: RowFormatContext): CatalogStorageFormat = withOrigin(ctx) {
+ ctx match {
--- End diff --
yeah, that's the whole reason why I changed it. There's too much magic going on in `typedAccept` or whatever it's called, especially in code that is generated and hidden from the IDE.
---
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-14388][SQL] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59423143
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -121,84 +123,114 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
}
/**
- * Create a [[CatalogStorageFormat]]. This is part of the [[CreateTableAsSelect]] command.
+ * Create a [[CatalogStorageFormat]] for creating tables.
*/
override def visitCreateFileFormat(
ctx: CreateFileFormatContext): CatalogStorageFormat = withOrigin(ctx) {
- if (ctx.storageHandler == null) {
- typedVisit[CatalogStorageFormat](ctx.fileFormat)
- } else {
- visitStorageHandler(ctx.storageHandler)
+ (ctx.fileFormat, ctx.storageHandler) match {
+ // Expected format: INPUTFORMAT input_format OUTPUTFORMAT output_format
+ case (c: TableFileFormatContext, null) =>
+ visitTableFileFormat(c)
+ // Expected format: SEQUENCEFILE | TEXTFILE | RCFILE | ORC | PARQUET | AVRO
+ case (c: GenericFileFormatContext, null) =>
+ visitGenericFileFormat(c)
+ case (null, storageHandler) =>
+ throw new ParseException("Operation not allowed: ... STORED BY storage_handler ...", ctx)
+ case _ =>
+ throw new ParseException("expected either STORED AS or STORED BY, not both", ctx)
}
}
/**
- * Create a [[CreateTableAsSelect]] command.
+ * Create a table, returning either a [[CreateTable]] or a [[CreateTableAsSelect]].
+ *
+ * This is not used to create datasource tables, which is handled through
+ * "CREATE TABLE ... USING ...".
+ *
+ * Note: several features are currently not supported - temporary tables, bucketing,
+ * skewed columns and storage handlers (STORED BY).
+ *
+ * Expected format:
+ * {{{
+ * CREATE [TEMPORARY] [EXTERNAL] TABLE [IF NOT EXISTS] [db_name.]table_name
+ * [(col1 data_type [COMMENT col_comment], ...)]
+ * [COMMENT table_comment]
+ * [PARTITIONED BY (col3 data_type [COMMENT col_comment], ...)]
+ * [CLUSTERED BY (col1, ...) [SORTED BY (col1 [ASC|DESC], ...)] INTO num_buckets BUCKETS]
+ * [SKEWED BY (col1, col2, ...) ON ((col_value, col_value, ...), ...) [STORED AS DIRECTORIES]]
+ * [ROW FORMAT row_format]
+ * [STORED AS file_format | STORED BY storage_handler_class [WITH SERDEPROPERTIES (...)]]
+ * [LOCATION path]
+ * [TBLPROPERTIES (property_name=property_value, ...)]
+ * [AS select_statement];
+ * }}}
*/
- override def visitCreateTable(ctx: CreateTableContext): LogicalPlan = {
- if (ctx.query == null) {
- HiveNativeCommand(command(ctx))
+ override def visitCreateTable(ctx: CreateTableContext): LogicalPlan = withOrigin(ctx) {
+ val (name, temp, ifNotExists, external) = visitCreateTableHeader(ctx.createTableHeader)
+ // TODO: implement temporary tables
+ if (temp) {
+ throw new ParseException(
+ "CREATE TEMPORARY TABLE is not supported yet. " +
+ "Please use registerTempTable as an alternative.", ctx)
+ }
+ if (ctx.skewSpec != null) {
+ throw new ParseException("Operation not allowed: CREATE TABLE ... SKEWED BY ...", ctx)
+ }
+ if (ctx.bucketSpec != null) {
+ throw new ParseException("Operation not allowed: CREATE TABLE ... CLUSTERED BY ...", ctx)
+ }
+ val tableType = if (external) {
+ CatalogTableType.EXTERNAL_TABLE
} else {
- // Get the table header.
- val (table, temp, ifNotExists, external) = visitCreateTableHeader(ctx.createTableHeader)
- val tableType = if (external) {
- CatalogTableType.EXTERNAL_TABLE
- } else {
- CatalogTableType.MANAGED_TABLE
- }
-
- // Unsupported clauses.
- if (temp) {
- throw new ParseException(s"Unsupported operation: TEMPORARY clause.", ctx)
- }
- if (ctx.bucketSpec != null) {
- // TODO add this - we need cluster columns in the CatalogTable for this to work.
- throw new ParseException("Unsupported operation: " +
- "CLUSTERED BY ... [ORDERED BY ...] INTO ... BUCKETS clause.", ctx)
- }
- if (ctx.skewSpec != null) {
- throw new ParseException("Operation not allowed: " +
- "SKEWED BY ... ON ... [STORED AS DIRECTORIES] clause.", ctx)
- }
-
- // Create the schema.
- val schema = Option(ctx.columns).toSeq.flatMap(visitCatalogColumns(_, _.toLowerCase))
-
- // Get the column by which the table is partitioned.
- val partitionCols = Option(ctx.partitionColumns).toSeq.flatMap(visitCatalogColumns(_))
-
- // Create the storage.
- def format(fmt: ParserRuleContext): CatalogStorageFormat = {
- Option(fmt).map(typedVisit[CatalogStorageFormat]).getOrElse(EmptyStorageFormat)
- }
- // Default storage.
+ CatalogTableType.MANAGED_TABLE
+ }
+ val comment = Option(ctx.STRING).map(string)
+ val partitionCols = Option(ctx.partitionColumns).toSeq.flatMap(visitCatalogColumns(_))
+ val cols = Option(ctx.columns).toSeq.flatMap(visitCatalogColumns(_, _.toLowerCase))
+ val properties = Option(ctx.tablePropertyList).map(visitTablePropertyList).getOrElse(Map.empty)
+ val selectQuery = Option(ctx.query).map(plan)
+
+ // Note: Hive requires partition columns to be distinct from the schema, so we need
+ // to include the partition columns here explicitly
+ val schema = cols ++ partitionCols
+
+ // Storage format
+ val defaultStorage: CatalogStorageFormat = {
val defaultStorageType = hiveConf.getVar(HiveConf.ConfVars.HIVEDEFAULTFILEFORMAT)
- val hiveSerDe = HiveSerDe.sourceToSerDe(defaultStorageType, hiveConf).getOrElse {
- HiveSerDe(
- inputFormat = Option("org.apache.hadoop.mapred.TextInputFormat"),
- outputFormat = Option("org.apache.hadoop.hive.ql.io.IgnoreKeyTextOutputFormat"))
- }
- // Defined storage.
- val fileStorage = format(ctx.createFileFormat)
- val rowStorage = format(ctx.rowFormat)
- val storage = CatalogStorageFormat(
- Option(ctx.locationSpec).map(visitLocationSpec),
- fileStorage.inputFormat.orElse(hiveSerDe.inputFormat),
- fileStorage.outputFormat.orElse(hiveSerDe.outputFormat),
- rowStorage.serde.orElse(hiveSerDe.serde).orElse(fileStorage.serde),
- rowStorage.serdeProperties ++ fileStorage.serdeProperties
- )
+ val defaultHiveSerde = HiveSerDe.sourceToSerDe(defaultStorageType, hiveConf)
+ CatalogStorageFormat(
+ locationUri = None,
+ inputFormat = defaultHiveSerde.flatMap(_.inputFormat)
+ .orElse(Option("org.apache.hadoop.mapred.TextInputFormat")),
+ outputFormat = defaultHiveSerde.flatMap(_.outputFormat)
+ .orElse(Option("org.apache.hadoop.hive.ql.io.IgnoreKeyTextOutputFormat")),
+ serde = defaultHiveSerde.flatMap(_.serde),
+ serdeProperties = Map())
+ }
+ val fileStorage = Option(ctx.createFileFormat).map(visitCreateFileFormat)
+ .getOrElse(EmptyStorageFormat)
+ val rowStorage = Option(ctx.rowFormat).map(visitRowFormat).getOrElse(EmptyStorageFormat)
+ val location = Option(ctx.locationSpec).map(visitLocationSpec)
+ val storage = CatalogStorageFormat(
+ locationUri = location,
+ inputFormat = fileStorage.inputFormat.orElse(defaultStorage.inputFormat),
+ outputFormat = fileStorage.outputFormat.orElse(defaultStorage.outputFormat),
+ serde = rowStorage.serde.orElse(fileStorage.serde).orElse(defaultStorage.serde),
+ serdeProperties = rowStorage.serdeProperties ++ fileStorage.serdeProperties)
+
+ // TODO support the sql text - have a proper location for this!
--- End diff --
@hvanhovell maybe explain this? :)
---
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209194477
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: [SPARK-14388][SQL][WIP] Implement CREATE TABLE
Posted by hvanhovell <gi...@git.apache.org>.
Github user hvanhovell commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59113861
--- Diff: sql/catalyst/src/main/antlr4/org/apache/spark/sql/catalyst/parser/SqlBase.g4 ---
@@ -271,8 +271,7 @@ createFileFormat
;
fileFormat
- : INPUTFORMAT inFmt=STRING OUTPUTFORMAT outFmt=STRING (SERDE serdeCls=STRING)?
- (INPUTDRIVER inDriver=STRING OUTPUTDRIVER outDriver=STRING)? #tableFileFormat
--- End diff --
@andrewor14 I wanted to make sure we supported the same grammar as Hive and I used their grammars as a basis. So this is defined in the following two locations:
- https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g#L1335-L1336
- https://github.com/apache/hive/blob/master/ql/src/java/org/apache/hadoop/hive/ql/parse/HiveParser.g#L1335-L1336
The main idea was that I could throw better errors. But if it is not supported by Hive itself then please remove it!
---
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209284615
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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209572170
**[Test build #55727 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55727/consoleFull)** for PR 12271 at commit [`55957bd`](https://github.com/apache/spark/commit/55957bdcd1dea06da750c5cf7cfa587da5250574).
* 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-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59593113
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -675,34 +680,38 @@ private[hive] class HiveClientImpl(
private def toHiveTable(table: CatalogTable): HiveTable = {
val hiveTable = new HiveTable(table.database, table.identifier.table)
- // For EXTERNAL_TABLE/MANAGED_TABLE, we also need to set EXTERNAL field in
- // the table properties accodringly. Otherwise, if EXTERNAL_TABLE is the table type
- // but EXTERNAL field is not set, Hive metastore will change the type to
- // MANAGED_TABLE (see
- // metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L1095-L1105)
+ // For EXTERNAL_TABLE, we also need to set EXTERNAL field in the table properties.
+ // Otherwise, Hive metastore will change the table to a MANAGED_TABLE.
+ // (metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L1095-L1105)
hiveTable.setTableType(table.tableType match {
case CatalogTableType.EXTERNAL_TABLE =>
hiveTable.setProperty("EXTERNAL", "TRUE")
HiveTableType.EXTERNAL_TABLE
case CatalogTableType.MANAGED_TABLE =>
- hiveTable.setProperty("EXTERNAL", "FALSE")
HiveTableType.MANAGED_TABLE
case CatalogTableType.INDEX_TABLE => HiveTableType.INDEX_TABLE
case CatalogTableType.VIRTUAL_VIEW => HiveTableType.VIRTUAL_VIEW
})
- hiveTable.setFields(table.schema.map(toHiveColumn).asJava)
- hiveTable.setPartCols(table.partitionColumns.map(toHiveColumn).asJava)
+ // Note: In Hive the schema and partition columns must be disjoint sets
+ val (partCols, schema) = table.schema.map(toHiveColumn).partition { c =>
+ table.partitionColumnNames.contains(c.getName)
+ }
+ hiveTable.setFields(schema.asJava)
+ hiveTable.setPartCols(partCols.asJava)
// TODO: set sort columns here too
+ hiveTable.setBucketCols(table.bucketColumnNames.asJava)
hiveTable.setOwner(conf.getUser)
hiveTable.setNumBuckets(table.numBuckets)
hiveTable.setCreateTime((table.createTime / 1000).toInt)
hiveTable.setLastAccessTime((table.lastAccessTime / 1000).toInt)
table.storage.locationUri.foreach { loc => shim.setDataLocation(hiveTable, loc) }
table.storage.inputFormat.map(toInputFormat).foreach(hiveTable.setInputFormatClass)
table.storage.outputFormat.map(toOutputFormat).foreach(hiveTable.setOutputFormatClass)
- table.storage.serde.foreach(hiveTable.setSerializationLib)
+ hiveTable.setSerializationLib(
+ table.storage.serde.getOrElse("org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe"))
--- End diff --
I feel it is better to have more checks in SessionCatalog to make sure that a request is valid (it can help your case).
---
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-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-208634574
**[Test build #55552 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55552/consoleFull)** for PR 12271 at commit [`2e95ecf`](https://github.com/apache/spark/commit/2e95ecf790dc5d5b12b6ec72c0bd2b4bca99b17d).
* 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: [SPARK-14388][SQL][WIP] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-207750222
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55426/
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-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59421753
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
@@ -218,14 +219,35 @@ case class CatalogTable(
tableType: CatalogTableType,
storage: CatalogStorageFormat,
schema: Seq[CatalogColumn],
- partitionColumns: Seq[CatalogColumn] = Seq.empty,
- sortColumns: Seq[CatalogColumn] = Seq.empty,
+ partitionColumnNames: Seq[String] = Seq.empty,
+ sortColumnNames: Seq[String] = Seq.empty,
+ bucketColumnNames: Seq[String] = Seq.empty,
+ // e.g. (date, country)
+ skewColumnNames: Seq[String] = Seq.empty,
+ // e.g. ('2008-08-08', 'us), ('2009-09-09', 'uk')
+ skewColumnValues: Seq[Seq[String]] = Seq.empty,
numBuckets: Int = 0,
createTime: Long = System.currentTimeMillis,
lastAccessTime: Long = System.currentTimeMillis,
properties: Map[String, String] = Map.empty,
viewOriginalText: Option[String] = None,
- viewText: Option[String] = None) {
+ viewText: Option[String] = None,
+ comment: Option[String] = None) {
+
+ // Verify that the provided columns are part of the schema
+ private val colNames = schema.map(_.name).toSet
+ private def requireSubsetOfSchema(cols: Seq[String], colType: String): Unit = {
+ require(cols.toSet.subsetOf(colNames), s"$colType columns (${cols.mkString(", ")}) " +
+ s"must be a subset of schema (${colNames.mkString(", ")}) in table '$identifier'")
+ }
+ requireSubsetOfSchema(partitionColumnNames, "partition")
--- End diff --
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 pull request: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59497215
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -435,13 +480,15 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
/**
* Create a sequence of [[CatalogColumn]]s from a column list
*/
- private def visitCatalogColumns(
- ctx: ColTypeListContext,
- formatter: String => String = identity): Seq[CatalogColumn] = withOrigin(ctx) {
+ private def visitCatalogColumns(ctx: ColTypeListContext): Seq[CatalogColumn] = withOrigin(ctx) {
ctx.colType.asScala.map { col =>
CatalogColumn(
- formatter(col.identifier.getText),
- col.dataType.getText.toLowerCase, // TODO validate this?
+ col.identifier.getText.toLowerCase,
+ // Note: for types like "STRUCT<myFirstName: STRING, myLastName: STRING>" we can't
+ // just convert the whole type string to lower case, otherwise the struct field names
+ // will no longer be case sensitive. Instead, we rely on our parser to get the proper
+ // case before passing it to Hive.
+ HiveMetastoreTypes.toDataType(col.dataType.getText).simpleString,
--- End diff --
I think we need to use `parseDataType`.
---
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209572492
Test PASSed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55727/
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-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59421840
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -121,84 +123,114 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
}
/**
- * Create a [[CatalogStorageFormat]]. This is part of the [[CreateTableAsSelect]] command.
+ * Create a [[CatalogStorageFormat]] for creating tables.
*/
override def visitCreateFileFormat(
ctx: CreateFileFormatContext): CatalogStorageFormat = withOrigin(ctx) {
- if (ctx.storageHandler == null) {
- typedVisit[CatalogStorageFormat](ctx.fileFormat)
- } else {
- visitStorageHandler(ctx.storageHandler)
+ (ctx.fileFormat, ctx.storageHandler) match {
+ // Expected format: INPUTFORMAT input_format OUTPUTFORMAT output_format
+ case (c: TableFileFormatContext, null) =>
+ visitTableFileFormat(c)
+ // Expected format: SEQUENCEFILE | TEXTFILE | RCFILE | ORC | PARQUET | AVRO
+ case (c: GenericFileFormatContext, null) =>
+ visitGenericFileFormat(c)
+ case (null, storageHandler) =>
+ throw new ParseException("Operation not allowed: ... STORED BY storage_handler ...", ctx)
+ case _ =>
+ throw new ParseException("expected either STORED AS or STORED BY, not both", ctx)
}
}
/**
- * Create a [[CreateTableAsSelect]] command.
+ * Create a table, returning either a [[CreateTable]] or a [[CreateTableAsSelect]].
+ *
+ * This is not used to create datasource tables, which is handled through
+ * "CREATE TABLE ... USING ...".
+ *
+ * Note: several features are currently not supported - temporary tables, bucketing,
+ * skewed columns and storage handlers (STORED BY).
+ *
+ * Expected format:
+ * {{{
+ * CREATE [TEMPORARY] [EXTERNAL] TABLE [IF NOT EXISTS] [db_name.]table_name
+ * [(col1 data_type [COMMENT col_comment], ...)]
+ * [COMMENT table_comment]
+ * [PARTITIONED BY (col3 data_type [COMMENT col_comment], ...)]
+ * [CLUSTERED BY (col1, ...) [SORTED BY (col1 [ASC|DESC], ...)] INTO num_buckets BUCKETS]
+ * [SKEWED BY (col1, col2, ...) ON ((col_value, col_value, ...), ...) [STORED AS DIRECTORIES]]
+ * [ROW FORMAT row_format]
+ * [STORED AS file_format | STORED BY storage_handler_class [WITH SERDEPROPERTIES (...)]]
+ * [LOCATION path]
+ * [TBLPROPERTIES (property_name=property_value, ...)]
+ * [AS select_statement];
+ * }}}
*/
- override def visitCreateTable(ctx: CreateTableContext): LogicalPlan = {
- if (ctx.query == null) {
- HiveNativeCommand(command(ctx))
+ override def visitCreateTable(ctx: CreateTableContext): LogicalPlan = withOrigin(ctx) {
+ val (name, temp, ifNotExists, external) = visitCreateTableHeader(ctx.createTableHeader)
+ // TODO: implement temporary tables
--- End diff --
+1
---
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-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59573943
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -675,34 +680,38 @@ private[hive] class HiveClientImpl(
private def toHiveTable(table: CatalogTable): HiveTable = {
val hiveTable = new HiveTable(table.database, table.identifier.table)
- // For EXTERNAL_TABLE/MANAGED_TABLE, we also need to set EXTERNAL field in
- // the table properties accodringly. Otherwise, if EXTERNAL_TABLE is the table type
- // but EXTERNAL field is not set, Hive metastore will change the type to
- // MANAGED_TABLE (see
- // metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L1095-L1105)
+ // For EXTERNAL_TABLE, we also need to set EXTERNAL field in the table properties.
+ // Otherwise, Hive metastore will change the table to a MANAGED_TABLE.
+ // (metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L1095-L1105)
hiveTable.setTableType(table.tableType match {
case CatalogTableType.EXTERNAL_TABLE =>
hiveTable.setProperty("EXTERNAL", "TRUE")
HiveTableType.EXTERNAL_TABLE
case CatalogTableType.MANAGED_TABLE =>
- hiveTable.setProperty("EXTERNAL", "FALSE")
HiveTableType.MANAGED_TABLE
case CatalogTableType.INDEX_TABLE => HiveTableType.INDEX_TABLE
case CatalogTableType.VIRTUAL_VIEW => HiveTableType.VIRTUAL_VIEW
})
- hiveTable.setFields(table.schema.map(toHiveColumn).asJava)
- hiveTable.setPartCols(table.partitionColumns.map(toHiveColumn).asJava)
+ // Note: In Hive the schema and partition columns must be disjoint sets
+ val (partCols, schema) = table.schema.map(toHiveColumn).partition { c =>
+ table.partitionColumnNames.contains(c.getName)
+ }
+ hiveTable.setFields(schema.asJava)
+ hiveTable.setPartCols(partCols.asJava)
// TODO: set sort columns here too
+ hiveTable.setBucketCols(table.bucketColumnNames.asJava)
hiveTable.setOwner(conf.getUser)
hiveTable.setNumBuckets(table.numBuckets)
hiveTable.setCreateTime((table.createTime / 1000).toInt)
hiveTable.setLastAccessTime((table.lastAccessTime / 1000).toInt)
table.storage.locationUri.foreach { loc => shim.setDataLocation(hiveTable, loc) }
table.storage.inputFormat.map(toInputFormat).foreach(hiveTable.setInputFormatClass)
table.storage.outputFormat.map(toOutputFormat).foreach(hiveTable.setOutputFormatClass)
- table.storage.serde.foreach(hiveTable.setSerializationLib)
+ hiveTable.setSerializationLib(
+ table.storage.serde.getOrElse("org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe"))
--- End diff --
hmm... Without this PR, the storage related fields of the table defined in `org.apache.spark.sql.hive.MetastoreDataSourcesSuite.persistent JSON table` are:
```
[SerDe Library: org.apache.hadoop.hive.serde2.MetadataTypedColumnsetSerDe ]
[InputFormat: org.apache.hadoop.mapred.SequenceFileInputFormat ]
[OutputFormat: org.apache.hadoop.hive.ql.io.HiveSequenceFileOutputFormat ]
```
Looks like the change at here somehow breaks the test (without the change, we can somehow trigger a weird code path in Hive and get at least have one column called `col`).
---
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209272026
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55692/
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-207816655
@andrewor14 Just want to let you know, @xwu0226 is doing the command `SHOW CREATE TABLE`. He is writing many test cases in the PR: https://github.com/apache/spark/pull/12132/files. It might help you in this PR. 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 pull request: [SPARK-14388][SQL][WIP] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-207731862
**[Test build #55425 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55425/consoleFull)** for PR 12271 at commit [`5e0fe03`](https://github.com/apache/spark/commit/5e0fe03bfa655c6de854bc8adaa73186a17a0b0c).
* 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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209158266
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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209087417
**[Test build #55634 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55634/consoleFull)** for PR 12271 at commit [`250f402`](https://github.com/apache/spark/commit/250f402372e9826865749f3b81cd96a7cdaff657).
* 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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209252514
**[Test build #55693 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55693/consoleFull)** for PR 12271 at commit [`a60e66a`](https://github.com/apache/spark/commit/a60e66a71dec96973043ec62e2b6d4213c5add2c).
---
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-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209135892
**[Test build #55658 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55658/consoleFull)** for PR 12271 at commit [`50a2054`](https://github.com/apache/spark/commit/50a2054ec7a7276d45c1ab5adabd4550e00c7811).
---
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209058443
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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209091270
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 pull request: [SPARK-14388][SQL][WIP] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59121511
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -665,9 +672,17 @@ private[hive] class HiveClientImpl(
case CatalogTableType.INDEX_TABLE => HiveTableType.INDEX_TABLE
case CatalogTableType.VIRTUAL_VIEW => HiveTableType.VIRTUAL_VIEW
})
- hiveTable.setFields(table.schema.map(toHiveColumn).asJava)
- hiveTable.setPartCols(table.partitionColumns.map(toHiveColumn).asJava)
+ // TODO: pass the comment to Hive somehow
--- End diff --
comment is stored in the table properties with a field called `comment`.
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59111479
--- Diff: sql/core/src/test/scala/org/apache/spark/sql/execution/command/DDLSuite.scala ---
@@ -320,8 +320,6 @@ class DDLSuite extends QueryTest with SharedSQLContext with BeforeAndAfterEach {
assertUnsupported("ALTER TABLE dbx.tab1 NOT STORED AS DIRECTORIES")
}
- // TODO: ADD a testcase for Drop Database in Restric when we can create tables in SQLContext
--- End diff --
this TODO comment is duplicated in line 197
---
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-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209061013
**[Test build #55634 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55634/consoleFull)** for PR 12271 at commit [`250f402`](https://github.com/apache/spark/commit/250f402372e9826865749f3b81cd96a7cdaff657).
---
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-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209536900
**[Test build #55727 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55727/consoleFull)** for PR 12271 at commit [`55957bd`](https://github.com/apache/spark/commit/55957bdcd1dea06da750c5cf7cfa587da5250574).
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59118235
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
@@ -218,14 +219,35 @@ case class CatalogTable(
tableType: CatalogTableType,
storage: CatalogStorageFormat,
schema: Seq[CatalogColumn],
- partitionColumns: Seq[CatalogColumn] = Seq.empty,
- sortColumns: Seq[CatalogColumn] = Seq.empty,
+ partitionColumnNames: Seq[String] = Seq.empty,
+ sortColumnNames: Seq[String] = Seq.empty,
+ bucketColumnNames: Seq[String] = Seq.empty,
+ // e.g. (date, country)
+ skewColumnNames: Seq[String] = Seq.empty,
+ // e.g. ('2008-08-08', 'us), ('2009-09-09', 'uk')
+ skewColumnValues: Seq[Seq[String]] = Seq.empty,
numBuckets: Int = 0,
createTime: Long = System.currentTimeMillis,
lastAccessTime: Long = System.currentTimeMillis,
properties: Map[String, String] = Map.empty,
viewOriginalText: Option[String] = None,
- viewText: Option[String] = None) {
+ viewText: Option[String] = None,
+ comment: Option[String] = None) {
+
+ // Verify that the provided columns are part of the schema
+ private val colNames = schema.map(_.name).toSet
+ private def requireSubsetOfSchema(cols: Seq[String], colType: String): Unit = {
+ require(cols.toSet.subsetOf(colNames), s"$colType columns (${cols.mkString(", ")}) " +
+ s"must be a subset of schema (${colNames.mkString(", ")}) in table '$identifier'")
+ }
+ requireSubsetOfSchema(partitionColumnNames, "partition")
--- End diff --
I find the following example from Hive's doc
```
CREATE TABLE page_view(viewTime INT, userid BIGINT,
page_url STRING, referrer_url STRING,
ip STRING COMMENT 'IP Address of the User')
COMMENT 'This is the page view table'
PARTITIONED BY(dt STRING, country STRING)
ROW FORMAT DELIMITED
FIELDS TERMINATED BY '\001'
STORED AS SEQUENCEFILE;
```
Will this CREATE TABLE statement trigger an error 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
[GitHub] spark pull request: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59491266
--- Diff: sql/hive/compatibility/src/test/scala/org/apache/spark/sql/hive/execution/HiveCompatibilitySuite.scala ---
@@ -509,7 +555,7 @@ class HiveCompatibilitySuite extends HiveQueryFileTest with BeforeAndAfter {
"date_comparison",
"date_join1",
"date_serde",
- "decimal_1",
+ //"decimal_1", // TODO: cannot parse column decimal(5)
--- End diff --
https://issues.apache.org/jira/browse/SPARK-14591
---
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-208634671
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55552/
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-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59492242
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -121,84 +123,115 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
}
/**
- * Create a [[CatalogStorageFormat]]. This is part of the [[CreateTableAsSelect]] command.
+ * Create a [[CatalogStorageFormat]] for creating tables.
*/
override def visitCreateFileFormat(
ctx: CreateFileFormatContext): CatalogStorageFormat = withOrigin(ctx) {
- if (ctx.storageHandler == null) {
- typedVisit[CatalogStorageFormat](ctx.fileFormat)
- } else {
- visitStorageHandler(ctx.storageHandler)
+ (ctx.fileFormat, ctx.storageHandler) match {
+ // Expected format: INPUTFORMAT input_format OUTPUTFORMAT output_format
+ case (c: TableFileFormatContext, null) =>
+ visitTableFileFormat(c)
+ // Expected format: SEQUENCEFILE | TEXTFILE | RCFILE | ORC | PARQUET | AVRO
+ case (c: GenericFileFormatContext, null) =>
+ visitGenericFileFormat(c)
+ case (null, storageHandler) =>
+ throw new ParseException("Operation not allowed: ... STORED BY storage_handler ...", ctx)
+ case _ =>
+ throw new ParseException("expected either STORED AS or STORED BY, not both", ctx)
}
}
/**
- * Create a [[CreateTableAsSelect]] command.
+ * Create a table, returning either a [[CreateTable]] or a [[CreateTableAsSelect]].
+ *
+ * This is not used to create datasource tables, which is handled through
+ * "CREATE TABLE ... USING ...".
+ *
+ * Note: several features are currently not supported - temporary tables, bucketing,
+ * skewed columns and storage handlers (STORED BY).
+ *
+ * Expected format:
+ * {{{
+ * CREATE [TEMPORARY] [EXTERNAL] TABLE [IF NOT EXISTS] [db_name.]table_name
+ * [(col1 data_type [COMMENT col_comment], ...)]
+ * [COMMENT table_comment]
+ * [PARTITIONED BY (col3 data_type [COMMENT col_comment], ...)]
+ * [CLUSTERED BY (col1, ...) [SORTED BY (col1 [ASC|DESC], ...)] INTO num_buckets BUCKETS]
+ * [SKEWED BY (col1, col2, ...) ON ((col_value, col_value, ...), ...) [STORED AS DIRECTORIES]]
+ * [ROW FORMAT row_format]
+ * [STORED AS file_format | STORED BY storage_handler_class [WITH SERDEPROPERTIES (...)]]
+ * [LOCATION path]
+ * [TBLPROPERTIES (property_name=property_value, ...)]
+ * [AS select_statement];
+ * }}}
*/
- override def visitCreateTable(ctx: CreateTableContext): LogicalPlan = {
- if (ctx.query == null) {
- HiveNativeCommand(command(ctx))
+ override def visitCreateTable(ctx: CreateTableContext): LogicalPlan = withOrigin(ctx) {
+ val (name, temp, ifNotExists, external) = visitCreateTableHeader(ctx.createTableHeader)
+ // TODO: implement temporary tables
+ if (temp) {
+ throw new ParseException(
+ "CREATE TEMPORARY TABLE is not supported yet. " +
+ "Please use registerTempTable as an alternative.", ctx)
+ }
+ if (ctx.skewSpec != null) {
+ throw new ParseException("Operation not allowed: CREATE TABLE ... SKEWED BY ...", ctx)
+ }
+ if (ctx.bucketSpec != null) {
+ throw new ParseException("Operation not allowed: CREATE TABLE ... CLUSTERED BY ...", ctx)
+ }
+ val tableType = if (external) {
+ CatalogTableType.EXTERNAL_TABLE
} else {
- // Get the table header.
- val (table, temp, ifNotExists, external) = visitCreateTableHeader(ctx.createTableHeader)
- val tableType = if (external) {
- CatalogTableType.EXTERNAL_TABLE
- } else {
- CatalogTableType.MANAGED_TABLE
- }
-
- // Unsupported clauses.
- if (temp) {
- throw new ParseException(s"Unsupported operation: TEMPORARY clause.", ctx)
- }
- if (ctx.bucketSpec != null) {
- // TODO add this - we need cluster columns in the CatalogTable for this to work.
- throw new ParseException("Unsupported operation: " +
- "CLUSTERED BY ... [ORDERED BY ...] INTO ... BUCKETS clause.", ctx)
- }
- if (ctx.skewSpec != null) {
- throw new ParseException("Operation not allowed: " +
- "SKEWED BY ... ON ... [STORED AS DIRECTORIES] clause.", ctx)
- }
-
- // Create the schema.
- val schema = Option(ctx.columns).toSeq.flatMap(visitCatalogColumns(_, _.toLowerCase))
-
- // Get the column by which the table is partitioned.
- val partitionCols = Option(ctx.partitionColumns).toSeq.flatMap(visitCatalogColumns(_))
-
- // Create the storage.
- def format(fmt: ParserRuleContext): CatalogStorageFormat = {
- Option(fmt).map(typedVisit[CatalogStorageFormat]).getOrElse(EmptyStorageFormat)
- }
- // Default storage.
+ CatalogTableType.MANAGED_TABLE
+ }
+ val comment = Option(ctx.STRING).map(string)
+ val partitionCols = Option(ctx.partitionColumns).toSeq.flatMap(visitCatalogColumns)
+ val cols = Option(ctx.columns).toSeq.flatMap(visitCatalogColumns)
+ val properties = Option(ctx.tablePropertyList).map(visitTablePropertyList).getOrElse(Map.empty)
+ val selectQuery = Option(ctx.query).map(plan)
+
+ // Note: Hive requires partition columns to be distinct from the schema, so we need
+ // to include the partition columns here explicitly
+ val schema = cols ++ partitionCols
+
+ // Storage format
+ val defaultStorage: CatalogStorageFormat = {
val defaultStorageType = hiveConf.getVar(HiveConf.ConfVars.HIVEDEFAULTFILEFORMAT)
- val hiveSerDe = HiveSerDe.sourceToSerDe(defaultStorageType, hiveConf).getOrElse {
- HiveSerDe(
- inputFormat = Option("org.apache.hadoop.mapred.TextInputFormat"),
- outputFormat = Option("org.apache.hadoop.hive.ql.io.IgnoreKeyTextOutputFormat"))
- }
- // Defined storage.
- val fileStorage = format(ctx.createFileFormat)
- val rowStorage = format(ctx.rowFormat)
- val storage = CatalogStorageFormat(
- Option(ctx.locationSpec).map(visitLocationSpec),
- fileStorage.inputFormat.orElse(hiveSerDe.inputFormat),
- fileStorage.outputFormat.orElse(hiveSerDe.outputFormat),
- rowStorage.serde.orElse(hiveSerDe.serde).orElse(fileStorage.serde),
- rowStorage.serdeProperties ++ fileStorage.serdeProperties
- )
+ val defaultHiveSerde = HiveSerDe.sourceToSerDe(defaultStorageType, hiveConf)
+ CatalogStorageFormat(
+ locationUri = None,
+ inputFormat = defaultHiveSerde.flatMap(_.inputFormat)
+ .orElse(Some("org.apache.hadoop.mapred.TextInputFormat")),
+ outputFormat = defaultHiveSerde.flatMap(_.outputFormat)
+ .orElse(Some("org.apache.hadoop.hive.ql.io.IgnoreKeyTextOutputFormat")),
+ serde = defaultHiveSerde.flatMap(_.serde)
+ .orElse(Some("org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe")),
--- End diff --
This one breaks the CTAS 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 pull request: [SPARK-14388][SQL][WIP] Implement CREATE TABLE
Posted by xwu0226 <gi...@git.apache.org>.
Github user xwu0226 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59117509
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
@@ -218,14 +219,35 @@ case class CatalogTable(
tableType: CatalogTableType,
storage: CatalogStorageFormat,
schema: Seq[CatalogColumn],
- partitionColumns: Seq[CatalogColumn] = Seq.empty,
- sortColumns: Seq[CatalogColumn] = Seq.empty,
+ partitionColumnNames: Seq[String] = Seq.empty,
--- End diff --
The hive syntax has PARTITIONED BY (col_name data_type [COMMENT col_comment], ...). The column definition here is the same as the ones in schema. CatalogColumn can represent these 3 aspects of each column.. why we need to change to use String 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 pull request: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59421983
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -121,84 +123,114 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
}
/**
- * Create a [[CatalogStorageFormat]]. This is part of the [[CreateTableAsSelect]] command.
+ * Create a [[CatalogStorageFormat]] for creating tables.
*/
override def visitCreateFileFormat(
ctx: CreateFileFormatContext): CatalogStorageFormat = withOrigin(ctx) {
- if (ctx.storageHandler == null) {
- typedVisit[CatalogStorageFormat](ctx.fileFormat)
- } else {
- visitStorageHandler(ctx.storageHandler)
+ (ctx.fileFormat, ctx.storageHandler) match {
+ // Expected format: INPUTFORMAT input_format OUTPUTFORMAT output_format
+ case (c: TableFileFormatContext, null) =>
+ visitTableFileFormat(c)
+ // Expected format: SEQUENCEFILE | TEXTFILE | RCFILE | ORC | PARQUET | AVRO
+ case (c: GenericFileFormatContext, null) =>
+ visitGenericFileFormat(c)
+ case (null, storageHandler) =>
+ throw new ParseException("Operation not allowed: ... STORED BY storage_handler ...", ctx)
+ case _ =>
+ throw new ParseException("expected either STORED AS or STORED BY, not both", ctx)
}
}
/**
- * Create a [[CreateTableAsSelect]] command.
+ * Create a table, returning either a [[CreateTable]] or a [[CreateTableAsSelect]].
+ *
+ * This is not used to create datasource tables, which is handled through
+ * "CREATE TABLE ... USING ...".
+ *
+ * Note: several features are currently not supported - temporary tables, bucketing,
+ * skewed columns and storage handlers (STORED BY).
+ *
+ * Expected format:
+ * {{{
+ * CREATE [TEMPORARY] [EXTERNAL] TABLE [IF NOT EXISTS] [db_name.]table_name
+ * [(col1 data_type [COMMENT col_comment], ...)]
+ * [COMMENT table_comment]
+ * [PARTITIONED BY (col3 data_type [COMMENT col_comment], ...)]
+ * [CLUSTERED BY (col1, ...) [SORTED BY (col1 [ASC|DESC], ...)] INTO num_buckets BUCKETS]
+ * [SKEWED BY (col1, col2, ...) ON ((col_value, col_value, ...), ...) [STORED AS DIRECTORIES]]
+ * [ROW FORMAT row_format]
+ * [STORED AS file_format | STORED BY storage_handler_class [WITH SERDEPROPERTIES (...)]]
+ * [LOCATION path]
+ * [TBLPROPERTIES (property_name=property_value, ...)]
+ * [AS select_statement];
+ * }}}
*/
- override def visitCreateTable(ctx: CreateTableContext): LogicalPlan = {
- if (ctx.query == null) {
- HiveNativeCommand(command(ctx))
+ override def visitCreateTable(ctx: CreateTableContext): LogicalPlan = withOrigin(ctx) {
+ val (name, temp, ifNotExists, external) = visitCreateTableHeader(ctx.createTableHeader)
+ // TODO: implement temporary tables
+ if (temp) {
+ throw new ParseException(
+ "CREATE TEMPORARY TABLE is not supported yet. " +
+ "Please use registerTempTable as an alternative.", ctx)
+ }
+ if (ctx.skewSpec != null) {
+ throw new ParseException("Operation not allowed: CREATE TABLE ... SKEWED BY ...", ctx)
+ }
+ if (ctx.bucketSpec != null) {
+ throw new ParseException("Operation not allowed: CREATE TABLE ... CLUSTERED BY ...", ctx)
+ }
+ val tableType = if (external) {
+ CatalogTableType.EXTERNAL_TABLE
} else {
- // Get the table header.
- val (table, temp, ifNotExists, external) = visitCreateTableHeader(ctx.createTableHeader)
- val tableType = if (external) {
- CatalogTableType.EXTERNAL_TABLE
- } else {
- CatalogTableType.MANAGED_TABLE
- }
-
- // Unsupported clauses.
- if (temp) {
- throw new ParseException(s"Unsupported operation: TEMPORARY clause.", ctx)
- }
- if (ctx.bucketSpec != null) {
- // TODO add this - we need cluster columns in the CatalogTable for this to work.
- throw new ParseException("Unsupported operation: " +
- "CLUSTERED BY ... [ORDERED BY ...] INTO ... BUCKETS clause.", ctx)
- }
- if (ctx.skewSpec != null) {
- throw new ParseException("Operation not allowed: " +
- "SKEWED BY ... ON ... [STORED AS DIRECTORIES] clause.", ctx)
- }
-
- // Create the schema.
- val schema = Option(ctx.columns).toSeq.flatMap(visitCatalogColumns(_, _.toLowerCase))
-
- // Get the column by which the table is partitioned.
- val partitionCols = Option(ctx.partitionColumns).toSeq.flatMap(visitCatalogColumns(_))
-
- // Create the storage.
- def format(fmt: ParserRuleContext): CatalogStorageFormat = {
- Option(fmt).map(typedVisit[CatalogStorageFormat]).getOrElse(EmptyStorageFormat)
- }
- // Default storage.
+ CatalogTableType.MANAGED_TABLE
+ }
+ val comment = Option(ctx.STRING).map(string)
+ val partitionCols = Option(ctx.partitionColumns).toSeq.flatMap(visitCatalogColumns(_))
+ val cols = Option(ctx.columns).toSeq.flatMap(visitCatalogColumns(_, _.toLowerCase))
+ val properties = Option(ctx.tablePropertyList).map(visitTablePropertyList).getOrElse(Map.empty)
+ val selectQuery = Option(ctx.query).map(plan)
+
+ // Note: Hive requires partition columns to be distinct from the schema, so we need
+ // to include the partition columns here explicitly
+ val schema = cols ++ partitionCols
+
+ // Storage format
+ val defaultStorage: CatalogStorageFormat = {
val defaultStorageType = hiveConf.getVar(HiveConf.ConfVars.HIVEDEFAULTFILEFORMAT)
- val hiveSerDe = HiveSerDe.sourceToSerDe(defaultStorageType, hiveConf).getOrElse {
- HiveSerDe(
- inputFormat = Option("org.apache.hadoop.mapred.TextInputFormat"),
- outputFormat = Option("org.apache.hadoop.hive.ql.io.IgnoreKeyTextOutputFormat"))
- }
- // Defined storage.
- val fileStorage = format(ctx.createFileFormat)
- val rowStorage = format(ctx.rowFormat)
- val storage = CatalogStorageFormat(
- Option(ctx.locationSpec).map(visitLocationSpec),
- fileStorage.inputFormat.orElse(hiveSerDe.inputFormat),
- fileStorage.outputFormat.orElse(hiveSerDe.outputFormat),
- rowStorage.serde.orElse(hiveSerDe.serde).orElse(fileStorage.serde),
- rowStorage.serdeProperties ++ fileStorage.serdeProperties
- )
+ val defaultHiveSerde = HiveSerDe.sourceToSerDe(defaultStorageType, hiveConf)
+ CatalogStorageFormat(
+ locationUri = None,
+ inputFormat = defaultHiveSerde.flatMap(_.inputFormat)
+ .orElse(Option("org.apache.hadoop.mapred.TextInputFormat")),
+ outputFormat = defaultHiveSerde.flatMap(_.outputFormat)
+ .orElse(Option("org.apache.hadoop.hive.ql.io.IgnoreKeyTextOutputFormat")),
+ serde = defaultHiveSerde.flatMap(_.serde),
+ serdeProperties = Map())
+ }
+ val fileStorage = Option(ctx.createFileFormat).map(visitCreateFileFormat)
+ .getOrElse(EmptyStorageFormat)
+ val rowStorage = Option(ctx.rowFormat).map(visitRowFormat).getOrElse(EmptyStorageFormat)
+ val location = Option(ctx.locationSpec).map(visitLocationSpec)
+ val storage = CatalogStorageFormat(
+ locationUri = location,
+ inputFormat = fileStorage.inputFormat.orElse(defaultStorage.inputFormat),
+ outputFormat = fileStorage.outputFormat.orElse(defaultStorage.outputFormat),
+ serde = rowStorage.serde.orElse(fileStorage.serde).orElse(defaultStorage.serde),
+ serdeProperties = rowStorage.serdeProperties ++ fileStorage.serdeProperties)
+
+ // TODO support the sql text - have a proper location for this!
--- End diff --
Maybe explain this?
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59300573
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command
+
+import org.apache.spark.sql.{Row, SQLContext}
+import org.apache.spark.sql.catalyst.TableIdentifier
+import org.apache.spark.sql.catalyst.catalog.CatalogTable
+
+
+// TODO: move the rest of the table commands from ddl.scala to this file
+
+/**
+ * A command to create a table.
+ *
+ * Note: This is currently used only for creating Hive tables.
+ * This is not intended for temporary tables.
+ *
+ * The syntax of using this command in SQL is:
+ * {{{
+ * CREATE [EXTERNAL] TABLE [IF NOT EXISTS] [db_name.]table_name
+ * [(col1 data_type [COMMENT col_comment], ...)]
+ * [COMMENT table_comment]
+ * [PARTITIONED BY (col3 data_type [COMMENT col_comment], ...)]
+ * [CLUSTERED BY (col1, ...) [SORTED BY (col1 [ASC|DESC], ...)] INTO num_buckets BUCKETS]
+ * [SKEWED BY (col1, col2, ...) ON ((col_value, col_value, ...), ...)
--- End diff --
ok
---
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-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59491359
--- Diff: sql/hive/compatibility/src/test/scala/org/apache/spark/sql/hive/execution/HiveCompatibilitySuite.scala ---
@@ -490,15 +538,13 @@ class HiveCompatibilitySuite extends HiveQueryFileTest with BeforeAndAfter {
"count",
"cp_mj_rc",
"create_insert_outputformat",
- "create_like_tbl_props",
+ //"create_like_tbl_props", // TODO: results don't match
--- End diff --
https://issues.apache.org/jira/browse/SPARK-14592
---
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59118131
--- Diff: sql/core/src/main/scala/org/apache/spark/sql/execution/command/tables.scala ---
@@ -0,0 +1,80 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.spark.sql.execution.command
+
+import org.apache.spark.sql.{Row, SQLContext}
+import org.apache.spark.sql.catalyst.TableIdentifier
+import org.apache.spark.sql.catalyst.catalog.CatalogTable
+
+
+// TODO: move the rest of the table commands from ddl.scala to this file
+
+/**
+ * A command to create a table.
+ *
+ * Note: This is currently used only for creating Hive tables.
+ * This is not intended for temporary tables.
+ *
+ * The syntax of using this command in SQL is:
+ * {{{
+ * CREATE [EXTERNAL] TABLE [IF NOT EXISTS] [db_name.]table_name
+ * [(col1 data_type [COMMENT col_comment], ...)]
+ * [COMMENT table_comment]
+ * [PARTITIONED BY (col3 data_type [COMMENT col_comment], ...)]
+ * [CLUSTERED BY (col1, ...) [SORTED BY (col1 [ASC|DESC], ...)] INTO num_buckets BUCKETS]
+ * [SKEWED BY (col1, col2, ...) ON ((col_value, col_value, ...), ...)
--- End diff --
@andrewor14 I just did a quick check with our `InsertIntoHiveTable` command. Seems this command does not really understand how to handle a table having specifications on `CLUSTERED BY`, `SORTED BY` or `SKEWED BY`. How about we just throw exceptions when a define provide these specs?
---
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-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209158146
**[Test build #55658 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55658/consoleFull)** for PR 12271 at commit [`50a2054`](https://github.com/apache/spark/commit/50a2054ec7a7276d45c1ab5adabd4550e00c7811).
* 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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209572290
OK. 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 pull request: [SPARK-14388][SQL][WIP] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59137856
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -134,82 +136,117 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
}
/**
- * Create a [[CatalogStorageFormat]]. This is part of the [[CreateTableAsSelect]] command.
+ * Create a [[CatalogStorageFormat]] for creating tables.
*/
override def visitCreateFileFormat(
ctx: CreateFileFormatContext): CatalogStorageFormat = withOrigin(ctx) {
- if (ctx.storageHandler == null) {
- typedVisit[CatalogStorageFormat](ctx.fileFormat)
- } else {
- visitStorageHandler(ctx.storageHandler)
+ (ctx.fileFormat, ctx.storageHandler) match {
+ case (fileFormat, null) if fileFormat != null =>
+ fileFormat match {
+ // Expected format: INPUTFORMAT input_format OUTPUTFORMAT output_format
+ case c: TableFileFormatContext => visitTableFileFormat(c)
+ // Expected format: SEQUENCEFILE | TEXTFILE | RCFILE | ORC | PARQUET | AVRO
+ case c: GenericFileFormatContext => visitGenericFileFormat(c)
+ }
+ case (null, storageHandler) if storageHandler != null =>
+ throw new ParseException("Operation not allowed: ... STORED BY storage_handler ...", ctx)
+ case (null, null) =>
+ throw new ParseException("expected one of STORED AS or STORED BY", ctx)
+ case _ =>
+ throw new ParseException("expected either STORED AS or STORED BY, not both", ctx)
}
}
/**
- * Create a [[CreateTableAsSelect]] command.
+ * Create a table. TODO: expand this comment!
+ *
+ * For example:
+ * {{{
+ * CREATE [TEMPORARY] [EXTERNAL] TABLE [IF NOT EXISTS] [db_name.]table_name
+ * [(col1 data_type [COMMENT col_comment], ...)]
+ * [COMMENT table_comment]
+ * [PARTITIONED BY (col3 data_type [COMMENT col_comment], ...)]
+ * [CLUSTERED BY (col1, ...) [SORTED BY (col1 [ASC|DESC], ...)] INTO num_buckets BUCKETS]
+ * [SKEWED BY (col1, col2, ...) ON ((col_value, col_value, ...), ...)
+ * [STORED AS DIRECTORIES]
+ * [ROW FORMAT row_format]
+ * [STORED AS file_format | STORED BY storage_handler_class [WITH SERDEPROPERTIES (...)]]
+ * [LOCATION path]
+ * [TBLPROPERTIES (property_name=property_value, ...)]
+ * [AS select_statement];
+ * }}}
*/
override def visitCreateTable(ctx: CreateTableContext): LogicalPlan = {
- if (ctx.query == null) {
- HiveNativeCommand(command(ctx))
+ val (name, temp, ifNotExists, external) = visitCreateTableHeader(ctx.createTableHeader)
+ // TODO: implement temporary tables
+ if (temp) {
+ throw new AnalysisException(
--- End diff --
I'll address all of these in SPARK-14414. For now I just made this (and a few others) an `AnalysisException` so we can reuse some code in testing. I'll fix this before 2.0.
---
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209115331
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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209169833
**[Test build #55666 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55666/consoleFull)** for PR 12271 at commit [`8dc554a`](https://github.com/apache/spark/commit/8dc554a38c9989fc43b119645bfe5c8ceb7b6cdb).
* 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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59589025
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -675,34 +680,38 @@ private[hive] class HiveClientImpl(
private def toHiveTable(table: CatalogTable): HiveTable = {
val hiveTable = new HiveTable(table.database, table.identifier.table)
- // For EXTERNAL_TABLE/MANAGED_TABLE, we also need to set EXTERNAL field in
- // the table properties accodringly. Otherwise, if EXTERNAL_TABLE is the table type
- // but EXTERNAL field is not set, Hive metastore will change the type to
- // MANAGED_TABLE (see
- // metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L1095-L1105)
+ // For EXTERNAL_TABLE, we also need to set EXTERNAL field in the table properties.
+ // Otherwise, Hive metastore will change the table to a MANAGED_TABLE.
+ // (metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L1095-L1105)
hiveTable.setTableType(table.tableType match {
case CatalogTableType.EXTERNAL_TABLE =>
hiveTable.setProperty("EXTERNAL", "TRUE")
HiveTableType.EXTERNAL_TABLE
case CatalogTableType.MANAGED_TABLE =>
- hiveTable.setProperty("EXTERNAL", "FALSE")
HiveTableType.MANAGED_TABLE
case CatalogTableType.INDEX_TABLE => HiveTableType.INDEX_TABLE
case CatalogTableType.VIRTUAL_VIEW => HiveTableType.VIRTUAL_VIEW
})
- hiveTable.setFields(table.schema.map(toHiveColumn).asJava)
- hiveTable.setPartCols(table.partitionColumns.map(toHiveColumn).asJava)
+ // Note: In Hive the schema and partition columns must be disjoint sets
+ val (partCols, schema) = table.schema.map(toHiveColumn).partition { c =>
+ table.partitionColumnNames.contains(c.getName)
+ }
+ hiveTable.setFields(schema.asJava)
+ hiveTable.setPartCols(partCols.asJava)
// TODO: set sort columns here too
+ hiveTable.setBucketCols(table.bucketColumnNames.asJava)
hiveTable.setOwner(conf.getUser)
hiveTable.setNumBuckets(table.numBuckets)
hiveTable.setCreateTime((table.createTime / 1000).toInt)
hiveTable.setLastAccessTime((table.lastAccessTime / 1000).toInt)
table.storage.locationUri.foreach { loc => shim.setDataLocation(hiveTable, loc) }
table.storage.inputFormat.map(toInputFormat).foreach(hiveTable.setInputFormatClass)
table.storage.outputFormat.map(toOutputFormat).foreach(hiveTable.setOutputFormatClass)
- table.storage.serde.foreach(hiveTable.setSerializationLib)
+ hiveTable.setSerializationLib(
+ table.storage.serde.getOrElse("org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe"))
--- End diff --
Now, after a few bad experiences, I think we need to add all the test cases to ensure the Hive APIs work as what we expected. See another issue I hit in `alter table ... drop partition`: https://github.com/apache/spark/pull/12220#discussion-diff-59555113
---
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209272015
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: [SPARK-14388][SQL][WIP] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-207750180
**[Test build #55426 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55426/consoleFull)** for PR 12271 at commit [`f7501d9`](https://github.com/apache/spark/commit/f7501d9ebc5c4f08374788a937de6a56689258b8).
* 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: [SPARK-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209169944
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55666/
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-14388][SQL][WIP] Implement CREATE TABLE
Posted by andrewor14 <gi...@git.apache.org>.
Github user andrewor14 commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59280215
--- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/catalog/interface.scala ---
@@ -218,14 +219,35 @@ case class CatalogTable(
tableType: CatalogTableType,
storage: CatalogStorageFormat,
schema: Seq[CatalogColumn],
- partitionColumns: Seq[CatalogColumn] = Seq.empty,
- sortColumns: Seq[CatalogColumn] = Seq.empty,
+ partitionColumnNames: Seq[String] = Seq.empty,
+ sortColumnNames: Seq[String] = Seq.empty,
--- End diff --
also that has nothing to do with this patch
---
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209158268
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55658/
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-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59492629
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/execution/HiveSqlParser.scala ---
@@ -121,84 +123,115 @@ class HiveSqlAstBuilder extends SparkSqlAstBuilder {
}
/**
- * Create a [[CatalogStorageFormat]]. This is part of the [[CreateTableAsSelect]] command.
+ * Create a [[CatalogStorageFormat]] for creating tables.
*/
override def visitCreateFileFormat(
ctx: CreateFileFormatContext): CatalogStorageFormat = withOrigin(ctx) {
- if (ctx.storageHandler == null) {
- typedVisit[CatalogStorageFormat](ctx.fileFormat)
- } else {
- visitStorageHandler(ctx.storageHandler)
+ (ctx.fileFormat, ctx.storageHandler) match {
+ // Expected format: INPUTFORMAT input_format OUTPUTFORMAT output_format
+ case (c: TableFileFormatContext, null) =>
+ visitTableFileFormat(c)
+ // Expected format: SEQUENCEFILE | TEXTFILE | RCFILE | ORC | PARQUET | AVRO
+ case (c: GenericFileFormatContext, null) =>
+ visitGenericFileFormat(c)
+ case (null, storageHandler) =>
+ throw new ParseException("Operation not allowed: ... STORED BY storage_handler ...", ctx)
+ case _ =>
+ throw new ParseException("expected either STORED AS or STORED BY, not both", ctx)
}
}
/**
- * Create a [[CreateTableAsSelect]] command.
+ * Create a table, returning either a [[CreateTable]] or a [[CreateTableAsSelect]].
+ *
+ * This is not used to create datasource tables, which is handled through
+ * "CREATE TABLE ... USING ...".
+ *
+ * Note: several features are currently not supported - temporary tables, bucketing,
+ * skewed columns and storage handlers (STORED BY).
+ *
+ * Expected format:
+ * {{{
+ * CREATE [TEMPORARY] [EXTERNAL] TABLE [IF NOT EXISTS] [db_name.]table_name
+ * [(col1 data_type [COMMENT col_comment], ...)]
+ * [COMMENT table_comment]
+ * [PARTITIONED BY (col3 data_type [COMMENT col_comment], ...)]
+ * [CLUSTERED BY (col1, ...) [SORTED BY (col1 [ASC|DESC], ...)] INTO num_buckets BUCKETS]
+ * [SKEWED BY (col1, col2, ...) ON ((col_value, col_value, ...), ...) [STORED AS DIRECTORIES]]
+ * [ROW FORMAT row_format]
+ * [STORED AS file_format | STORED BY storage_handler_class [WITH SERDEPROPERTIES (...)]]
+ * [LOCATION path]
+ * [TBLPROPERTIES (property_name=property_value, ...)]
+ * [AS select_statement];
+ * }}}
*/
- override def visitCreateTable(ctx: CreateTableContext): LogicalPlan = {
- if (ctx.query == null) {
- HiveNativeCommand(command(ctx))
+ override def visitCreateTable(ctx: CreateTableContext): LogicalPlan = withOrigin(ctx) {
+ val (name, temp, ifNotExists, external) = visitCreateTableHeader(ctx.createTableHeader)
+ // TODO: implement temporary tables
+ if (temp) {
+ throw new ParseException(
+ "CREATE TEMPORARY TABLE is not supported yet. " +
+ "Please use registerTempTable as an alternative.", ctx)
+ }
+ if (ctx.skewSpec != null) {
+ throw new ParseException("Operation not allowed: CREATE TABLE ... SKEWED BY ...", ctx)
+ }
+ if (ctx.bucketSpec != null) {
+ throw new ParseException("Operation not allowed: CREATE TABLE ... CLUSTERED BY ...", ctx)
+ }
+ val tableType = if (external) {
+ CatalogTableType.EXTERNAL_TABLE
} else {
- // Get the table header.
- val (table, temp, ifNotExists, external) = visitCreateTableHeader(ctx.createTableHeader)
- val tableType = if (external) {
- CatalogTableType.EXTERNAL_TABLE
- } else {
- CatalogTableType.MANAGED_TABLE
- }
-
- // Unsupported clauses.
- if (temp) {
- throw new ParseException(s"Unsupported operation: TEMPORARY clause.", ctx)
- }
- if (ctx.bucketSpec != null) {
- // TODO add this - we need cluster columns in the CatalogTable for this to work.
- throw new ParseException("Unsupported operation: " +
- "CLUSTERED BY ... [ORDERED BY ...] INTO ... BUCKETS clause.", ctx)
- }
- if (ctx.skewSpec != null) {
- throw new ParseException("Operation not allowed: " +
- "SKEWED BY ... ON ... [STORED AS DIRECTORIES] clause.", ctx)
- }
-
- // Create the schema.
- val schema = Option(ctx.columns).toSeq.flatMap(visitCatalogColumns(_, _.toLowerCase))
-
- // Get the column by which the table is partitioned.
- val partitionCols = Option(ctx.partitionColumns).toSeq.flatMap(visitCatalogColumns(_))
-
- // Create the storage.
- def format(fmt: ParserRuleContext): CatalogStorageFormat = {
- Option(fmt).map(typedVisit[CatalogStorageFormat]).getOrElse(EmptyStorageFormat)
- }
- // Default storage.
+ CatalogTableType.MANAGED_TABLE
+ }
+ val comment = Option(ctx.STRING).map(string)
+ val partitionCols = Option(ctx.partitionColumns).toSeq.flatMap(visitCatalogColumns)
+ val cols = Option(ctx.columns).toSeq.flatMap(visitCatalogColumns)
+ val properties = Option(ctx.tablePropertyList).map(visitTablePropertyList).getOrElse(Map.empty)
+ val selectQuery = Option(ctx.query).map(plan)
+
+ // Note: Hive requires partition columns to be distinct from the schema, so we need
+ // to include the partition columns here explicitly
+ val schema = cols ++ partitionCols
+
+ // Storage format
+ val defaultStorage: CatalogStorageFormat = {
val defaultStorageType = hiveConf.getVar(HiveConf.ConfVars.HIVEDEFAULTFILEFORMAT)
- val hiveSerDe = HiveSerDe.sourceToSerDe(defaultStorageType, hiveConf).getOrElse {
- HiveSerDe(
- inputFormat = Option("org.apache.hadoop.mapred.TextInputFormat"),
- outputFormat = Option("org.apache.hadoop.hive.ql.io.IgnoreKeyTextOutputFormat"))
- }
- // Defined storage.
- val fileStorage = format(ctx.createFileFormat)
- val rowStorage = format(ctx.rowFormat)
- val storage = CatalogStorageFormat(
- Option(ctx.locationSpec).map(visitLocationSpec),
- fileStorage.inputFormat.orElse(hiveSerDe.inputFormat),
- fileStorage.outputFormat.orElse(hiveSerDe.outputFormat),
- rowStorage.serde.orElse(hiveSerDe.serde).orElse(fileStorage.serde),
- rowStorage.serdeProperties ++ fileStorage.serdeProperties
- )
+ val defaultHiveSerde = HiveSerDe.sourceToSerDe(defaultStorageType, hiveConf)
+ CatalogStorageFormat(
+ locationUri = None,
+ inputFormat = defaultHiveSerde.flatMap(_.inputFormat)
+ .orElse(Some("org.apache.hadoop.mapred.TextInputFormat")),
+ outputFormat = defaultHiveSerde.flatMap(_.outputFormat)
+ .orElse(Some("org.apache.hadoop.hive.ql.io.IgnoreKeyTextOutputFormat")),
+ serde = defaultHiveSerde.flatMap(_.serde)
+ .orElse(Some("org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe")),
--- End diff --
I guess we should fill this in as late as possible? Probably in `toHiveTable`? In `HiveClientImpl.toHiveTable`, we first create a `org.apache.hadoop.hive.ql.metadata.Table`, which uses `MetadataTypedColumnsetSerDe` as the default one (so, originally `InsertIntoHiveTableSuite` was broken without this change).
---
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-14388][SQL] Implement CREATE TABLE
Posted by AmplabJenkins <gi...@git.apache.org>.
Github user AmplabJenkins commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209135570
Test FAILed.
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/55654/
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-14388][SQL] Implement CREATE TABLE
Posted by gatorsmile <gi...@git.apache.org>.
Github user gatorsmile commented on a diff in the pull request:
https://github.com/apache/spark/pull/12271#discussion_r59593820
--- Diff: sql/hive/src/main/scala/org/apache/spark/sql/hive/client/HiveClientImpl.scala ---
@@ -675,34 +680,38 @@ private[hive] class HiveClientImpl(
private def toHiveTable(table: CatalogTable): HiveTable = {
val hiveTable = new HiveTable(table.database, table.identifier.table)
- // For EXTERNAL_TABLE/MANAGED_TABLE, we also need to set EXTERNAL field in
- // the table properties accodringly. Otherwise, if EXTERNAL_TABLE is the table type
- // but EXTERNAL field is not set, Hive metastore will change the type to
- // MANAGED_TABLE (see
- // metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L1095-L1105)
+ // For EXTERNAL_TABLE, we also need to set EXTERNAL field in the table properties.
+ // Otherwise, Hive metastore will change the table to a MANAGED_TABLE.
+ // (metastore/src/java/org/apache/hadoop/hive/metastore/ObjectStore.java#L1095-L1105)
hiveTable.setTableType(table.tableType match {
case CatalogTableType.EXTERNAL_TABLE =>
hiveTable.setProperty("EXTERNAL", "TRUE")
HiveTableType.EXTERNAL_TABLE
case CatalogTableType.MANAGED_TABLE =>
- hiveTable.setProperty("EXTERNAL", "FALSE")
HiveTableType.MANAGED_TABLE
case CatalogTableType.INDEX_TABLE => HiveTableType.INDEX_TABLE
case CatalogTableType.VIRTUAL_VIEW => HiveTableType.VIRTUAL_VIEW
})
- hiveTable.setFields(table.schema.map(toHiveColumn).asJava)
- hiveTable.setPartCols(table.partitionColumns.map(toHiveColumn).asJava)
+ // Note: In Hive the schema and partition columns must be disjoint sets
+ val (partCols, schema) = table.schema.map(toHiveColumn).partition { c =>
+ table.partitionColumnNames.contains(c.getName)
+ }
+ hiveTable.setFields(schema.asJava)
+ hiveTable.setPartCols(partCols.asJava)
// TODO: set sort columns here too
+ hiveTable.setBucketCols(table.bucketColumnNames.asJava)
hiveTable.setOwner(conf.getUser)
hiveTable.setNumBuckets(table.numBuckets)
hiveTable.setCreateTime((table.createTime / 1000).toInt)
hiveTable.setLastAccessTime((table.lastAccessTime / 1000).toInt)
table.storage.locationUri.foreach { loc => shim.setDataLocation(hiveTable, loc) }
table.storage.inputFormat.map(toInputFormat).foreach(hiveTable.setInputFormatClass)
table.storage.outputFormat.map(toOutputFormat).foreach(hiveTable.setOutputFormatClass)
- table.storage.serde.foreach(hiveTable.setSerializationLib)
+ hiveTable.setSerializationLib(
+ table.storage.serde.getOrElse("org.apache.hadoop.hive.serde2.lazy.LazySimpleSerDe"))
--- End diff --
Yeah, agree if you are talking about my case. : )
---
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-14388][SQL] Implement CREATE TABLE
Posted by SparkQA <gi...@git.apache.org>.
Github user SparkQA commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209172735
**[Test build #55674 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/55674/consoleFull)** for PR 12271 at commit [`8e273fd`](https://github.com/apache/spark/commit/8e273fdc4f95d08cb6d09f4641472861587a3a01).
---
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-14388][SQL] Implement CREATE TABLE
Posted by yhuai <gi...@git.apache.org>.
Github user yhuai commented on the pull request:
https://github.com/apache/spark/pull/12271#issuecomment-209030454
LGTM
---
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