-
Notifications
You must be signed in to change notification settings - Fork 29k
[SPARK-14525][SQL] Make DataFrameWrite.save work for jdbc #12601
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
6be5046
db639a5
69f7c7b
88d181e
c44271e
0a98e45
cb9889e
d18efef
754b360
1d0d61c
c8e4143
f7f2615
95431e3
e8c2d7d
ae6ad8b
c387c17
57ac87e
de53734
379e00f
ea9d2fe
c686b0e
7ef7a48
c9dcdc4
447ab82
4a02c82
a238156
06c1cba
8fb86b4
724bbe2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -244,7 +244,11 @@ final class DataFrameWriter private[sql](df: DataFrame) { | |
| bucketSpec = getBucketSpec, | ||
| options = extraOptions.toMap) | ||
|
|
||
| dataSource.write(mode, df) | ||
| dataSource.providingClass.newInstance() match { | ||
| case jdbc: execution.datasources.jdbc.DefaultSource => | ||
| jdbc.write(mode, df, extraOptions.toMap) | ||
| case _ => dataSource.write(mode, df) | ||
| } | ||
| } | ||
|
|
||
| /** | ||
|
|
@@ -489,46 +493,12 @@ final class DataFrameWriter private[sql](df: DataFrame) { | |
| * @since 1.4.0 | ||
| */ | ||
| def jdbc(url: String, table: String, connectionProperties: Properties): Unit = { | ||
| val props = new Properties() | ||
| extraOptions.foreach { case (key, value) => | ||
| props.put(key, value) | ||
| } | ||
| import scala.collection.JavaConverters._ | ||
|
||
| // connectionProperties should override settings in extraOptions | ||
| props.putAll(connectionProperties) | ||
| val conn = JdbcUtils.createConnectionFactory(url, props)() | ||
|
|
||
| try { | ||
| var tableExists = JdbcUtils.tableExists(conn, url, table) | ||
|
|
||
| if (mode == SaveMode.Ignore && tableExists) { | ||
| return | ||
| } | ||
|
|
||
| if (mode == SaveMode.ErrorIfExists && tableExists) { | ||
| sys.error(s"Table $table already exists.") | ||
| } | ||
|
|
||
| if (mode == SaveMode.Overwrite && tableExists) { | ||
| JdbcUtils.dropTable(conn, table) | ||
| tableExists = false | ||
| } | ||
|
|
||
| // Create the table if the table didn't exist. | ||
| if (!tableExists) { | ||
| val schema = JdbcUtils.schemaString(df, url) | ||
| val sql = s"CREATE TABLE $table ($schema)" | ||
| val statement = conn.createStatement | ||
| try { | ||
| statement.executeUpdate(sql) | ||
| } finally { | ||
| statement.close() | ||
| } | ||
| } | ||
| } finally { | ||
| conn.close() | ||
| } | ||
|
|
||
| JdbcUtils.saveTable(df, url, table, props) | ||
| this.extraOptions = this.extraOptions ++ (connectionProperties.asScala) | ||
| // explicit url and dbtable should override all | ||
| this.extraOptions += ("url" -> url, "dbtable" -> table) | ||
| format("jdbc").save | ||
|
||
| } | ||
|
|
||
| /** | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -151,4 +151,16 @@ class JDBCWriteSuite extends SharedSQLContext with BeforeAndAfter { | |
| assert(2 === sqlContext.read.jdbc(url1, "TEST.PEOPLE1", properties).count) | ||
| assert(2 === sqlContext.read.jdbc(url1, "TEST.PEOPLE1", properties).collect()(0).length) | ||
| } | ||
|
|
||
| test("save works for format(\"jdbc\") if url and dbtable are set") { | ||
| val df = sqlContext.createDataFrame(sparkContext.parallelize(arr2x2), schema2) | ||
|
|
||
| df.write.format("jdbc") | ||
| .options(Map("url" -> url, "dbtable" -> "TEST.BASICCREATETEST")) | ||
| .save | ||
|
||
|
|
||
| assert(2 === sqlContext.read.jdbc(url, "TEST.BASICCREATETEST", new Properties).count) | ||
| assert( | ||
| 2 === sqlContext.read.jdbc(url, "TEST.BASICCREATETEST", new Properties).collect()(0).length) | ||
| } | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks a new method is introduced. I think we don't necessarily have to introduce this new function but use the existing interfaces, eg.
CreatableRelationProviderininterfaces.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree and admit I was being lazy in not trying to figure out how to make the current implementation return a
BaseRelation. I'll take another look today at just turning the DefaultSource into aCreatableRelationProvider