You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "zeruibao (via GitHub)" <gi...@apache.org> on 2023/10/05 18:49:28 UTC
[PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
zeruibao opened a new pull request, #43232:
URL: https://github.com/apache/spark/pull/43232
### What changes were proposed in this pull request?
Mapped TINYINT to ShortType for MsSqlServerDialect
### Why are the changes needed?
TINYINT of SQL server is not correctly mapped to spark types when using JDBC connector. For now, it is mapped to the `IntegerType`, which is not accurate. It should be mapped to `ShortType`.
### Does this PR introduce _any_ user-facing change?
Yes, after the change, `TINYINT` of SQL server will be mapped to `ShortType`
### How was this patch tested?
UT
### Was this patch authored or co-authored using generative AI tooling?
No
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "zeruibao (via GitHub)" <gi...@apache.org>.
zeruibao commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348964160
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
Sure, just add a comment!
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ByteType for MsSqlServerDialect [spark]
Posted by "zeruibao (via GitHub)" <gi...@apache.org>.
zeruibao commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348018735
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
Yeah agree. Let me make this change.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ByteType for MsSqlServerDialect [spark]
Posted by "zeruibao (via GitHub)" <gi...@apache.org>.
zeruibao commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348024885
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
To make it consistent across all database, can we make it a ShortType instead?
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "zeruibao (via GitHub)" <gi...@apache.org>.
zeruibao commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348031386
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
For mysql, TinyInt can be a signed value. I think it's more appropriate to use ShortType instead.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "zeruibao (via GitHub)" <gi...@apache.org>.
zeruibao commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348125559
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
I don't think it's Symmetrical.
SMALLINT and TINYTINT -> ShortType
ShortType -> SMALLINT
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on PR #43232:
URL: https://github.com/apache/spark/pull/43232#issuecomment-1751440578
Thanks, merging to master
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348032699
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
ByteType is signed too
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348124072
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
Also, in JDBC, is it required to be Symmetrical for `getCatalystType` and `getJDBCType`?
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "zeruibao (via GitHub)" <gi...@apache.org>.
zeruibao commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348031386
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
For mysql, TinyInt can be a signed value.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425][SQL] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "beliefer (via GitHub)" <gi...@apache.org>.
beliefer commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1349454359
##########
connector/docker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MsSqlServerIntegrationSuite.scala:
##########
@@ -193,7 +197,11 @@ class MsSqlServerIntegrationSuite extends DockerJDBCIntegrationSuite {
assert(types(10).equals("class java.math.BigDecimal"))
assert(types(11).equals("class java.math.BigDecimal"))
assert(row.getBoolean(0) == false)
- assert(row.getInt(1) == 255)
+ if (flag) {
+ assert(row.getInt(1) == 255)
+ } else {
+ assert(row.getShort(1) == 255)
Review Comment:
LGTM later.
Surprisingly, SQL Server's tinyint does not support negative numbers.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348943709
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
interesting. Can we add a code comment to mention it and reference to the sqlserver doc?
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "zeruibao (via GitHub)" <gi...@apache.org>.
zeruibao commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348900999
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
https://learn.microsoft.com/en-us/sql/t-sql/data-types/int-bigint-smallint-and-tinyint-transact-sql?view=sql-server-ver16
You can refer to this doc. Thank you.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425][SQL] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang closed pull request #43232: [SPARK-45425][SQL] Mapped TINYINT to ShortType for MsSqlServerDialect
URL: https://github.com/apache/spark/pull/43232
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "zeruibao (via GitHub)" <gi...@apache.org>.
zeruibao commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348018735
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
Yeah agree. Let me make this change.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348122247
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
@zeruibao I am leaving this one to @yaooqinn @MaxGekk @sadikovi
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348008214
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
We should map it to `ByteType`
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "cloud-fan (via GitHub)" <gi...@apache.org>.
cloud-fan commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348727887
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
is it true only for TINYINT or all integer types? If INT can be unsigned as well, then shall we map INT to LongType?
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "zeruibao (via GitHub)" <gi...@apache.org>.
zeruibao commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348897524
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
It's true only for TINYINT.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "zeruibao (via GitHub)" <gi...@apache.org>.
zeruibao commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348278886
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
I just tried it. Using `ByteType` to read `TinyInt` of 255, I got **-1** instead of **255**. I have to use **ShortType** to get the correct answer.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "gengliangwang (via GitHub)" <gi...@apache.org>.
gengliangwang commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348028419
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
> To make it consistent across all database
could you provide more details?
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "zeruibao (via GitHub)" <gi...@apache.org>.
zeruibao commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348122216
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
From the doc https://spark.apache.org/docs/latest/sql-ref-datatypes.html, it seems that it cannot represent unsigned value like 255.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "sadikovi (via GitHub)" <gi...@apache.org>.
sadikovi commented on code in PR #43232:
URL: https://github.com/apache/spark/pull/43232#discussion_r1348141605
##########
sql/core/src/main/scala/org/apache/spark/sql/jdbc/MsSqlServerDialect.scala:
##########
@@ -109,7 +109,7 @@ private object MsSqlServerDialect extends JdbcDialect {
None
} else {
sqlType match {
- case java.sql.Types.SMALLINT => Some(ShortType)
+ case java.sql.Types.SMALLINT | java.sql.Types.TINYINT => Some(ShortType)
Review Comment:
What happens if you use ByteType for TINYINT? Have you tried that? I think it is worth trying it on a few versions of SQL Server with mapping to ByteType or ShortType and see what results you get.
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-45425] Mapped TINYINT to ShortType for MsSqlServerDialect [spark]
Posted by "zeruibao (via GitHub)" <gi...@apache.org>.
zeruibao commented on PR #43232:
URL: https://github.com/apache/spark/pull/43232#issuecomment-1751439707
All tests passed now, can we merge it? @gengliangwang
--
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-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org