[SPARK-35446] Override getJDBCType in MySQLDialect to map FloatType to FLOAT #32605

mariosmeim-db · 2021-05-20T09:29:02Z

What changes were proposed in this pull request?

Override getJDBCType method in MySQLDialect so that FloatType is mapped to FLOAT instead of REAL

Why are the changes needed?

MySQL treats REAL as a synonym to DOUBLE by default (see https://dev.mysql.com/doc/refman/8.0/en/numeric-types.html). Therefore, when creating a table with a column of REAL type, it will be created as DOUBLE. However, currently, MySQLDialect does not provide an implementation for getJDBCType, and will thus ultimately fall back to JdbcUtils.getCommonJDBCType, which maps FloatType to REAL. This change is needed so that we can properly map the FloatType to FLOAT for MySQL.

Does this PR introduce any user-facing change?

Prior to this PR, when writing a dataframe with a FloatType column to a MySQL table, it will create a DOUBLE column. After the PR, it will create a FLOAT column.

How was this patch tested?

Added a test case in JDBCSuite that verifies the mapping.

sql/core/src/main/scala/org/apache/spark/sql/jdbc/MySQLDialect.scala

sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala

HyukjinKwon · 2021-05-20T11:21:27Z

ok to test

HyukjinKwon · 2021-05-20T11:21:34Z

cc @maropu FYI

maropu · 2021-05-20T12:11:35Z

Thank you for the your contribution, @mariosmeim-db . Could you add tests in MySQLIntegrationSuite, too? Our test framework does not run the integration test now, so please run it on your env.

Thank you for pining me, @HyukjinKwon. btw, is it better to run these integration tests in GA, too?

SparkQA · 2021-05-20T12:49:39Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43270/

SparkQA · 2021-05-20T12:59:41Z

Test build #138748 has finished for PR 32605 at commit 30e6717.

This patch fails Spark unit tests.
This patch merges cleanly.
This patch adds no public classes.

SparkQA · 2021-05-20T13:24:57Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43270/

srowen · 2021-05-20T23:28:51Z

Jenkins test this please

SparkQA · 2021-05-20T23:57:23Z

Test build #138773 has started for PR 32605 at commit 30e6717.

SparkQA · 2021-05-21T00:59:50Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43296/

SparkQA · 2021-05-21T01:34:03Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43296/

maropu · 2021-05-22T06:28:48Z

Thank you for pining me, @HyukjinKwon. btw, is it better to run these integration tests in GA, too?

See #32620 for the related work.

SparkQA · 2021-05-25T16:12:23Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43454/

SparkQA · 2021-05-25T16:47:31Z

Kubernetes integration test status failure
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43454/

SparkQA · 2021-05-25T20:08:10Z

Test build #138933 has finished for PR 32605 at commit 60afab1.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon

I think it's fine.

HyukjinKwon · 2021-06-03T04:35:14Z

sql/core/src/main/scala/org/apache/spark/sql/jdbc/MySQLDialect.scala

+  override def getJDBCType(dt: DataType): Option[JdbcType] = dt match {
+    // See SPARK-35446: MySQL treats REAL as a synonym to DOUBLE by default
+    // We override getJDBCType so that FloatType is mapped to FLOAT instead
+    case FloatType => Option(JdbcType("FLOAT", java.sql.Types.FLOAT))


@mariosmeim-db sorry one last thing. can we update migration guide too? https://github.com/apache/spark/blob/master/docs/sql-migration-guide.md

SparkQA · 2021-06-04T09:09:57Z

Kubernetes integration test starting
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43850/

SparkQA · 2021-06-04T09:45:04Z

Kubernetes integration test status success
URL: https://amplab.cs.berkeley.edu/jenkins/job/SparkPullRequestBuilder-K8s/43850/

SparkQA · 2021-06-04T12:12:34Z

Test build #139328 has finished for PR 32605 at commit 77cf81c.

This patch passes all tests.
This patch merges cleanly.
This patch adds no public classes.

HyukjinKwon · 2021-06-05T03:44:07Z

Merged to master.

Override getJDBCType in MySQLDialect to map FloatType to FLOAT

da18503

github-actions bot added the SQL label May 20, 2021

mariosmeim-db added 3 commits May 20, 2021 11:56

re-trigger tests

615338a

revert import changes

795948b

added type imports

30e6717

HyukjinKwon reviewed May 20, 2021

View reviewed changes

sql/core/src/main/scala/org/apache/spark/sql/jdbc/MySQLDialect.scala Outdated Show resolved Hide resolved

HyukjinKwon reviewed May 20, 2021

View reviewed changes

sql/core/src/test/scala/org/apache/spark/sql/jdbc/JDBCSuite.scala Outdated Show resolved Hide resolved

Merge branch 'master' of github.com:apache/spark into SPARK-35446

f54267c

Addressed comments

60afab1

mariosmeim-db requested a review from HyukjinKwon May 27, 2021 15:46

mariosmeim-db changed the title ~~[WIP][SPARK-35446] Override getJDBCType in MySQLDialect to map FloatType to FLOAT~~ [SPARK-35446] Override getJDBCType in MySQLDialect to map FloatType to FLOAT May 27, 2021

mariosmeim-db requested a review from maropu May 27, 2021 15:47

HyukjinKwon approved these changes May 28, 2021

View reviewed changes

HyukjinKwon reviewed Jun 3, 2021

View reviewed changes

mariosmeim-db added 2 commits June 4, 2021 10:05

Merge branch 'master' of github.com:apache/spark into SPARK-35446

24863b8

added migration note

77cf81c

github-actions bot added the DOCS label Jun 4, 2021

HyukjinKwon closed this in b5678be Jun 5, 2021

[SPARK-35446] Override getJDBCType in MySQLDialect to map FloatType to FLOAT #32605

[SPARK-35446] Override getJDBCType in MySQLDialect to map FloatType to FLOAT #32605

Uh oh!

Conversation

mariosmeim-db commented May 20, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What changes were proposed in this pull request?

Why are the changes needed?

Does this PR introduce any user-facing change?

How was this patch tested?

Uh oh!

Uh oh!

Uh oh!

HyukjinKwon commented May 20, 2021

Uh oh!

HyukjinKwon commented May 20, 2021

Uh oh!

maropu commented May 20, 2021

Uh oh!

SparkQA commented May 20, 2021

Uh oh!

SparkQA commented May 20, 2021

Uh oh!

SparkQA commented May 20, 2021

Uh oh!

srowen commented May 20, 2021

Uh oh!

SparkQA commented May 20, 2021

Uh oh!

SparkQA commented May 21, 2021

Uh oh!

SparkQA commented May 21, 2021

Uh oh!

maropu commented May 22, 2021

Uh oh!

SparkQA commented May 25, 2021

Uh oh!

SparkQA commented May 25, 2021

Uh oh!

SparkQA commented May 25, 2021

Uh oh!

HyukjinKwon left a comment

Choose a reason for hiding this comment

Uh oh!

HyukjinKwon Jun 3, 2021

Choose a reason for hiding this comment

Uh oh!

SparkQA commented Jun 4, 2021

Uh oh!

SparkQA commented Jun 4, 2021

Uh oh!

SparkQA commented Jun 4, 2021

Uh oh!

HyukjinKwon commented Jun 5, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

mariosmeim-db commented May 20, 2021 •

edited

Loading