[SPARK-48385][SQL] Migrate the jdbc driver of mariadb from `2.x` to `3.x` #46655

panbingkun · 2024-05-20T03:22:42Z

What changes were proposed in this pull request?

The pr aims to

migrate the jdbc driver of mariadb from 2.x to 3.x.
upgrade the image version of mariadb from 10.5.12 to 10.5.24.

Why are the changes needed?

1.First PR of Series 3.x:
https://github.com/mariadb-corporation/mariadb-connector-j/releases/tag/3.0.0-alpha

2.Seen from the Maven central repo, series 3. x should have become the mainstream, with faster release frequency
https://mvnrepository.com/artifact/org.mariadb.jdbc/mariadb-java-client

Does this PR introduce any user-facing change?

No.

How was this patch tested?

Update existed UT.
Pass GA

Was this patch authored or co-authored using generative AI tooling?

No.

panbingkun · 2024-05-22T07:26:40Z

...-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MariaDBKrbIntegrationSuite.scala

@@ -46,7 +46,7 @@ class MariaDBKrbIntegrationSuite extends DockerKrbJDBCIntegrationSuite {
    override val jdbcPort = 3306

    override def getJdbcUrl(ip: String, port: Int): String =
-      s"jdbc:mysql://$ip:$port/mysql?user=$principal"
+      s"jdbc:mysql://$ip:$port/mysql?permitMysqlScheme&user=$principal"


permitMysqlScheme:
https://mariadb.com/kb/en/mariadb-connector-j-303-release-notes/#jdbcmariadb-scheme

panbingkun · 2024-05-22T07:27:15Z

cc @yaooqinn

yaooqinn · 2024-05-22T08:59:08Z

shall we also update the server? https://mariadb.com/kb/en/mariadb-10-5-25-release-notes/

panbingkun · 2024-05-22T09:06:44Z

shall we also update the server? https://mariadb.com/kb/en/mariadb-10-5-25-release-notes/

Sure, Let me verify locally first.

panbingkun · 2024-05-22T09:18:42Z

shall we also update the server? https://mariadb.com/kb/en/mariadb-10-5-25-release-notes/

https://hub.docker.com/_/mariadb/tags?page=&page_size=&ordering=&name=10.5.25
It seems that the official image 10.5.25 is not ready yet, but 10.5.24 is ok.

dongjoon-hyun · 2024-05-22T18:04:02Z

...ocker-integration-tests/src/test/scala/org/apache/spark/sql/jdbc/MySQLIntegrationSuite.scala

+      assert(rows.get(0).isInstanceOf[Integer])
+      assert(rows.get(1).isInstanceOf[Long])
+      assert(rows.get(2).isInstanceOf[Integer])
+      assert(rows.get(3).isInstanceOf[BigDecimal])


Could you describe the root cause of this change, @panbingkun ? For me, this looks suspicious like a breaking change.

- assert(rows.get(3).isInstanceOf[Long]) + assert(rows.get(3).isInstanceOf[BigDecimal])

the root cause of this change

change:
index[0], Short => Integer
index[1], Integer => Long
index[3], Long => Decimal

Before - (mariadb jdbc driver: 2.7.12):

After - (mariadb jdbc driver: 3.4.0):

Summary
the data types obtained from the underlying meta are different

We can also add compatible logic in MySQLDialect##getCatalystType. Do we need to do this in this PR?

We can make changes as follows to keep this compatible:

If we need to do this, how about making related changes directly in this PR? Or a new seperated PR?

After - (mariadb jdbc driver: 3.4.0):

These mariadb SQL Data Type <-> JDBC Data Type mapping rules seem incorrect, do they have a statement why they made such change

Let me try to find it.

#46655 (comment)

panbingkun · 2024-05-23T09:15:31Z

Through comparative analysis, it is found that

In version 2.x, there is no distinction between signed and unsigned. For example, a type is defined in MySQL: small SMALLINT UNSIGNED
Through the mariadb jdbc driver, the final logic is:
https://github.com/mariadb-corporation/mariadb-connector-j/blob/maintenance/2.x/src/main/java/org/mariadb/jdbc/MariaDbResultSetMetaData.java#L283

https://github.com/mariadb-corporation/mariadb-connector-j/blob/maintenance/2.x/src/main/java/org/mariadb/jdbc/internal/ColumnType.java#L66

Types.SMALLINT will be returned to Spark

On the Spark side, we will go to the following logic:

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

Lines 142 to 147 in 14d3f44

    
           case Types.SMALLINT => 
        
             if (md.build().getBoolean("isSigned")) { 
        
               Some(ShortType) 
        
             } else { 
        
               Some(IntegerType) 
        
             }

Therefore, Integer will be finally obtained.

In version 3.x, there is a distinction between signed and unsigned. For example, a type is defined in MySQL: med MEDIUMINT UNSIGNED
Through the mariadb jdbc driver, the final logic is:
https://github.com/mariadb-corporation/mariadb-connector-j/blob/master/src/main/java/org/mariadb/jdbc/client/result/ResultSetMetaData.java#L232-L234

https://github.com/mariadb-corporation/mariadb-connector-j/blob/master/src/main/java/org/mariadb/jdbc/client/result/ResultSetMetaData.java#L297-L302

UnsignedSmallIntColumn
https://github.com/mariadb-corporation/mariadb-connector-j/blob/master/src/main/java/org/mariadb/jdbc/client/column/UnsignedSmallIntColumn.java

https://github.com/mariadb-corporation/mariadb-connector-j/blob/master/src/main/java/org/mariadb/jdbc/client/column/UnsignedSmallIntColumn.java#L68-L70

Types.INTEGER will be returned to Spark

On the Spark side, we will go to the following logic:

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

Line 161 in 14d3f44

case _ => None

spark/sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/jdbc/JdbcUtils.scala

Line 208 in 14d3f44

case java.sql.Types.INTEGER => if (signed) IntegerType else LongType

Therefore, Long will be finally obtained.

panbingkun · 2024-05-23T09:37:26Z

IMO, it seems that the 3.x version is more reasonable, but we are not compatible with this case in Spark, so I propose that we can make related changes on the Spark side

@yaooqinn @dongjoon-hyun

yaooqinn · 2024-05-23T09:50:46Z

IMO, it seems that the 3.x version is more reasonable,

It's unreasonable to me. The 2.x and MySQL official both use java.sql.Types.SMALLINT + Signed signal = [UN]SIGNED SMALLINT

panbingkun · 2024-05-23T11:43:09Z

IMO, it seems that the 3.x version is more reasonable,

It's unreasonable to me. The 2.x and MySQL official both use java.sql.Types.SMALLINT + Signed signal = [UN]SIGNED SMALLINT

Okay, I see what you mean. What you say is reasonable is that you focus on the value returned in 3.x and do not make full use of isSigned. What I say is reasonable is that you focus on its own division of UnsignedSmallIntColumn and SignedSmallIntColumn without paying attention to its isSigned. Let me investigate again.
it should have been like this in the original version of 3.x (from 3.0.3)
mariadb-corporation/mariadb-connector-j@22cff70

panbingkun · 2024-05-24T01:28:35Z

Regarding the above issues, I have proposed a PR to mariadb-connector-j: mariadb-corporation/mariadb-connector-j#194

[ONLY TEST] Test mariadb 3.x

78519b4

github-actions bot added SQL BUILD labels May 20, 2024

update

0b38cec

github-actions bot removed the SQL label May 20, 2024

panbingkun added 2 commits May 21, 2024 14:45

add permitMysqlScheme

e32a4ed

Merge branch 'master' into mariadb_3

e284137

github-actions bot added the SQL label May 21, 2024

panbingkun added 2 commits May 21, 2024 15:56

fix

68ade95

fix JDBCSuite

94334aa

panbingkun commented May 22, 2024

View reviewed changes

panbingkun marked this pull request as ready for review May 22, 2024 07:26

panbingkun changed the title ~~[ONLY TEST] Test mariadb 3.x~~ [SPARK-48385][SQL] Migrate the driver of mariadb from 2.x to 3.x May 22, 2024

panbingkun changed the title ~~[SPARK-48385][SQL] Migrate the driver of mariadb from 2.x to 3.x~~ [SPARK-48385][SQL] Migrate the jdbc driver of mariadb from 2.x to 3.x May 22, 2024

update MariaDB image to 10.5.24

e30e5ed

yaooqinn previously approved these changes May 22, 2024

View reviewed changes

dongjoon-hyun reviewed May 22, 2024

View reviewed changes

panbingkun added 2 commits May 23, 2024 17:34

update

91a3d08

update

25f1742

panbingkun mentioned this pull request May 24, 2024

Fix unsigned jdbc types mariadb-corporation/mariadb-connector-j#194

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[SPARK-48385][SQL] Migrate the jdbc driver of mariadb from `2.x` to `3.x` #46655

[SPARK-48385][SQL] Migrate the jdbc driver of mariadb from `2.x` to `3.x` #46655

panbingkun commented May 20, 2024 •

edited

panbingkun May 22, 2024

panbingkun commented May 22, 2024

yaooqinn commented May 22, 2024

panbingkun commented May 22, 2024

panbingkun commented May 22, 2024

dongjoon-hyun May 22, 2024

panbingkun May 23, 2024 •

edited

panbingkun May 23, 2024

panbingkun May 23, 2024

yaooqinn May 23, 2024

panbingkun May 23, 2024

panbingkun commented May 23, 2024 •

edited

panbingkun commented May 23, 2024

yaooqinn commented May 23, 2024

panbingkun commented May 23, 2024

panbingkun commented May 24, 2024

[SPARK-48385][SQL] Migrate the jdbc driver of mariadb from 2.x to 3.x #46655

Are you sure you want to change the base?

[SPARK-48385][SQL] Migrate the jdbc driver of mariadb from 2.x to 3.x #46655

Conversation

panbingkun commented May 20, 2024 • edited

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?

Was this patch authored or co-authored using generative AI tooling?

panbingkun May 22, 2024

Choose a reason for hiding this comment

panbingkun commented May 22, 2024

yaooqinn commented May 22, 2024

panbingkun commented May 22, 2024

panbingkun commented May 22, 2024

dongjoon-hyun May 22, 2024

Choose a reason for hiding this comment

panbingkun May 23, 2024 • edited

Choose a reason for hiding this comment

panbingkun May 23, 2024

Choose a reason for hiding this comment

panbingkun May 23, 2024

Choose a reason for hiding this comment

yaooqinn May 23, 2024

Choose a reason for hiding this comment

panbingkun May 23, 2024

Choose a reason for hiding this comment

panbingkun commented May 23, 2024 • edited

panbingkun commented May 23, 2024

yaooqinn commented May 23, 2024

panbingkun commented May 23, 2024

panbingkun commented May 24, 2024

[SPARK-48385][SQL] Migrate the jdbc driver of mariadb from `2.x` to `3.x` #46655

[SPARK-48385][SQL] Migrate the jdbc driver of mariadb from `2.x` to `3.x` #46655

panbingkun commented May 20, 2024 •

edited

panbingkun May 23, 2024 •

edited

panbingkun commented May 23, 2024 •

edited