embulk-input-jdbc icon indicating copy to clipboard operation
embulk-input-jdbc copied to clipboard

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

Open hiroyuki-sato opened this issue 5 years ago • 16 comments

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

hiroyuki-sato avatar Jun 14 '19 13:06 hiroyuki-sato

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

hito4t avatar Jun 17 '19 02:06 hito4t

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.

hiroyuki-sato avatar Jun 17 '19 02:06 hiroyuki-sato

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.

t-yuki avatar Jan 27 '21 08:01 t-yuki

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);
             }

hiroyuki-sato avatar Jan 27 '21 10:01 hiroyuki-sato

@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();

hito4t avatar Jan 29 '21 00:01 hito4t

@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

hiroyuki-sato avatar Jan 29 '21 01:01 hiroyuki-sato

@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?

hito4t avatar Jan 29 '21 01:01 hito4t

@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);
             }

hiroyuki-sato avatar Jan 29 '21 11:01 hiroyuki-sato

@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.

dmikurube avatar Feb 01 '21 05:02 dmikurube

@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 avatar Feb 01 '21 11:02 hiroyuki-sato

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.

hiroyuki-sato avatar Feb 02 '21 07:02 hiroyuki-sato

@hiroyuki-sato

Is there anything I can do to help move this forward? What was the problem for merging https://github.com/embulk/embulk-input-jdbc/pull/200 ?

rajyan avatar Aug 09 '24 03:08 rajyan

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.

hiroyuki-sato avatar Aug 09 '24 04:08 hiroyuki-sato

@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).

rajyan avatar Aug 09 '24 05:08 rajyan

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

hiroyuki-sato avatar Aug 09 '24 06:08 hiroyuki-sato

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

hiroyuki-sato avatar Aug 10 '24 13:08 hiroyuki-sato