Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
Hisoka-X commented on PR #41855: URL: https://github.com/apache/spark/pull/41855#issuecomment-1764064853 Thanks @MaxGekk and all! -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
MaxGekk closed pull request #41855: [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect URL: https://github.com/apache/spark/pull/41855 -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
MaxGekk commented on PR #41855: URL: https://github.com/apache/spark/pull/41855#issuecomment-1764023875 +1, LGTM. Merging to master. Thank you, @Hisoka-X and @cloud-fan @beliefer @yaooqinn @HyukjinKwon @fbiville for review. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
cloud-fan commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1358221878
##
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala:
##
@@ -114,22 +115,19 @@ object JdbcUtils extends Logging with SQLConfHelper {
isCaseSensitive: Boolean,
dialect: JdbcDialect): String = {
Review Comment:
note: `getInsertStatement` is not a public API but we don't stop people from
calling `JdbcUtils` as there is no `private[spark]`. I'm fine keeping it to
avoid troubles for third-party Spark vendors.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
Hisoka-X commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1358194353
##
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala:
##
@@ -104,34 +105,6 @@ object JdbcUtils extends Logging with SQLConfHelper {
JdbcDialects.get(url).isCascadingTruncateTable()
}
- /**
- * Returns an Insert SQL statement for inserting a row into the target table
via JDBC conn.
- */
- def getInsertStatement(
Review Comment:
addressed all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
beliefer commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1358189908
##
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala:
##
@@ -670,7 +643,6 @@ object JdbcUtils extends Logging with SQLConfHelper {
* updated even with error if it doesn't support transaction, as there're
dirty outputs.
*/
def savePartition(
- table: String,
Review Comment:
Yes.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
Hisoka-X commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1358187351
##
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala:
##
@@ -670,7 +643,6 @@ object JdbcUtils extends Logging with SQLConfHelper {
* updated even with error if it doesn't support transaction, as there're
dirty outputs.
*/
def savePartition(
- table: String,
Review Comment:
Should I revert this as well? @beliefer
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
beliefer commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1358154414
##
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala:
##
@@ -104,34 +105,6 @@ object JdbcUtils extends Logging with SQLConfHelper {
JdbcDialects.get(url).isCascadingTruncateTable()
}
- /**
- * Returns an Insert SQL statement for inserting a row into the target table
via JDBC conn.
- */
- def getInsertStatement(
Review Comment:
This is a public API too. Shall we refactor `getInsertStatement` by use
`dialect.insertIntoTable` ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
Hisoka-X commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1358145882
##
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala:
##
@@ -854,7 +854,6 @@ object JdbcUtils extends Logging with SQLConfHelper {
def saveTable(
df: DataFrame,
tableSchema: Option[StructType],
- isCaseSensitive: Boolean,
Review Comment:
done.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
beliefer commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1358121363
##
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala:
##
@@ -854,7 +854,6 @@ object JdbcUtils extends Logging with SQLConfHelper {
def saveTable(
df: DataFrame,
tableSchema: Option[StructType],
- isCaseSensitive: Boolean,
Review Comment:
This is a public API.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
Hisoka-X commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1358087806
##
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala:
##
@@ -854,7 +854,6 @@ object JdbcUtils extends Logging with SQLConfHelper {
def saveTable(
df: DataFrame,
tableSchema: Option[StructType],
- isCaseSensitive: Boolean,
Review Comment:
Is there any special reason?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
beliefer commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1358085152
##
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala:
##
@@ -854,7 +854,6 @@ object JdbcUtils extends Logging with SQLConfHelper {
def saveTable(
df: DataFrame,
tableSchema: Option[StructType],
- isCaseSensitive: Boolean,
Review Comment:
Please not change the signature. We still not use it is OK.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
Hisoka-X commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1358041445
##
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala:
##
@@ -193,6 +193,26 @@ abstract class JdbcDialect extends Serializable with
Logging {
statement.executeUpdate(s"CREATE TABLE $tableName ($strSchema)
$createTableOptions")
}
+ /**
+ * Returns an Insert SQL statement template for inserting a row into the
target table via JDBC
+ * conn. Use "?" as placeholder for each value to be inserted.
+ * E.g. `INSERT INTO t ("name", "age", "gender") VALUES (?, ?, ?)`
+ *
+ * @param table The name of the table.
+ * @param fields The fields of the row that will be inserted.
+ * @param isCaseSensitive Whether the table name and field names are case
sensitive.
Review Comment:
Reverted.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
beliefer commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1357992627
##
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala:
##
@@ -193,6 +193,26 @@ abstract class JdbcDialect extends Serializable with
Logging {
statement.executeUpdate(s"CREATE TABLE $tableName ($strSchema)
$createTableOptions")
}
+ /**
+ * Returns an Insert SQL statement template for inserting a row into the
target table via JDBC
+ * conn. Use "?" as placeholder for each value to be inserted.
+ * E.g. `INSERT INTO t ("name", "age", "gender") VALUES (?, ?, ?)`
+ *
+ * @param table The name of the table.
+ * @param fields The fields of the row that will be inserted.
+ * @param isCaseSensitive Whether the table name and field names are case
sensitive.
Review Comment:
Maybe we can remove the isCaseSensitive parameter now.
We can add an option `isCaseSensitive` if users find the use case in future.
##
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala:
##
@@ -193,6 +193,26 @@ abstract class JdbcDialect extends Serializable with
Logging {
statement.executeUpdate(s"CREATE TABLE $tableName ($strSchema)
$createTableOptions")
}
+ /**
+ * Returns an Insert SQL statement template for inserting a row into the
target table via JDBC
+ * conn. Use "?" as placeholder for each value to be inserted.
+ * E.g. `INSERT INTO t ("name", "age", "gender") VALUES (?, ?, ?)`
+ *
+ * @param table The name of the table.
+ * @param fields The fields of the row that will be inserted.
+ * @param isCaseSensitive Whether the table name and field names are case
sensitive.
Review Comment:
Maybe we can remove the `isCaseSensitive` parameter now.
We can add an option `isCaseSensitive` if users find the use case in future.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
cloud-fan commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1357981977
##
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala:
##
@@ -193,6 +193,26 @@ abstract class JdbcDialect extends Serializable with
Logging {
statement.executeUpdate(s"CREATE TABLE $tableName ($strSchema)
$createTableOptions")
}
+ /**
+ * Returns an Insert SQL statement template for inserting a row into the
target table via JDBC
+ * conn. Use "?" as placeholder for each value to be inserted.
+ * E.g. `INSERT INTO t ("name", "age", "gender") VALUES (?, ?, ?)`
+ *
+ * @param table The name of the table.
+ * @param fields The fields of the row that will be inserted.
+ * @param isCaseSensitive Whether the table name and field names are case
sensitive.
Review Comment:
we can if it's useful. what's the use case?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
beliefer commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1357829064
##
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala:
##
@@ -193,6 +193,26 @@ abstract class JdbcDialect extends Serializable with
Logging {
statement.executeUpdate(s"CREATE TABLE $tableName ($strSchema)
$createTableOptions")
}
+ /**
+ * Returns an Insert SQL statement template for inserting a row into the
target table via JDBC
+ * conn. Use "?" as placeholder for each value to be inserted.
+ * E.g. `INSERT INTO t ("name", "age", "gender") VALUES (?, ?, ?)`
+ *
+ * @param table The name of the table.
+ * @param fields The fields of the row that will be inserted.
+ * @param isCaseSensitive Whether the table name and field names are case
sensitive.
Review Comment:
@cloud-fan Could we add an option `isCaseSensitive`?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
Hisoka-X commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1356840216
##
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala:
##
@@ -193,6 +193,26 @@ abstract class JdbcDialect extends Serializable with
Logging {
statement.executeUpdate(s"CREATE TABLE $tableName ($strSchema)
$createTableOptions")
}
+ /**
+ * Returns an Insert SQL statement template for inserting a row into the
target table via JDBC
+ * conn. Use "?" as placeholder for each value to be inserted.
+ * E.g. `INSERT INTO t ("name", "age", "gender") VALUES (?, ?, ?)`
+ *
+ * @param table The name of the table.
+ * @param fields The fields of the row that will be inserted.
+ * @param isCaseSensitive Whether the table name and field names are case
sensitive.
Review Comment:
I feel same with you... @beliefer Could you give us some tips? Thanks a lot.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
cloud-fan commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1356835446
##
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala:
##
@@ -193,6 +193,26 @@ abstract class JdbcDialect extends Serializable with
Logging {
statement.executeUpdate(s"CREATE TABLE $tableName ($strSchema)
$createTableOptions")
}
+ /**
+ * Returns an Insert SQL statement template for inserting a row into the
target table via JDBC
+ * conn. Use "?" as placeholder for each value to be inserted.
+ * E.g. `INSERT INTO t ("name", "age", "gender") VALUES (?, ?, ?)`
+ *
+ * @param table The name of the table.
+ * @param fields The fields of the row that will be inserted.
+ * @param isCaseSensitive Whether the table name and field names are case
sensitive.
Review Comment:
Can you give some examples of how the implementation should respect this
case sensitivity flag? I don't quite get it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
Hisoka-X commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1356698826
##
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala:
##
@@ -104,34 +105,6 @@ object JdbcUtils extends Logging with SQLConfHelper {
JdbcDialects.get(url).isCascadingTruncateTable()
}
- /**
- * Returns an Insert SQL statement for inserting a row into the target table
via JDBC conn.
- */
- def getInsertStatement(
- table: String,
- rddSchema: StructType,
- tableSchema: Option[StructType],
- isCaseSensitive: Boolean,
Review Comment:
Addressed. Please take a look again. Thanks.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
beliefer commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1356653007
##
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala:
##
@@ -104,34 +105,6 @@ object JdbcUtils extends Logging with SQLConfHelper {
JdbcDialects.get(url).isCascadingTruncateTable()
}
- /**
- * Returns an Insert SQL statement for inserting a row into the target table
via JDBC conn.
- */
- def getInsertStatement(
- table: String,
- rddSchema: StructType,
- tableSchema: Option[StructType],
- isCaseSensitive: Boolean,
Review Comment:
@Hisoka-X I know that the caller of `JdbcUtils.getInsertStatement` uses the
config "spark.sql.caseSensitive".
Even if the origin `isCaseSensitive` is not used at all and I think the
case-sensitive could decided by jdbc dialect.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
cloud-fan commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1356522186
##
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala:
##
@@ -104,34 +105,6 @@ object JdbcUtils extends Logging with SQLConfHelper {
JdbcDialects.get(url).isCascadingTruncateTable()
}
- /**
- * Returns an Insert SQL statement for inserting a row into the target table
via JDBC conn.
- */
- def getInsertStatement(
- table: String,
- rddSchema: StructType,
- tableSchema: Option[StructType],
- isCaseSensitive: Boolean,
Review Comment:
+1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
Hisoka-X commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1356466802
##
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala:
##
@@ -104,34 +105,6 @@ object JdbcUtils extends Logging with SQLConfHelper {
JdbcDialects.get(url).isCascadingTruncateTable()
}
- /**
- * Returns an Insert SQL statement for inserting a row into the target table
via JDBC conn.
- */
- def getInsertStatement(
- table: String,
- rddSchema: StructType,
- tableSchema: Option[StructType],
- isCaseSensitive: Boolean,
Review Comment:
I'm a little confused about this. Whether a database is case-sensitive
should be determined based on the implementation of the dialect. If there are
two databases, one is case-sensitive and the other is not, how should we
configure this configuration? I understand that this configuration should only
be about whether Spark SQL is case-sensitive, and has nothing to do with
whether the database is case-sensitive. Am I right?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
beliefer commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1356421158
##
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala:
##
@@ -104,34 +105,6 @@ object JdbcUtils extends Logging with SQLConfHelper {
JdbcDialects.get(url).isCascadingTruncateTable()
}
- /**
- * Returns an Insert SQL statement for inserting a row into the target table
via JDBC conn.
- */
- def getInsertStatement(
- table: String,
- rddSchema: StructType,
- tableSchema: Option[StructType],
- isCaseSensitive: Boolean,
Review Comment:
The origin interface keep the `isCaseSensitive` is useful, even if it was
not used in the code directly.
It seems we'd better add `isCaseSensitive` to the
`JdbcDialect.insertIntoTable`.
Maybe some jdbc dialect is case sensitive.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
Hisoka-X commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1356398498
##
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala:
##
@@ -193,6 +193,22 @@ abstract class JdbcDialect extends Serializable with
Logging {
statement.executeUpdate(s"CREATE TABLE $tableName ($strSchema)
$createTableOptions")
}
+ /**
+ * Returns an Insert SQL statement for inserting a row into the target table
via JDBC conn.
Review Comment:
Done
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
cloud-fan commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1356384362
##
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala:
##
@@ -193,6 +193,22 @@ abstract class JdbcDialect extends Serializable with
Logging {
statement.executeUpdate(s"CREATE TABLE $tableName ($strSchema)
$createTableOptions")
}
+ /**
+ * Returns an Insert SQL statement for inserting a row into the target table
via JDBC conn.
Review Comment:
To be more accurate
```
Returns an Insert SQL statement template ...
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
Hisoka-X commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1356380866
##
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala:
##
@@ -193,6 +193,22 @@ abstract class JdbcDialect extends Serializable with
Logging {
statement.executeUpdate(s"CREATE TABLE $tableName ($strSchema)
$createTableOptions")
}
+ /**
+ * Returns an Insert SQL statement for inserting a row into the target table
via JDBC conn.
+ *
+ * @param table The name of the table.
+ * @param fields The fields of the row that will be inserted.
+ * @return The SQL query to use for insert data into table.
+ */
+ @Since("4.0.0")
+ def insertIntoTable(
+table: String,
Review Comment:
addressed.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
Hisoka-X commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1356380332
##
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala:
##
@@ -193,6 +193,22 @@ abstract class JdbcDialect extends Serializable with
Logging {
statement.executeUpdate(s"CREATE TABLE $tableName ($strSchema)
$createTableOptions")
}
+ /**
+ * Returns an Insert SQL statement for inserting a row into the target table
via JDBC conn.
Review Comment:
What about now?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
Re: [PR] [SPARK-44262][SQL] Add `dropTable` and `getInsertStatement` to JdbcDialect [spark]
cloud-fan commented on code in PR #41855:
URL: https://github.com/apache/spark/pull/41855#discussion_r1356116942
##
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala:
##
@@ -193,6 +193,22 @@ abstract class JdbcDialect extends Serializable with
Logging {
statement.executeUpdate(s"CREATE TABLE $tableName ($strSchema)
$createTableOptions")
}
+ /**
+ * Returns an Insert SQL statement for inserting a row into the target table
via JDBC conn.
Review Comment:
We need more document here, to explain the placeholder thing.
##
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala:
##
@@ -193,6 +193,22 @@ abstract class JdbcDialect extends Serializable with
Logging {
statement.executeUpdate(s"CREATE TABLE $tableName ($strSchema)
$createTableOptions")
}
+ /**
+ * Returns an Insert SQL statement for inserting a row into the target table
via JDBC conn.
+ *
+ * @param table The name of the table.
+ * @param fields The fields of the row that will be inserted.
+ * @return The SQL query to use for insert data into table.
+ */
+ @Since("4.0.0")
+ def insertIntoTable(
+table: String,
Review Comment:
nit: 4 spaces indentation
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
