[GitHub] spark pull request #23094: [SPARK-26077][SQL] Reserved SQL words are not esc...
Github user golovan commented on a diff in the pull request:
https://github.com/apache/spark/pull/23094#discussion_r238246140
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala ---
@@ -87,11 +87,32 @@ abstract class JdbcDialect extends Serializable {
def getJDBCType(dt: DataType): Option[JdbcType] = None
/**
- * Quotes the identifier. This is used to put quotes around the
identifier in case the column
- * name is a reserved keyword, or in case it contains characters that
require quotes (e.g. space).
+ * Gets the character used for identifier quoting.
+ */
+ def getIdentifierQuoteCharacter: String = """
--- End diff --
> I like a simpler API design; how about splitting an identifier into the
two parts (db and table names) outside `JdbcDialect`? Then, how about applying
`quoteIdentifer` into each name part?
I'll review tonight. If meanwhile you can point me to the best place, you
are welcome!
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23094: [SPARK-26077][SQL] Reserved SQL words are not esc...
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/23094#discussion_r238239581
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala ---
@@ -87,11 +87,32 @@ abstract class JdbcDialect extends Serializable {
def getJDBCType(dt: DataType): Option[JdbcType] = None
/**
- * Quotes the identifier. This is used to put quotes around the
identifier in case the column
- * name is a reserved keyword, or in case it contains characters that
require quotes (e.g. space).
+ * Gets the character used for identifier quoting.
+ */
+ def getIdentifierQuoteCharacter: String = """
--- End diff --
btw, I feel we need more general name handling here like
`UnresolvedAttribute. parseAttributeName`
https://github.com/apache/spark/blob/master/sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/analysis/unresolved.scala#L151
cc: @gatorsmile
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23094: [SPARK-26077][SQL] Reserved SQL words are not esc...
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/23094#discussion_r238239195
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala ---
@@ -87,11 +87,32 @@ abstract class JdbcDialect extends Serializable {
def getJDBCType(dt: DataType): Option[JdbcType] = None
/**
- * Quotes the identifier. This is used to put quotes around the
identifier in case the column
- * name is a reserved keyword, or in case it contains characters that
require quotes (e.g. space).
+ * Gets the character used for identifier quoting.
+ */
+ def getIdentifierQuoteCharacter: String = """
--- End diff --
I like a simpler API design; how about splitting an identifier into the two
parts (db and table names) outside `JdbcDialect`? Then, how about applying
`quoteIdentifer` into each name part?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23094: [SPARK-26077][SQL] Reserved SQL words are not esc...
Github user golovan commented on a diff in the pull request:
https://github.com/apache/spark/pull/23094#discussion_r238235137
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala ---
@@ -87,11 +87,32 @@ abstract class JdbcDialect extends Serializable {
def getJDBCType(dt: DataType): Option[JdbcType] = None
/**
- * Quotes the identifier. This is used to put quotes around the
identifier in case the column
- * name is a reserved keyword, or in case it contains characters that
require quotes (e.g. space).
+ * Gets the character used for identifier quoting.
+ */
+ def getIdentifierQuoteCharacter: String = """
--- End diff --
Yes, as we also need to support cases when we have not only table but
schema as well. E.g. for MySQL DB.TABLE becomes `DB`.`TABLE`
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23094: [SPARK-26077][SQL] Reserved SQL words are not esc...
Github user maropu commented on a diff in the pull request:
https://github.com/apache/spark/pull/23094#discussion_r238232050
--- Diff:
sql/core/src/main/scala/org/apache/spark/sql/jdbc/JdbcDialects.scala ---
@@ -87,11 +87,32 @@ abstract class JdbcDialect extends Serializable {
def getJDBCType(dt: DataType): Option[JdbcType] = None
/**
- * Quotes the identifier. This is used to put quotes around the
identifier in case the column
- * name is a reserved keyword, or in case it contains characters that
require quotes (e.g. space).
+ * Gets the character used for identifier quoting.
+ */
+ def getIdentifierQuoteCharacter: String = """
--- End diff --
We need this new API instead of `quoteIdentifier`?
---
-
To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org
For additional commands, e-mail: reviews-h...@spark.apache.org
[GitHub] spark pull request #23094: [SPARK-26077][SQL] Reserved SQL words are not esc...
GitHub user golovan opened a pull request: https://github.com/apache/spark/pull/23094 [SPARK-26077][SQL] Reserved SQL words are not escaped by JDBC writer ⦠â¦for table name ## What changes were proposed in this pull request? Currently, JDBC Writer doesn't quote tables names. This PR uses dialects to quote the them, too. ## How was this patch tested? Related unit tests adjusted. In addition added new unit test to cover this bug. All tests are passing. You can merge this pull request into a Git repository by running: $ git pull https://github.com/golovan/spark SPARK-26077 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/spark/pull/23094.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 #23094 commit ddbff7974583832f4ed7f8a04fdf61cb6caf4961 Author: Eugene Golovan Date: 2018-11-20T10:39:57Z [SPARK-26077][SQL] Reserved SQL words are not escaped by JDBC writer for table name --- - To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org
