Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

embulk-input-mysql doesn't support Connector/J 8.X #163

Open
hiroyuki-sato opened this issue Jun 14, 2019 · 16 comments
Open

embulk-input-mysql doesn't support Connector/J 8.X #163

hiroyuki-sato opened this issue Jun 14, 2019 · 16 comments

Comments

@hiroyuki-sato
Copy link
Member

It seems that com.mysql.jdbc.TimeUtil removed.

  • embulk: 0.9.17
  • embulk-input-mysql: 0.10.0
  • Connector/J: mysql-connector-java-8.0.16
mysql> select * from space_test;
+----+--------+
| id | name   |
+----+--------+
|  1 | apple  |
|  2 | apple  |
+----+--------+
2 rows in set (0.00 sec)
in:
  type: mysql
  user: user
  password: password
  database: embulk_test
  query: select name,id from space_test
  host: localhost
  driver_path: /tmp/mysql-connector-java-8.0.16.jar
out:
  type: stdout
2019-06-14 21:53:02.396 +0900: Embulk v0.9.17
2019-06-14 21:53:03.018 +0900 [WARN] (main): DEPRECATION: JRuby org.jruby.embed.ScriptingContainer is directly injected.
2019-06-14 21:53:05.806 +0900 [INFO] (main): Gem's home and path are set by default: "/Users/user/.embulk/lib/gems"
2019-06-14 21:53:07.001 +0900 [INFO] (main): Started Embulk v0.9.17
2019-06-14 21:53:07.090 +0900 [INFO] (0001:transaction): Loaded plugin embulk-input-mysql (0.10.0)
Loading class `com.mysql.jdbc.Driver'. This is deprecated. The new driver class is `com.mysql.cj.jdbc.Driver'. The driver is automatically registered via the SPI and manual loading of the driver class is generally unnecessary.
2019-06-14 21:53:07.147 +0900 [INFO] (0001:transaction): Fetch size is 10000. Using server-side prepared statement.
2019-06-14 21:53:07.149 +0900 [INFO] (0001:transaction): Connecting to jdbc:mysql://localhost:3306/embulk_test options {useCompression=true, socketTimeout=1800000, useSSL=false, user=root, useLegacyDatetimeCode=false, tcpKeepAlive=true, useCursorFetch=true, connectTimeout=300000, password=***, zeroDateTimeBehavior=convertToNull}
org.embulk.exec.PartialExecutionException: java.lang.RuntimeException: java.lang.ClassNotFoundException: com.mysql.jdbc.TimeUtil
	at org.embulk.exec.BulkLoader$LoaderState.buildPartialExecuteException(BulkLoader.java:340)
	at org.embulk.exec.BulkLoader.doRun(BulkLoader.java:566)
	at org.embulk.exec.BulkLoader.access$000(BulkLoader.java:35)
	at org.embulk.exec.BulkLoader$1.run(BulkLoader.java:353)
	at org.embulk.exec.BulkLoader$1.run(BulkLoader.java:350)
	at org.embulk.spi.Exec.doWith(Exec.java:22)
	at org.embulk.exec.BulkLoader.run(BulkLoader.java:350)
	at org.embulk.EmbulkEmbed.run(EmbulkEmbed.java:178)
	at org.embulk.EmbulkRunner.runInternal(EmbulkRunner.java:292)
	at org.embulk.EmbulkRunner.run(EmbulkRunner.java:156)
	at org.embulk.cli.EmbulkRun.runSubcommand(EmbulkRun.java:433)
	at org.embulk.cli.EmbulkRun.run(EmbulkRun.java:90)
	at org.embulk.cli.Main.main(Main.java:64)
	Suppressed: java.lang.NullPointerException
		at org.embulk.exec.BulkLoader.doCleanup(BulkLoader.java:463)
		at org.embulk.exec.BulkLoader$3.run(BulkLoader.java:397)
		at org.embulk.exec.BulkLoader$3.run(BulkLoader.java:394)
		at org.embulk.spi.Exec.doWith(Exec.java:22)
		at org.embulk.exec.BulkLoader.cleanup(BulkLoader.java:394)
		at org.embulk.EmbulkEmbed.run(EmbulkEmbed.java:181)
		... 5 more
Caused by: java.lang.RuntimeException: java.lang.ClassNotFoundException: com.mysql.jdbc.TimeUtil
	at com.google.common.base.Throwables.propagate(Throwables.java:160)
	at org.embulk.input.MySQLInputPlugin.loadTimeZoneMappings(MySQLInputPlugin.java:172)
	at org.embulk.input.MySQLInputPlugin.newConnection(MySQLInputPlugin.java:124)
	at org.embulk.input.MySQLInputPlugin.newConnection(MySQLInputPlugin.java:23)
	at org.embulk.input.jdbc.AbstractJdbcInputPlugin.transaction(AbstractJdbcInputPlugin.java:205)
	at org.embulk.exec.BulkLoader.doRun(BulkLoader.java:507)
	... 11 more
