[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-13 Thread asfgit
Github user asfgit closed the pull request at:

https://github.com/apache/spark/pull/14559


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-12 Thread HyukjinKwon
Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74542387
  
--- Diff: 
sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCWriteSuite.scala ---
@@ -174,6 +174,18 @@ class JDBCWriteSuite extends SharedSQLContext with 
BeforeAndAfter {
 JdbcDialects.unregisterDialect(testH2Dialect)
   }
 
+  test("createTableOptions") {
+JdbcDialects.registerDialect(testH2Dialect)
+val df = spark.createDataFrame(sparkContext.parallelize(arr2x2), 
schema2)
+
+val m = intercept[org.h2.jdbc.JdbcSQLException] {
+  df.write.option("createTableOptions", "ENGINE tableEngineName")
+  .jdbc(url1, "TEST.CREATETBLOPTS", properties)
+  }.getMessage
--- End diff --

Maybe the indentation can be as below (it seems there are extra double 
spaces now):

```scala
val m = intercept[org.h2.jdbc.JdbcSQLException] {
  ...
}.getMessage
```


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-12 Thread GraceH
Github user GraceH commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74550111
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -423,6 +423,10 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
 props.putAll(connectionProperties)
 val conn = JdbcUtils.createConnectionFactory(url, props)()
 
+// to add required options like URL and dbtable
+val params = extraOptions.toMap ++ Map("url" -> url, "dbtable" -> 
table)
+val jdbcOptions = new JDBCOptions(params)
+
 try {
   var tableExists = JdbcUtils.tableExists(conn, url, table)
--- End diff --

BTW, there are several places of (url and table). shall we replace all 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-12 Thread HyukjinKwon
Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74542181
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCOptions.scala
 ---
@@ -20,14 +20,21 @@ package org.apache.spark.sql.execution.datasources.jdbc
 /**
  * Options for the JDBC data source.
  */
-private[jdbc] class JDBCOptions(
+private[sql] class JDBCOptions(
--- End diff --

Actually, if my understanding is correct, we don't need `private[sql]` 
here. (See  
https://github.com/apache/spark/commit/511f52f8423e151b0d0133baf040d34a0af3d422)



---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-12 Thread HyukjinKwon
Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74550467
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -423,6 +423,10 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
 props.putAll(connectionProperties)
 val conn = JdbcUtils.createConnectionFactory(url, props)()
 
+// to add required options like URL and dbtable
+val params = extraOptions.toMap ++ Map("url" -> url, "dbtable" -> 
table)
+val jdbcOptions = new JDBCOptions(params)
+
 try {
   var tableExists = JdbcUtils.tableExists(conn, url, table)
--- End diff --

Yes, I think we can.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-12 Thread GraceH
Github user GraceH commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74546716
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -423,6 +423,10 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
 props.putAll(connectionProperties)
 val conn = JdbcUtils.createConnectionFactory(url, props)()
 
+// to add required options like URL and dbtable
+val params = extraOptions.toMap ++ Map("url" -> url, "dbtable" -> 
table)
+val jdbcOptions = new JDBCOptions(params)
+
 try {
   var tableExists = JdbcUtils.tableExists(conn, url, table)
--- End diff --

Sure. make sense.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-12 Thread HyukjinKwon
Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74546013
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -423,6 +423,10 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
 props.putAll(connectionProperties)
 val conn = JdbcUtils.createConnectionFactory(url, props)()
 
+// to add required options like URL and dbtable
+val params = extraOptions.toMap ++ Map("url" -> url, "dbtable" -> 
table)
+val jdbcOptions = new JDBCOptions(params)
+
 try {
   var tableExists = JdbcUtils.tableExists(conn, url, table)
--- End diff --

Maybe I think we can use `jsonOptions.url` and `jsonOptions.table` here. I 
know there are no differences but as we made the `JDBCOptions` object to tie up 
the extra options.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-11 Thread GraceH
Github user GraceH commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74542628
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JDBCOptions.scala
 ---
@@ -20,14 +20,21 @@ package org.apache.spark.sql.execution.datasources.jdbc
 /**
  * Options for the JDBC data source.
  */
-private[jdbc] class JDBCOptions(
+private[sql] class JDBCOptions(
--- End diff --

OK. Just intend to follow that origin style. I will fix that. 


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-10 Thread GraceH
Github user GraceH commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74278231
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -447,7 +447,11 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
   // Create the table if the table didn't exist.
   if (!tableExists) {
 val schema = JdbcUtils.schemaString(df, url)
-val sql = s"CREATE TABLE $table ($schema)"
+// To allow certain options to append when create a new table, 
which can be
+// table_options or partition_options.
+// E.g., "CREATE TABLE t (name string) ENGINE=InnoDB DEFAULT 
CHARSET=utf8"
+val createtblOptions = 
extraOptions.getOrElse("createTableOptions", "")
--- End diff --

@HyukjinKwon  For those database specific options can be merged as 
createTableOptions. Not recommend to enumerate them one by one. Just as @srowen 
suggested.

Thanks to @HyukjinKwon and @srowen. I will propose a draft, and back to 
both of you later. 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-10 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74207701
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -447,7 +447,11 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
   // Create the table if the table didn't exist.
   if (!tableExists) {
 val schema = JdbcUtils.schemaString(df, url)
-val sql = s"CREATE TABLE $table ($schema)"
+// To allow certain options to append when create a new table, 
which can be
+// table_options or partition_options.
+// E.g., "CREATE TABLE t (name string) ENGINE=InnoDB DEFAULT 
CHARSET=utf8"
+val createtblOptions = 
extraOptions.getOrElse("createTableOptions", "")
--- End diff --

Oh good point about JDBCOptions. I wouldn't mind fixing truncate while 
we're here, yeah, although it's a slightly different issue.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-10 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74207520
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -447,7 +447,11 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
   // Create the table if the table didn't exist.
   if (!tableExists) {
 val schema = JdbcUtils.schemaString(df, url)
-val sql = s"CREATE TABLE $table ($schema)"
+// To allow certain options to append when create a new table, 
which can be
+// table_options or partition_options.
+// E.g., "CREATE TABLE t (name string) ENGINE=InnoDB DEFAULT 
CHARSET=utf8"
+val createtblOptions = 
extraOptions.getOrElse("createTableOptions", "")
--- End diff --

These options are all database-specific, so we can give an example like 
this but not document all 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-10 Thread HyukjinKwon
Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74192910
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -447,7 +447,11 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
   // Create the table if the table didn't exist.
   if (!tableExists) {
 val schema = JdbcUtils.schemaString(df, url)
-val sql = s"CREATE TABLE $table ($schema)"
+// To allow certain options to append when create a new table, 
which can be
+// table_options or partition_options.
+// E.g., "CREATE TABLE t (name string) ENGINE=InnoDB DEFAULT 
CHARSET=utf8"
+val createtblOptions = 
extraOptions.getOrElse("createTableOptions", "")
--- End diff --

(BTW, I am not used to the options such as `ENGINE=InnoDB DEFAULT 
CHARSET=utf8`. Maybe we can just wait for the better feedback.)


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-10 Thread HyukjinKwon
Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74192492
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -447,7 +447,11 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
   // Create the table if the table didn't exist.
   if (!tableExists) {
 val schema = JdbcUtils.schemaString(df, url)
-val sql = s"CREATE TABLE $table ($schema)"
+// To allow certain options to append when create a new table, 
which can be
+// table_options or partition_options.
+// E.g., "CREATE TABLE t (name string) ENGINE=InnoDB DEFAULT 
CHARSET=utf8"
+val createtblOptions = 
extraOptions.getOrElse("createTableOptions", "")
--- End diff --

I think it'd make sense if those options are documented too because I am 
sure all the options should be documented rather than only some.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-09 Thread GraceH
Github user GraceH commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74188842
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -447,7 +447,11 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
   // Create the table if the table didn't exist.
   if (!tableExists) {
 val schema = JdbcUtils.schemaString(df, url)
-val sql = s"CREATE TABLE $table ($schema)"
+// To allow certain options to append when create a new table, 
which can be
+// table_options or partition_options.
+// E.g., "CREATE TABLE t (name string) ENGINE=InnoDB DEFAULT 
CHARSET=utf8"
+val createtblOptions = 
extraOptions.getOrElse("createTableOptions", "")
--- End diff --

A quick question here. The JDBCOptions also contain URL and table 
information, like:
```
 // a JDBC URL
  val url = parameters.getOrElse("url", sys.error("Option 'url' not 
specified"))
  // name of table
  val table = parameters.getOrElse("dbtable", sys.error("Option 'dbtable' 
not specified"))
```
Shall we merge those information into current DataFrameWriter as well?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-09 Thread GraceH
Github user GraceH commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74187289
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -447,7 +447,11 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
   // Create the table if the table didn't exist.
   if (!tableExists) {
 val schema = JdbcUtils.schemaString(df, url)
-val sql = s"CREATE TABLE $table ($schema)"
+// To allow certain options to append when create a new table, 
which can be
+// table_options or partition_options.
+// E.g., "CREATE TABLE t (name string) ENGINE=InnoDB DEFAULT 
CHARSET=utf8"
+val createtblOptions = 
extraOptions.getOrElse("createTableOptions", "")
--- End diff --

OK. Let me take a look. and give a quick version.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-09 Thread HyukjinKwon
Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74178124
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -447,7 +447,11 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
   // Create the table if the table didn't exist.
   if (!tableExists) {
 val schema = JdbcUtils.schemaString(df, url)
-val sql = s"CREATE TABLE $table ($schema)"
+// To allow certain options to append when create a new table, 
which can be
+// table_options or partition_options.
+// E.g., "CREATE TABLE t (name string) ENGINE=InnoDB DEFAULT 
CHARSET=utf8"
+val createtblOptions = 
extraOptions.getOrElse("createTableOptions", "")
--- End diff --

One more thing is, I guess we might need a test in `JDBCWriteSuite`.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-09 Thread HyukjinKwon
Github user HyukjinKwon commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74177938
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -447,7 +447,11 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
   // Create the table if the table didn't exist.
   if (!tableExists) {
 val schema = JdbcUtils.schemaString(df, url)
-val sql = s"CREATE TABLE $table ($schema)"
+// To allow certain options to append when create a new table, 
which can be
+// table_options or partition_options.
+// E.g., "CREATE TABLE t (name string) ENGINE=InnoDB DEFAULT 
CHARSET=utf8"
+val createtblOptions = 
extraOptions.getOrElse("createTableOptions", "")
--- End diff --

I am just wondering if it is possible to put the option in `JDBCOptions` 
(maybe with `truncate` as well). Also, I guess i'd be better if this option is 
documented in both `DataFrameWriter` and `readwriter.py`. For example 
`truncate` is documented above. 

In addition, I just found the documentation for `truncate` is missed in 
`readwriter.py`. It might be nicer if this one is fixed together 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-09 Thread GraceH
Github user GraceH commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74029250
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -447,7 +447,16 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
   // Create the table if the table didn't exist.
   if (!tableExists) {
 val schema = JdbcUtils.schemaString(df, url)
-val sql = s"CREATE TABLE $table ($schema)"
+// To allow certain options to append when create a new table, 
which can be
+// table_options or partition_options.
+// E.g., "CREATE TABLE t (name string) ENGINE=InnoDB DEFAULT 
CHARSET=utf8"
+val createtblOptions = {
+  extraOptions.get("jdbc.create.table.options") match {
+case Some(value) => " " + value
+case None => ""
+  }
+}
+val sql = s"CREATE TABLE $table ($schema)" + createtblOptions
--- End diff --

Totally got your idea. Thanks a lot for the prompt response. I will update 
the 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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-09 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74028772
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -447,7 +447,16 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
   // Create the table if the table didn't exist.
   if (!tableExists) {
 val schema = JdbcUtils.schemaString(df, url)
-val sql = s"CREATE TABLE $table ($schema)"
+// To allow certain options to append when create a new table, 
which can be
+// table_options or partition_options.
+// E.g., "CREATE TABLE t (name string) ENGINE=InnoDB DEFAULT 
CHARSET=utf8"
+val createtblOptions = {
+  extraOptions.get("jdbc.create.table.options") match {
+case Some(value) => " " + value
+case None => ""
+  }
+}
+val sql = s"CREATE TABLE $table ($schema)" + createtblOptions
--- End diff --

You could `map` and then `getOrElse` to add the space. The space isn't a 
big deal anyway. I think it's tidier this way, though originally I was just 
suggesting not mixing concatenation and interpolation in one line.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-09 Thread GraceH
Github user GraceH commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74028584
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -447,7 +447,16 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
   // Create the table if the table didn't exist.
   if (!tableExists) {
 val schema = JdbcUtils.schemaString(df, url)
-val sql = s"CREATE TABLE $table ($schema)"
+// To allow certain options to append when create a new table, 
which can be
+// table_options or partition_options.
+// E.g., "CREATE TABLE t (name string) ENGINE=InnoDB DEFAULT 
CHARSET=utf8"
+val createtblOptions = {
+  extraOptions.get("jdbc.create.table.options") match {
+case Some(value) => " " + value
+case None => ""
+  }
+}
+val sql = s"CREATE TABLE $table ($schema)" + createtblOptions
--- End diff --

To have a better format(to tell two parts explicitly), how about this?
```
val createtblOptions = extraOptions.getOrElse("createTableOptions", "")
val sql = s"CREATE TABLE $table ($schema) $createtblOptions"
```
The only problem here is to introduce a redundant space if option is empty. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-09 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74027679
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -447,7 +447,16 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
   // Create the table if the table didn't exist.
   if (!tableExists) {
 val schema = JdbcUtils.schemaString(df, url)
-val sql = s"CREATE TABLE $table ($schema)"
+// To allow certain options to append when create a new table, 
which can be
+// table_options or partition_options.
+// E.g., "CREATE TABLE t (name string) ENGINE=InnoDB DEFAULT 
CHARSET=utf8"
+val createtblOptions = {
+  extraOptions.get("jdbc.create.table.options") match {
--- End diff --

Ah I think that's better, yeah. This is indeed just an option to the method 
call.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-09 Thread GraceH
Github user GraceH commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74027475
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -447,7 +447,16 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
   // Create the table if the table didn't exist.
   if (!tableExists) {
 val schema = JdbcUtils.schemaString(df, url)
-val sql = s"CREATE TABLE $table ($schema)"
+// To allow certain options to append when create a new table, 
which can be
+// table_options or partition_options.
+// E.g., "CREATE TABLE t (name string) ENGINE=InnoDB DEFAULT 
CHARSET=utf8"
+val createtblOptions = {
+  extraOptions.get("jdbc.create.table.options") match {
--- End diff --

Thanks Sean. Actually, here I have a little bit hesitation. For example, 
"mergeSchema" which may not be so that similar to the other option name 
(prefixed with "spark"). 
```
val mergedDF = spark.read.option("mergeSchema", 
"true").parquet("data/test_table")
```

How about to use some short name as "createTableOptions"?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-09 Thread GraceH
Github user GraceH commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74026903
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -447,7 +447,16 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
   // Create the table if the table didn't exist.
   if (!tableExists) {
 val schema = JdbcUtils.schemaString(df, url)
-val sql = s"CREATE TABLE $table ($schema)"
+// To allow certain options to append when create a new table, 
which can be
+// table_options or partition_options.
+// E.g., "CREATE TABLE t (name string) ENGINE=InnoDB DEFAULT 
CHARSET=utf8"
+val createtblOptions = {
+  extraOptions.get("jdbc.create.table.options") match {
+case Some(value) => " " + value
+case None => ""
+  }
+}
+val sql = s"CREATE TABLE $table ($schema)" + createtblOptions
--- End diff --

Yes. so right. will fix that, which looks better as a whole part. 


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-09 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74015718
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -447,7 +447,16 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
   // Create the table if the table didn't exist.
   if (!tableExists) {
 val schema = JdbcUtils.schemaString(df, url)
-val sql = s"CREATE TABLE $table ($schema)"
+// To allow certain options to append when create a new table, 
which can be
+// table_options or partition_options.
+// E.g., "CREATE TABLE t (name string) ENGINE=InnoDB DEFAULT 
CHARSET=utf8"
+val createtblOptions = {
+  extraOptions.get("jdbc.create.table.options") match {
+case Some(value) => " " + value
+case None => ""
+  }
+}
+val sql = s"CREATE TABLE $table ($schema)" + createtblOptions
--- End diff --

Why not also use interpolation for the new var?


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-09 Thread srowen
Github user srowen commented on a diff in the pull request:

https://github.com/apache/spark/pull/14559#discussion_r74015696
  
--- Diff: 
sql/core/src/main/scala/org/apache/spark/sql/DataFrameWriter.scala ---
@@ -447,7 +447,16 @@ final class DataFrameWriter[T] private[sql](ds: 
Dataset[T]) {
   // Create the table if the table didn't exist.
   if (!tableExists) {
 val schema = JdbcUtils.schemaString(df, url)
-val sql = s"CREATE TABLE $table ($schema)"
+// To allow certain options to append when create a new table, 
which can be
+// table_options or partition_options.
+// E.g., "CREATE TABLE t (name string) ENGINE=InnoDB DEFAULT 
CHARSET=utf8"
+val createtblOptions = {
+  extraOptions.get("jdbc.create.table.options") match {
--- End diff --

Probably need a different prop name starting with spark. See other option 
naming conventions. The outer scope isn't necessary.


---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org



[GitHub] spark pull request #14559: [SPARK-16968]Add additional options in jdbc when ...

2016-08-09 Thread GraceH
GitHub user GraceH opened a pull request:

https://github.com/apache/spark/pull/14559

[SPARK-16968]Add additional options in jdbc when creating a new table

## What changes were proposed in this pull request?

In the PR, we just allow the user to add additional options when create a 
new table in JDBC writer. 
The options can be table_options or partition_options.
E.g., "CREATE TABLE t (name string) ENGINE=InnoDB DEFAULT CHARSET=utf8"

## How was this patch tested?

(Please explain how this patch was tested. E.g. unit tests, integration 
tests, manual tests)
will apply test result soon.



You can merge this pull request into a Git repository by running:

$ git pull https://github.com/GraceH/spark jdbc_options

Alternatively you can review and apply these changes as the patch at:

https://github.com/apache/spark/pull/14559.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 #14559


commit b302b1c7ec75ae1e78d132f7ecdb9bb7f33816d4
Author: GraceH <93113...@qq.com>
Date:   2016-08-09T06:47:51Z

Add additional options in jdbc when creating a new table




---
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 infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---

-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org