You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by GitBox <gi...@apache.org> on 2021/02/05 18:30:23 UTC
[GitHub] [spark] sarutak opened a new pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
sarutak opened a new pull request #31491:
URL: https://github.com/apache/spark/pull/31491
### What changes were proposed in this pull request?
<!--
Please clarify what changes you are proposing. The purpose of this section is to outline the changes and how this PR fixes the issue.
If possible, please consider writing useful notes for better and faster reviews in your PR. See the examples below.
1. If you refactor some codes with changing classes, showing the class hierarchy will help reviewers.
2. If you fix some SQL features, you can provide some references of other DBMSes.
3. If there is design documentation, please add the link.
4. If there is a discussion in the mailing list, please add the link.
-->
This PR fix an issue that `java.sql.RowId` is mapped to `LongType` and prefer `StringType`.
In the current implementation, JDBC RowID type is mapped to `LongType` except for `OracleDialect`, but there is no guarantee to be able to convert RowID to long.
`java.sql.RowId` declares `toString` and the specification of `java.sql.RowId` says
> _all methods on the RowId interface must be fully implemented if the JDBC driver supports the data type_
(https://docs.oracle.com/javase/8/docs/api/java/sql/RowId.html)
So, we should prefer StringType to LongType.
### Why are the changes needed?
<!--
Please clarify why the changes are needed. For instance,
1. If you propose a new API, clarify the use case for a new API.
2. If you fix a bug, you can clarify why it is a bug.
-->
This seems to be a potential bug.
### Does this PR introduce _any_ user-facing change?
<!--
Note that it means *any* user-facing change including all aspects such as the documentation fix.
If yes, please clarify the previous behavior and the change this PR proposes - provide the console output, description and/or an example to show the behavior difference if possible.
If possible, please also clarify if this is a user-facing change compared to the released Spark versions or within the unreleased branches such as master.
If no, write 'No'.
-->
Yes. RowID is mapped to StringType rather than LongType.
### How was this patch tested?
<!--
If tests were added, say they were added here. Please make sure to add some test cases that check the changes thoroughly including negative and positive cases if possible.
If it was tested in a way different from regular unit tests, please clarify how you tested step by step, ideally copy and paste-able, so that other reviewers can test and check, and descendants can verify in the future.
If tests were not added, please describe why they were not added and/or why it was difficult to add.
-->
The existing test case `SPARK-32992: map Oracle's ROWID type to StringType` in `OracleIntegrationSuite` passes.
The test is about the RowID conversion.
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-780248988
**[Test build #135184 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135184/testReport)** for PR 31491 at commit [`73f53d9`](https://github.com/apache/spark/commit/73f53d91e609a7450819ee3bb14b81eee34d42f9).
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA removed a comment on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-776545551
**[Test build #135096 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135096/testReport)** for PR 31491 at commit [`315c7e5`](https://github.com/apache/spark/commit/315c7e5aa21a4090c9b44accac028d5cd76bf9ff).
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-776545551
**[Test build #135096 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135096/testReport)** for PR 31491 at commit [`315c7e5`](https://github.com/apache/spark/commit/315c7e5aa21a4090c9b44accac028d5cd76bf9ff).
----------------------------------------------------------------
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.
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
[GitHub] [spark] maropu commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-776697707
It looks fine otherwise (note: I've checked that `OracleIntegrationSuite` can pass)
----------------------------------------------------------------
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.
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
[GitHub] [spark] srowen commented on a change in pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
srowen commented on a change in pull request #31491:
URL: https://github.com/apache/spark/pull/31491#discussion_r574642812
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -226,7 +226,7 @@ object JdbcUtils extends Logging {
case java.sql.Types.REAL => DoubleType
case java.sql.Types.REF => StringType
case java.sql.Types.REF_CURSOR => null
- case java.sql.Types.ROWID => LongType
+ case java.sql.Types.ROWID => StringType
Review comment:
So basically we can't assume the row ID is <= 8 bytes? if that's true then I agree.
----------------------------------------------------------------
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.
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
[GitHub] [spark] sarutak commented on a change in pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #31491:
URL: https://github.com/apache/spark/pull/31491#discussion_r571755432
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -226,7 +226,7 @@ object JdbcUtils extends Logging {
case java.sql.Types.REAL => DoubleType
case java.sql.Types.REF => StringType
case java.sql.Types.REF_CURSOR => null
- case java.sql.Types.ROWID => LongType
+ case java.sql.Types.ROWID => StringType
Review comment:
Thanks. I've added.
----------------------------------------------------------------
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.
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
[GitHub] [spark] maropu commented on a change in pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #31491:
URL: https://github.com/apache/spark/pull/31491#discussion_r573431650
##########
File path: docs/sql-migration-guide.md
##########
@@ -24,6 +24,8 @@ license: |
## Upgrading from Spark SQL 3.1 to 3.2
+ - Since Spark 3.2, all the supported JDBC dialects use StringType for ROWID. Previously, Oracle dialect uses StringType and the other dialects use LongType.
Review comment:
nit: Previously, => In Spark 3.1 or earlier,
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-776635937
Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39678/
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-776705376
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135096/
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-774338901
**[Test build #134939 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134939/testReport)** for PR 31491 at commit [`222be6e`](https://github.com/apache/spark/commit/222be6ea3e76bbd471dba88bdee6bc173826d6dc).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-774268029
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39522/
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA removed a comment on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-774230546
**[Test build #134939 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134939/testReport)** for PR 31491 at commit [`222be6e`](https://github.com/apache/spark/commit/222be6ea3e76bbd471dba88bdee6bc173826d6dc).
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-780297927
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135184/
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-774960235
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134997/
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-774947449
**[Test build #134997 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134997/testReport)** for PR 31491 at commit [`899706f`](https://github.com/apache/spark/commit/899706f5d89f29c4c4d93db92179da081f5bb10d).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-774872221
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39580/
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-774288816
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39522/
----------------------------------------------------------------
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.
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
[GitHub] [spark] sarutak commented on a change in pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #31491:
URL: https://github.com/apache/spark/pull/31491#discussion_r573846295
##########
File path: docs/sql-migration-guide.md
##########
@@ -24,6 +24,8 @@ license: |
## Upgrading from Spark SQL 3.1 to 3.2
+ - Since Spark 3.2, all the supported JDBC dialects use StringType for ROWID. In Spark 3.1 or earlier, Oracle dialect uses StringType and the other dialects use LongType.
Review comment:
At least, I use `dialects` here as a general word, not represents specific implementations like `PostgresDialect`.
`dialect` and `dialects` have been used from before in the migration guide.
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-774841886
**[Test build #134997 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134997/testReport)** for PR 31491 at commit [`899706f`](https://github.com/apache/spark/commit/899706f5d89f29c4c4d93db92179da081f5bb10d).
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-774960235
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134997/
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-776577692
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39678/
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-776698083
**[Test build #135096 has finished](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/135096/testReport)** for PR 31491 at commit [`315c7e5`](https://github.com/apache/spark/commit/315c7e5aa21a4090c9b44accac028d5cd76bf9ff).
* This patch passes all tests.
* This patch merges cleanly.
* This patch adds no public classes.
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-774253651
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39522/
----------------------------------------------------------------
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.
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
[GitHub] [spark] maropu commented on a change in pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #31491:
URL: https://github.com/apache/spark/pull/31491#discussion_r573701737
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala
##########
@@ -1781,4 +1783,28 @@ class JDBCSuite extends QueryTest
assert(options.asProperties.get("url") == url)
assert(options.asProperties.get("dbtable") == "table3")
}
+
+ test("SPARK-34379: Map JDBC RowID to StringType rather than LongType") {
+ val mockRsmd = mock(classOf[java.sql.ResultSetMetaData])
Review comment:
h2 database cannot generate rowid-typed data?
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-774872221
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39580/
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-780297927
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135184/
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-776643185
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39678/
----------------------------------------------------------------
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.
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
[GitHub] [spark] maropu commented on a change in pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
maropu commented on a change in pull request #31491:
URL: https://github.com/apache/spark/pull/31491#discussion_r573709268
##########
File path: docs/sql-migration-guide.md
##########
@@ -24,6 +24,8 @@ license: |
## Upgrading from Spark SQL 3.1 to 3.2
+ - Since Spark 3.2, all the supported JDBC dialects use StringType for ROWID. In Spark 3.1 or earlier, Oracle dialect uses StringType and the other dialects use LongType.
Review comment:
`dialects` is an internal word? If so, how about saying "Since Spark 3.2, \`java.sql.ROWID\` is mapped to \`StringType\` when reading data from other databases via JDBC"?
----------------------------------------------------------------
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.
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
[GitHub] [spark] asfgit closed pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
asfgit closed pull request #31491:
URL: https://github.com/apache/spark/pull/31491
----------------------------------------------------------------
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.
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
[GitHub] [spark] maropu commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
maropu commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-776425851
Could you add tests in the `core` package?
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-774230546
**[Test build #134939 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134939/testReport)** for PR 31491 at commit [`222be6e`](https://github.com/apache/spark/commit/222be6ea3e76bbd471dba88bdee6bc173826d6dc).
----------------------------------------------------------------
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.
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
[GitHub] [spark] sarutak commented on a change in pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #31491:
URL: https://github.com/apache/spark/pull/31491#discussion_r574655496
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -226,7 +226,7 @@ object JdbcUtils extends Logging {
case java.sql.Types.REAL => DoubleType
case java.sql.Types.REF => StringType
case java.sql.Types.REF_CURSOR => null
- case java.sql.Types.ROWID => LongType
+ case java.sql.Types.ROWID => StringType
Review comment:
Not only we can't assume the length of the ROWID but also it's not required to be represented as integer.
JDBC RowId declares `getBytes` and `toString` to represent ROWID so I think we can safely map ROWID to StringType.
https://docs.oracle.com/javase/8/docs/api/java/sql/RowId.html
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-774339535
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134939/
----------------------------------------------------------------
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.
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
[GitHub] [spark] sarutak commented on a change in pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #31491:
URL: https://github.com/apache/spark/pull/31491#discussion_r573841048
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala
##########
@@ -1781,4 +1783,28 @@ class JDBCSuite extends QueryTest
assert(options.asProperties.get("url") == url)
assert(options.asProperties.get("dbtable") == "table3")
}
+
+ test("SPARK-34379: Map JDBC RowID to StringType rather than LongType") {
+ val mockRsmd = mock(classOf[java.sql.ResultSetMetaData])
Review comment:
It cannot.
H2 has `_rowid_` as a hidden column but it's not compatible with JDBC ROWID.
`_rowid_` is represented as `long` and H2 doesn't support `getRowId`.
https://github.com/h2database/h2database/blob/6290b79a2418189c5faa0e0506bf6503fc7630e6/h2/src/main/org/h2/jdbc/JdbcResultSet.java#L3292
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-776705376
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/135096/
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-774288816
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39522/
----------------------------------------------------------------
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.
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
[GitHub] [spark] srowen commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
srowen commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-782681399
@sarutak feel free to merge when ready
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-780279168
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39765/
----------------------------------------------------------------
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.
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
[GitHub] [spark] sarutak commented on a change in pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #31491:
URL: https://github.com/apache/spark/pull/31491#discussion_r571755432
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -226,7 +226,7 @@ object JdbcUtils extends Logging {
case java.sql.Types.REAL => DoubleType
case java.sql.Types.REF => StringType
case java.sql.Types.REF_CURSOR => null
- case java.sql.Types.ROWID => LongType
+ case java.sql.Types.ROWID => StringType
Review comment:
Thanks. I've updated.
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-774853447
Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39580/
----------------------------------------------------------------
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.
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
[GitHub] [spark] sarutak commented on a change in pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #31491:
URL: https://github.com/apache/spark/pull/31491#discussion_r573841048
##########
File path: sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala
##########
@@ -1781,4 +1783,28 @@ class JDBCSuite extends QueryTest
assert(options.asProperties.get("url") == url)
assert(options.asProperties.get("dbtable") == "table3")
}
+
+ test("SPARK-34379: Map JDBC RowID to StringType rather than LongType") {
+ val mockRsmd = mock(classOf[java.sql.ResultSetMetaData])
Review comment:
It cannot.
H2 has `_rowid_` as a hidden column but it's not compatible with JDBC ROWID.
`_rowid_` is represented as `long` and it's not `getRowId`.
https://github.com/h2database/h2database/blob/6290b79a2418189c5faa0e0506bf6503fc7630e6/h2/src/main/org/h2/jdbc/JdbcResultSet.java#L3292
----------------------------------------------------------------
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.
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
[GitHub] [spark] sarutak commented on a change in pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #31491:
URL: https://github.com/apache/spark/pull/31491#discussion_r573846295
##########
File path: docs/sql-migration-guide.md
##########
@@ -24,6 +24,8 @@ license: |
## Upgrading from Spark SQL 3.1 to 3.2
+ - Since Spark 3.2, all the supported JDBC dialects use StringType for ROWID. In Spark 3.1 or earlier, Oracle dialect uses StringType and the other dialects use LongType.
Review comment:
At least, I use `dialects` here as a general word, not represent specific implementations like `PostgresDialect`.
`dialect` and `dialects` have used from before in the migration guide.
----------------------------------------------------------------
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.
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
[GitHub] [spark] sarutak commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-782692634
Thanks all. Merged 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.
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
[GitHub] [spark] AmplabJenkins removed a comment on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins removed a comment on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-774339535
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder/134939/
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-780279168
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39765/
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
SparkQA commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-774863125
Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/39580/
----------------------------------------------------------------
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.
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
[GitHub] [spark] SparkQA removed a comment on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
SparkQA removed a comment on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-774841886
**[Test build #134997 has started](https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder/134997/testReport)** for PR 31491 at commit [`899706f`](https://github.com/apache/spark/commit/899706f5d89f29c4c4d93db92179da081f5bb10d).
----------------------------------------------------------------
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.
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
[GitHub] [spark] sarutak commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
sarutak commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-776525459
@maropu I've added.
----------------------------------------------------------------
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.
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
[GitHub] [spark] AmplabJenkins commented on pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
AmplabJenkins commented on pull request #31491:
URL: https://github.com/apache/spark/pull/31491#issuecomment-776643185
Refer to this link for build results (access rights to CI server needed):
https://amplab.cs.berkeley.edu/jenkins//job/SparkPullRequestBuilder-K8s/39678/
----------------------------------------------------------------
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.
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
[GitHub] [spark] sarutak commented on a change in pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
sarutak commented on a change in pull request #31491:
URL: https://github.com/apache/spark/pull/31491#discussion_r573846295
##########
File path: docs/sql-migration-guide.md
##########
@@ -24,6 +24,8 @@ license: |
## Upgrading from Spark SQL 3.1 to 3.2
+ - Since Spark 3.2, all the supported JDBC dialects use StringType for ROWID. In Spark 3.1 or earlier, Oracle dialect uses StringType and the other dialects use LongType.
Review comment:
At least, I use `dialects` here as a general word, not represents specific implementations like `PostgresDialect`.
`dialect` and `dialects` have used from before in the migration guide.
----------------------------------------------------------------
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.
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
[GitHub] [spark] HyukjinKwon commented on a change in pull request #31491: [SPARK-34379][SQL] Map JDBC RowID to StringType rather than LongType
Posted by GitBox <gi...@apache.org>.
HyukjinKwon commented on a change in pull request #31491:
URL: https://github.com/apache/spark/pull/31491#discussion_r571520005
##########
File path: sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala
##########
@@ -226,7 +226,7 @@ object JdbcUtils extends Logging {
case java.sql.Types.REAL => DoubleType
case java.sql.Types.REF => StringType
case java.sql.Types.REF_CURSOR => null
- case java.sql.Types.ROWID => LongType
+ case java.sql.Types.ROWID => StringType
Review comment:
@sarutak should we maybe update migration guide?
----------------------------------------------------------------
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.
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