Caused by: java.lang.ClassNotFoundException: com.mysql.jdbc.TimeUtil
	at org.embulk.plugin.PluginClassLoader.loadClass(PluginClassLoader.java:161)
	at java.lang.ClassLoader.loadClass(ClassLoader.java:357)
	at java.lang.Class.forName0(Native Method)
	at java.lang.Class.forName(Class.java:264)
	at org.embulk.input.MySQLInputPlugin.loadTimeZoneMappings(MySQLInputPlugin.java:158)
	... 15 more

Error: java.lang.RuntimeException: java.lang.ClassNotFoundException: com.mysql.jdbc.TimeUtil
@hito4t
Copy link
Contributor

hito4t commented Jun 17, 2019

@hiroyuki-sato
Thank you for the information!
It seems that com.mysql.jdbc.TimeUtil was moved to com.mysql.cj.util.TimeUtil .

@hiroyuki-sato
Copy link
Member Author

Hello, @hito4t

Thank you for your comment. I just wanted to share these issues.
I don't have a strong motivation to support Connector/J v8 yet.

I got it. It seems we need to switch com.mysql.jdbc.TimeUtil and com.mysql.cj.util.TimeUtil versions.

@t-yuki
Copy link

t-yuki commented Jan 27, 2021

It seems to be MySQL 8.x Server requires Connector/J v8 because current driver doesn't support caching_sha2_password auth option.
When I try to connect an MySQL 8.x Server, Error: java.lang.RuntimeException: java.sql.SQLException: Unable to load authentication plugin 'caching_sha2_password'. occurs.

@hiroyuki-sato
Copy link
Member Author

Hello, @t-yuki Thank you for your report.

  1. Change the authentication to mysql_native_password instead of caching_sha2_password.
  2. apply the following patch. and run ./gradlew gem It will create embulk-input-mysql-0.11.0-java.gem in the ./embulk-input-mysql/build/gems directory.
  3. I'll try to create a PR for connector/J 8. But I need more work because It needs to support the MySQL 5.x and 8.x.
diff --git a/embulk-input-mysql/build.gradle b/embulk-input-mysql/build.gradle
index 93a93b5..e4603ef 100644
--- a/embulk-input-mysql/build.gradle
+++ b/embulk-input-mysql/build.gradle
@@ -1,11 +1,11 @@
 dependencies {
     compile(project(path: ":embulk-input-jdbc", configuration: "runtimeElements"))
 
-    compileOnly "mysql:mysql-connector-java:5.1.44"
-    defaultJdbcDriver 'mysql:mysql-connector-java:5.1.44'
+    compileOnly "mysql:mysql-connector-java:8.0.23"
+    defaultJdbcDriver 'mysql:mysql-connector-java:8.0.23'
 
     testCompile 'org.embulk:embulk-standards:0.9.23'
-    testCompile "mysql:mysql-connector-java:5.1.44"
+    testCompile "mysql:mysql-connector-java:8.0.23"
 }
 
 embulkPlugin {
diff --git a/embulk-input-mysql/src/main/java/org/embulk/input/MySQLInputPlugin.java b/embulk-input-mysql/src/main/java/org/embulk/input/MySQLInputPlugin.java
index 04f1bca..0d14d50 100644
--- a/embulk-input-mysql/src/main/java/org/embulk/input/MySQLInputPlugin.java
+++ b/embulk-input-mysql/src/main/java/org/embulk/input/MySQLInputPlugin.java
@@ -158,7 +158,7 @@ public class MySQLInputPlugin
         // Here implements a workaround as as workaround.
         Field f = null;
         try {
-            Class<?> timeUtilClass = Class.forName("com.mysql.jdbc.TimeUtil");
+            Class<?> timeUtilClass = Class.forName("com.mysql.cj.util.TimeUtil");
             f = timeUtilClass.getDeclaredField("timeZoneMappings");
             f.setAccessible(true);
 
@@ -166,7 +166,7 @@ public class MySQLInputPlugin
             if (timeZoneMappings == null) {
                 timeZoneMappings = new Properties();
                 synchronized (timeUtilClass) {
-                    timeZoneMappings.load(this.getClass().getResourceAsStream("/com/mysql/jdbc/TimeZoneMapping.properties"));
+                    timeZoneMappings.load(this.getClass().getResourceAsStream("/com/mysql/cj/util/TimeZoneMapping.properties"));
                 }
                 f.set(null, timeZoneMappings);
             }

@hito4t hito4t changed the title MySQL: driver_path doesn't support Connector/J 8.X embulk-input-mysql doesn't support Connector/J 8.X Jan 29, 2021
@hito4t
Copy link
Contributor

hito4t commented Jan 29, 2021

@hiroyuki-sato
I'm looking forward to your PR!

It needs to support the MySQL 5.x and 8.x.

We can detect the driver version by the following code.

            Driver driver = DriverManager.getDriver(url);
            int majorVersion = driver.getMajorVersion();

@hiroyuki-sato
Copy link
Member Author

@hito4t Thank you for your comment. I created #200.
@dmikurube advised me about this implementation. twitter. So I don't use version checking.
Do you have any concerns? Let's discuss this on #200

@hito4t
Copy link
Contributor

hito4t commented Jan 29, 2021

@dmikurube @hiroyuki-sato
We can get the major version number as integer (for example, 5 or 8).
I think it is simpler than try-catch.
What do you think?

@hiroyuki-sato
Copy link
Member Author

@hito4t Thank you for your comment.

The proposed way is simpler than the current way.

@dmikurube What do you think?

diff --git a/embulk-input-mysql/src/main/java/org/embulk/input/MySQLInputPlugin.java b/embulk-input-mysql/src/main/java/org/embulk/input/MySQLInputPlugin.java
index 04f1bca..b333f91 100644
--- a/embulk-input-mysql/src/main/java/org/embulk/input/MySQLInputPlugin.java
+++ b/embulk-input-mysql/src/main/java/org/embulk/input/MySQLInputPlugin.java
@@ -2,6 +2,7 @@ package org.embulk.input;
 
 import java.io.IOException;
 import java.lang.reflect.Field;
+import java.sql.Driver;
 import java.util.Properties;
 import java.sql.Connection;
 import java.sql.DriverManager;
@@ -68,6 +69,7 @@ public class MySQLInputPlugin
     @Override
     protected MySQLInputConnection newConnection(PluginTask task) throws SQLException
     {
+        Driver driver;
         MySQLPluginTask t = (MySQLPluginTask) task;
 
         loadDriver("com.mysql.jdbc.Driver", t.getDriverPath());
@@ -123,8 +125,9 @@ public class MySQLInputPlugin
         props.putAll(t.getOptions());
         logConnectionProperties(url, props);
 
+        driver = DriverManager.getDriver(url);
         // load timezone mappings
-        loadTimeZoneMappings();
+        loadTimeZoneMappings(driver.getMajorVersion());
 
         Connection con = DriverManager.getConnection(url, props);
         try {
@@ -144,8 +147,18 @@ public class MySQLInputPlugin
         return new MySQLColumnGetterFactory(pageBuilder, dateTimeZone);
     }
 
-    private void loadTimeZoneMappings()
+    private void loadTimeZoneMappings(int version)
     {
+        String timeUtilClassName;
+        String timeZonePropName;
+
+        if ( version < 8 ) {
+            timeUtilClassName = "com.mysql.jdbc.TimeUtil";
+            timeZonePropName = "/com/mysql/jdbc/TimeZoneMapping.properties";
+        } else {
+            timeUtilClassName = "com.mysql.cj.util.TimeUtil";
+            timeZonePropName = "/com/mysql/cj/util/TimeZoneMapping.properties";
+        }
         // Here initializes com.mysql.jdbc.TimeUtil.timeZoneMappings static field by calling
         // static timeZoneMappings method using reflection.
         // The field is usually initialized when Driver#connect method is called. But the field
@@ -156,9 +169,11 @@ public class MySQLInputPlugin
         // that loaded com.mysql.jdbc.TimeUtil class rather than system class loader to read the
         // property file because the file should be in the same classpath with the class.
         // Here implements a workaround as as workaround.
+
+
         Field f = null;
         try {
-            Class<?> timeUtilClass = Class.forName("com.mysql.jdbc.TimeUtil");
+            Class<?> timeUtilClass = Class.forName(timeUtilClassName);
             f = timeUtilClass.getDeclaredField("timeZoneMappings");
             f.setAccessible(true);
 
@@ -166,7 +181,7 @@ public class MySQLInputPlugin
             if (timeZoneMappings == null) {
                 timeZoneMappings = new Properties();
                 synchronized (timeUtilClass) {
-                    timeZoneMappings.load(this.getClass().getResourceAsStream("/com/mysql/jdbc/TimeZoneMapping.properties"));
+                    timeZoneMappings.load(this.getClass().getResourceAsStream(timeZonePropName));
                 }
                 f.set(null, timeZoneMappings);
             }

@dmikurube
Copy link
Member

@hito4t @hiroyuki-sato The integer getMajorVersion() does look simpler than parsing a version string, but to be honest, class loading seems simpler to me, because of those questions below:

  • How does this work for MariaDB JDBC, and, their future updates?
  • Does MariaDB maintain their "major version" strategy discriminable from MySQL's one?
  • Is it documented and guaranteed that 5 uses com.mysql.jdbc.TimeUtil and 8 uses com.mysql.cj.util.TimeUtil?

Considering these cases, the version if-statements will grow complicatedly. Rather than that, making practical decisions based on the "actual classes/resources found" would make things simpler.

@hiroyuki-sato
Copy link
Member Author

hiroyuki-sato commented Feb 1, 2021

@dmikurube @hito4t

Here is my current opinion.

  1. If this plugin will supports MySQL Connector/J only, getMajorVersion() is better
  2. If this plugin will supports MariaDB Connector/J, classForName is may better.

There are two reasons.

  • getMajorVersion() can switch the class name com.mysql.jdbc.Driver and com.mysql.cj.jdbc.Driver easily If we supports MySQL only.
  • It seems that MariaDB Connector/J doesn't use TimeZoneMapping.properties and TimeUtil. So, It seems requires additional rework.

I don't modify the following code yet, current implementation load driver like the following.

loadDriver("com.mysql.jdbc.Driver", t.getDriverPath());

When I use this code with MySQL Connector/J v8, It outputs the following warning.

Loading class `com.mysql.jdbc.Driver'. This is deprecated. The new driver class is `com.mysql.cj.jdbc.Driver'. The driver is automatically registered via the SPI and manual loading of the driver class is generally unnecessary.

If we support MySQL 5/8 driver only, We can switch the class name easily.

if ( t.getDriverPath() < 8 ) {
    loadDriver("com.mysql.jdbc.Driver", t.getDriverPath());
} else {
    loadDriver("com.mysql.cj.jdbc.Driver", t.getDriverPath());
}

But It is hard to switch if we use MariaDB Connector/J. and I don't konw how to write is using classForName

It seems that the current input-mysql implementation hard to support MariaDB Connector/J(Because it doesn't have TimeZoneMapping.properties and TimeUtil) It's need more reworks. If we supports MySQL Connector/J only, getMajorVersion seems simpler way.

I have no idea how to write loadDriver() part using classForName yet.

@hiroyuki-sato
Copy link
Member Author

How does this work for MariaDB JDBC, and, their future updates?
Does MariaDB maintain their "major version" strategy discriminable from MySQL's one?

Probably, the latest Mariadb Connecotr/J getMajorVersion returns 2.
So, getMajorVersion() can't use MySQL/MariaDB version checking if we support MariaDB Connector/J

https://github.com/mariadb-corporation/mariadb-connector-j/blob/master/src/main/java/org/mariadb/jdbc/Driver.java#L146-L148
https://github.com/mariadb-corporation/mariadb-connector-j/blob/master/src/main/java/org/mariadb/jdbc/internal/util/constant/Version.java

Is it documented and guaranteed that 5 uses com.mysql.jdbc.TimeUtil and 8 uses com.mysql.cj.util.TimeUtil?

I can't find any documentation about TimeUtil.

@rajyan
Copy link

rajyan commented Aug 9, 2024

@hiroyuki-sato

Is there anything I can do to help move this forward?
What was the problem for merging #200 ?

@hiroyuki-sato
Copy link
Member Author

Hello, @rajyan

Please check this discussion.
https://github.com/orgs/embulk/discussions/19

We need to understand the following.

  • Incompatibility.
  • Deprecated features.

So, We need a summary of What Connector/J changed in the newer version and What changes should be made in the Embulk plugin.

@rajyan
Copy link

rajyan commented Aug 9, 2024

@hiroyuki-sato

Hi, thank you for your swift response!
Yeah, updating default JDBC driver seems a tough task, there are a lot of breaking changes...

Is there way to avoid this timeUtil error at least? Then we can try embulk with Connector/J 8.x (using driver_path) and test to search for the breaking changes.

I'll contribute back to embulk when I have some time to help upgrading the default jdbc driver versions.
For example, this PR https://github.com/embulk/embulk-output-jdbc/pull/343/files (still in progress, II'll look into it later) is from my colleague which is trying to solve the incompatibility of nullCatalogMeansCurrent defaults (ref https://dev.mysql.com/doc/connectors/en/connector-j-upgrading-to-8.0.html).

@hiroyuki-sato
Copy link
Member Author

@rajyan I'll ask other maintainers about your proposal. It is a good start for me. Please wait.

@hiroyuki-sato
Copy link
Member Author

@rajyan I revised #259 and am waiting for the review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

5 participants